GNOME Bugzilla – Bug 781312
Fix 'Bad file descriptor' error when checking libtool version
Last modified: 2017-06-15 08:05:38 UTC
Created attachment 349865 [details] [review] Fix Running g-ir-scanner on Fedora 25 (libtool 2.4.6) produces the following message on stderr, twice: /usr/bin/libtool: line 2460: printf: write error: Bad file descriptor The error is harmless as we are just checking if libtool is present, but it is misleading for g-ir-scanner to show such an error. Giving libtool a valid stdout fixes the issue. We could instead redirect stderr to /dev/null but this risks hiding a useful error message.
Review of attachment 349865 [details] [review]: ::: giscanner/utils.py @@ +160,3 @@ try: subprocess.check_call([libtool_cmd, '--version'], + stdout=subprocess.PIPE) Isn't the right fix here to do: stdout=open(os.devnull, 'w') ? The problem with doing a pipe is if we're not reading from it, it can result in deadlock.
Created attachment 349877 [details] [review] Fix v2 Thanks for the review. That's a very good point! Opening /dev/null for writing also fixes the issue and doesn't risk deadlocks if there's lots of output.
Review of attachment 349877 [details] [review]: LGTM.
Hi, Unfortunately this broke the build of .gir files on Windows on non-MSYS as the notion of /dev/null does not exist there. We still need to use os.devnull, and open it as writable. With blessings, thank you!
Created attachment 353782 [details] [review] giscanner/utils.py: Fix on non-UNIX-like platforms Hi, Here's the patch to use os.devnull rather than /dev/null. With blessings, thank you!
Review of attachment 353782 [details] [review]: This is triggering a vague memory about `os.devnull` maybe not being available on old Python, but I doubt we care. So LGTM.
Review of attachment 353782 [details] [review]: Hi Colin, Thanks! I pushed the patch as e8f39a0. With blessings, thank you!