test: quote the python interpreter

[compnerd]

This quotes the path to the interpreter which is required to ensure that
spaces in paths do not cause issues when executing the tests.

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

[compnerd]

CC: @drexin

[compnerd]

@swift-ci please smoke test

[drexin]

@compnerd and I discussed this offline. I think it would be good if we could find a way to make the change at the substitution level to avoid changing all the occurrences and also make it more robust for future cases.

[compnerd]

While I agree in principle that quoting in the lit infrastructure would be better, it causes ~200 new failures due to the quoting when the path contains spaces. In particular the use of -driver-use-frontend-path is used with a semicolon delimited argument list. The result is that it ends up getting something like:

"-driver-use-frontend-path" "'C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python37_64\python.exe';S:\SourceCache\swift\test\Driver\Dependencies/Inputs/update-dependencies.py;s:\binarycache\release\windows-x86_64\toolchain\bin\swift-dependency-tool.exe"

which will try to execute the quoted path which won't work.

[drexin]

cc: @yln Do you have any suggestions?

[yln]

My understanding:

• Support path with spaces (temporary workaround: don't have spaces in your Python path)
• Need quotes for normal invocation: %{python} %s %target-os ...
• Must not use quotes for path inside: -driver-use-frontend-path "%{python};%S/Inputs/update-dependencies.py;%swift-dependency-tool"

I also dislike changing %{python} -> "%{python}" everywhere for the reasons mentioned (large change, feels like should be handled at lit substitution level), but also because every new test author has to remember it/copy&paste it.

Is the purpose of the quotes around -driver-use-frontend-path "%{python};%S/Inputs/update-dependencies.py;%swift-dependency-tool" to support spaces in the path of %S and %swift-dependency-tool? Maybe there is a generic solution where all substitutions can be quoted as necessary when setting up their substitutions and then we could skip the quotes around the ;-separated list.

Or another hacky solution: quote %{python} and add %{python_unquoted} for use inside "".

[compnerd]

Yeah, I think that I ended up on the last option - introduce an unquoted python path that can be used for the custom frontend action. I don't think that there is a good solution to knowing what transform the driver should do - the expectation is that it is ; delimited arguments. I'll try to get the other approach working.

[yln]

I don't think that there is a good solution to knowing what transform the driver should do - the expectation is that it is ; delimited arguments.

Just to make sure we are on the same page: my understanding is that the following
-xxx %{python};%S/Inputs/update-dependencies.py could work as long as substations are quoted (when necessary)
-xxx "/path/with sp a c es/python";/some/path/Inputs/update-dependencies.py.

Meaning that the list itself doesn't need quotes as long as the individual elements of the list are properly quoted. I think this can be handled in the lit.cfg when stetting up substitutions. This would of course require more work, but I think would lead to the cleanest solution.

When you say "driver", do you mean the swift/clang frontend? I don't think they need to involved for this.

[compnerd]

I don't think that the -use-driver-frontend %{python};%S/Inputs/update-dependencies.py would work, I think that would get split up into multiple commands with bash. So the argument itself needs to be quoted as: -use-driver-frontend "%{python};%S/Inputs/update-dependencies.py". So, you cannot further quote the inner argument and it is also unnecessary since the driver knows to split on the ; and is able to treat the path with spaces as a single entity. That is not a lit substitution but rather a test dependent argument.

As to when I say driver, I mean the swift driver. The swift driver is involved - it consumes the -use-driver-action argument to control the beahviour of the driver to substitute out the frontend.

[yln]

I don't think that the -use-driver-frontend %{python};%S/Inputs/update-dependencies.py would work, I think that would get split up into multiple commands with bash.

Ahh, I think I now understand. Thanks for explaining!

So the issue is the choice of ; for the list delimiter which also means command separator in bash.
Does the driver allow other tokens to delimit lists? If not, then I don't have any better ideas than %{python_unquoted}.

[compnerd]

It does not, its hardcoded to use ;. While improving that is not a bad idea, I think that I would really be way out of scope for the work on the Python 3 enablement.

[compnerd]

I've updated #32573 and tested it on Python 3 with spaces and on Python 2 without spaces. I think that it is just as large, but less fragile. I'm going to close off this change. Id appreciate it if you could take a look at that change though @yln. Thanks!