[PATCH] D99728: [lit, test] Fix test cancellation feature detection

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre marked an inline comment as done.
thopre added inline comments.


================
Comment at: llvm/utils/lit/tests/lit.cfg:83
+                      stderr=subprocess.PIPE, env=config.environment,
+                      universal_newlines=True)
+if proc.returncode == 0:
----------------
thopre wrote:
> jhenderson wrote:
> > yln wrote:
> > > thopre wrote:
> > > > yln wrote:
> > > > > Please use `text=True` in favor of `universal_newlines=True`.
> > > > > 
> > > > > From the docs:
> > > > > > If encoding or errors are specified, or text is true, file objects for stdin, stdout and stderr are opened in text mode using the specified encoding and errors or the io.TextIOWrapper default. The universal_newlines argument is equivalent to text and is provided for backwards compatibility. By default, file objects are opened in binary mode.
> > > > > 
> > > > > We could also use `capture_output=True` instead of `stderr=subprocess.PIPE`.
> > > > Both of these require Python 3.7 and thus fail on Ubuntu 18.04 and RHEL
> > > You are right!  Please ignore my comment.
> > Does `sys.executable` instead of `"python3"` work for your problem (I am not familiar enough with the issue to be sure)? It feels nicer to me, although I can't say any particular reason why.
> I'm not sure what's the problem at the moment. There's a file not found exception on Windows but I have no idea why. I could give it a try and see.
Many thanks, that was the missing bit. I focused on the script but it was Python3 it could not find.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99728