[test] Capture stderr from 'tar --version' call as well

[mgorny]

Summary

Capture the stderr from 'tar --version' call as otherwise error messages
spill onto user's terminal unnecessarily (e.g. on NetBSD where tar does
not support long options). While at it, refactor the code to use
communicate() instead of reinventing the wheel.

[MaskRay]

test/lit.cfg.py

99 ↗

(On Diff #177238)

If you don't need stderr, remove stderr=subprocess.PIPE,

subprocess.Popen followed by communicate() can be replaced by check_output

if 'GNU tar' in sout: does not in Python 3 as sout would have type bytes, not str

[ruiu]

test/lit.cfg.py

99 ↗

(On Diff #177238)

I think as Fangrui suggested

subprocess.check_output(["/tmp/foo"], stderr=subprocess.STDOUT)

is better than subprocess.Popen followed by communicate() and wait().

[mgorny]

Sorry, it seems that my reply didn't go through.

test/lit.cfg.py

99 ↗

(On Diff #177238)

I need to silence stderr. Given that /dev/null is not portable, and subprocess.DEVNULL requires py3.3+, capturing it is the simplest solution.

check_output() will throw when tar doesn't support version, i.e. it would break NetBSD completely instead of silencing the error.

I'll fix the type mismatch.

[ruiu]

test/lit.cfg.py

99 ↗

(On Diff #177238)

Oh sorry I didn't know that check_output throws if a subprocess exits with non-zero exit code.

[MaskRay]

test/lit.cfg.py

101 ↗

(On Diff #177250)

LG, but serr can be replaced by _ if it is not used.

[ruiu]

test/lit.cfg.py

100 ↗

(On Diff #177265)

Maybe a silly question, but is this OK to remove this line?

[mgorny]

test/lit.cfg.py

100 ↗

(On Diff #177265)

Yes. communicate() waits on the process.

[krytarowski]

I don't feel enough comfortable with this Python code.

[ruiu]

LGTM

lld/trunk/test/lit.cfg.py

if (config.llvm_libxml2_enabled == '1'):
     config.available_features.add('libxml2')
 
 if config.have_dia_sdk:
     config.available_features.add("diasdk")
 
 tar_executable = lit.util.which('tar', config.environment['PATH'])
 if tar_executable:
     tar_version = subprocess.Popen(
-        [tar_executable, '--version'], stdout=subprocess.PIPE, env={'LANG': 'C'})
-    if 'GNU tar' in tar_version.stdout.read().decode():
+        [tar_executable, '--version'],
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        env={'LANG': 'C'})
+    sout, _ = tar_version.communicate()
+    if 'GNU tar' in sout.decode():
         config.available_features.add('gnutar')
-    tar_version.wait()