[PATCH] D18826: Add auto-exporting of symbols from tools so that plugins work on Windows

[John Brawn via llvm-commits]

john.brawn added inline comments.

================
Comment at: utils/extract_symbols.py:29
@@ +28,3 @@
+def dumpbin_get_symbols(lib):
+    process = subprocess.Popen(['dumpbin','/symbols',lib], bufsize=1,
+                               stdout=subprocess.PIPE, stdin=subprocess.PIPE)
----------------
silvas wrote:
> beanz wrote:
> > Rather than calling subprocess.Popen you might consider using subprocess.check_output. It would significantly reduce the amount of boiler plate code in this python script.
> Yeah. That's my main comment here too. 
Waiting for subprocess.check_output to have all of the output ready adds a significant delay. I did a quick experiment and with a debug build on Linux it increased time from ~25 seconds to ~45 seconds to run extract_symbols on all the libraries, and on Windows it increased from ~1 minutes to ~4 minutes. Using check_output in objdump_is_32bit_windows and readobj_is_32bit_windows is fine though, as the output is much smaller, so I'll do that.

================
Comment at: utils/extract_symbols.py:334
@@ +333,3 @@
+              # Assume that if nm exists then so does objdump (should be safe as
+              # both are part of binutils)
+              'nm' : (nm_get_symbols, objdump_is_32bit_windows),
----------------
beanz wrote:
> This is not a safe assumption. Apple ships nm but not objdump. In place of objdump we historically shipped otool. The most recent Xcode ships llvm-objdump, but you can't rely on that being present.
Ack. I'm not expecting this to work on OSX, but there's no reason not to fix that at least.


Repository:
  rL LLVM

http://reviews.llvm.org/D18826