[PATCH] D38010: lit.py: Allow configs and local configs to have a setup_script entry

[Jordan Rose via Phabricator via llvm-commits]

jordan_rose added inline comments.


================
Comment at: utils/lit/tests/Inputs/setup-script/subdir-custom/lit.local.cfg:1
+config.setup_script = 'Inputs/custom.sh'
----------------
zturner wrote:
> jordan_rose wrote:
> > zturner wrote:
> > > zturner wrote:
> > > > Can you make this not be a shell script?  This is essentially saying "This test doesn't work on Windows" when there's no particular reason why it shouldn't
> > > Actually I take that back.  This //can't// be a shell script, because it's saying "this //feature// doesn't work on Windows".  So I think this has to be a Python script, and you just `execfile` it.  If you want to run a shell script, you can still do that by having a Python script which runs a shell script.  But the lit infrastructure should only deal with Python scripts.
> > I'm happy to make the tests use Python—good point—but I don't see why the feature wouldn't work on Windows. Windows can already run Python scripts from the command line, right?
> No, you need to specify the path to the python executable.  You can't just run `foo.py`, you need to run `C:\Python27\python.exe foo.py`.  People can make this work by modifying some environment variables or doing some other tweaking, but that won't work here because it's a feature of the shell, not the OS.  And it could end up pointing to a different python executable than what they ran the test suite is.  If everything is Python you can address this by using `sys.executable`.
> 
> That aside though, given that lit is intended to be cross-platform, it seems like it should actively encourage people to write only cross-platform tests.  If you make it possible to execute shell scripts, people are going to make use of that functionality and then if they ever need to add portability down the line, it's going to be much harder.
Hm, I was treating the docs at https://docs.python.org/2/using/windows.html?highlight=windows#executing-scripts as saying that wasn't necessary, but you'd know better than me. The point about picking a different Python is taken, though.

> That aside though, given that lit is intended to be cross-platform, it seems like it should actively encourage people to write only cross-platform tests.

This part seems silly to me. Plenty of test //content// isn't cross-platform, and the most common kind of lit test are RUN-based shell-like tests that can invoke anything in PATH. I don't think it's a goal of lit to keep people from writing non-portable tests.

That said, I'm fine with making this Python-only.


Repository:
  rL LLVM

https://reviews.llvm.org/D38010