Created attachment 637839 [details] [diff] [review]
This causes problems with mingw compilation, since it's a MSVC tool. There is an alternative that looks really promising. It's pefile python library . The attached patch imports current trunk version of pefile and uses it in dependentlibs.py instead of dumpbin. This way we can handle PE files without any external dependency. Although the patch is quite big, it's because it imports whole pefile.py. This file required no modifications at all, so there is no maintenance cost. The library is quite powerful - it allows both extensive analyzing of PE files and modifying them. We don't need most of it right not, but the code is in place if needed at some point and it's easier to import it all than split parts (also in terms of maintenance). The license of pefile is MIT, so, as far as I understand, there is nothing special we have to do about it.
The modification of dependentlibs.py is very minimal.
Comment on attachment 637839 [details] [diff] [review]
I'm not a big fan of pulling 200KB of external code just to do that. I'd be more in favor of something like what i did for virtualenv for OSX in https://github.com/pypa/virtualenv/pull/289/files, instead of importing macholib. Alternatively, doesn't mingw come with an objdump that knows how to read PE files? You could also parse its output.
Created attachment 640563 [details] [diff] [review]
I'd prefer to follow your first suggestion, since parsing output of tools not really intended to be parsed is always hacky, but let's go with much simpler solution of using objdump. The attached patch uses objdump if dumpbin fails to execute.
Comment on attachment 640563 [details] [diff] [review]
Review of attachment 640563 [details] [diff] [review]:
@@ +20,5 @@
> + try:
> + proc = subprocess.Popen(['dumpbin', '-imports', lib], stdout = subprocess.PIPE)
> + except OSError:
> + # dumpbin is missing, probably mingw compilation. Try using objdump.
> + return dependentlibs_objdump(lib)
Please make that dependentlibs_mingw_objdump, since the mingw objdump output is significantly different from that of ELF objdump. r=me with that.
Thanks for the review, pushed fixed version: