ruiu added inline comments.

================
Comment at: test/lit.cfg.py:99
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        env={'LANG': 'C'})
----------------
ruiu wrote:
> mgorny wrote:
> > MaskRay wrote:
> > > 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`
> > 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.
> I think as Fangrui suggested
> 
>   subprocess.check_output(["/tmp/foo"], stderr=subprocess.STDOUT)
> 
> is better than subprocess.Popen followed by communicate() and wait().
Oh sorry I didn't know that `check_output` throws if a subprocess exits with 
non-zero exit code.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55443/new/

https://reviews.llvm.org/D55443



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to