Don't raise an error if there are spaces in file paths supplied to PIP_CONSTRAINT

[agriyakhetarpal]

Description

In the file

pyodide-build/pyodide_build/build_env.py

Line 339 in 7126c5f

"PIP_CONSTRAINT contains spaces so pip will misinterpret it. Make sure the path to pyodide has no spaces.\n"

and in the RecipeBuilder class, we were checking if the PIP_CONSTRAINT supplied contains spaces and we were raising a ValueError, and this came up in pypa/cibuildwheel#2002 (comment). The issue exists with pip: pypa/pip#10114; however, there is a reasonable workaround – we can use a URI in order to not let a space in the file path be interpreted as a separate file.

cc: @joerick @hoodmane

[agriyakhetarpal]

Would it be alright to release 0.30.5 with #209 and this PR?

[ryanking13]

Thanks! I left a minor comment.

[joerick]

I'm a little confused. If somebody outside pyodide_build is setting the environment variable PIP_CONSTRAINT to /path to file/with spaces.txt, then that's a misconfiguration - pip would fail to understand this.

It looks to me that pyodide-build will take the above and transform it into file:///path%20to%20file/with%20spaces.txt. That's potentially helpful(?), but also that means that users can't pass multiple files like file1.txt file2.txt, because pyodide-build would transform it into file://file1.txt%20file2.txt.

I'm not sure why pyodide-build needs to do any transformation to the value of PIP_CONSTRAINT. Is there a version of this where pyodide-build just leaves the value alone?

[agriyakhetarpal]

Thanks for your thoughts, @joerick – this was also my source of confusion; I thought that we were just wanting to accept spaces with file paths to accommodate cibuildwheel, but on re-reading combine_constraints(), it looks like it is combining cibuildwheel's constraints with the user's constraints, and the fact that pyodide-build is not allowing constraints with spaces is part of the problem.

I cannot think of a neat way to allow multiple constraints files with PIP_CONSTRAINT and still allow a file with spaces to be interpreted correctly, so I'll go ahead and remove the workaround from here.

So, would I be right here if I say that the implementation should be:

1. If a user supplies a file via PIP_CONSTRAINT to pyodide-build and there are no spaces, we proceed in the normal fashion, and cibuildwheel can combine/extend it with its constraints if it wants to
2. If a user wants to supply multiple constraints files to pyodide-build, they use PIP_CONSTRAINT="my_constraints1.txt my_constraints2.txt" as usual and pip interprets them as multiple files
3. If a user wants to supply a constraint file to pyodide-build whose name/path contains spaces, they are the ones to implement and use the URI workaround themselves, and we shouldn't operate on the PIP_CONSTRAINT file ourselves. cibuildwheel can then extend the PIP_CONSTRAINT it receives from pyodide-build correctly as it will have a file:// in the string, and combine its own constraints without issues.

with the assumption that our intention with this PR is to undo https://github.com/pypa/cibuildwheel/blob/e92147f52a3f32818588e968f4be9315c8c35635/test/test_environment.py#L103-L108?

[agriyakhetarpal]

With the above (reverted) commits and changes, we no longer raise an error when there are spaces in the file path, and rather add a message about this PIP_CONSTRAINTS problem to keep them informed. I still don't think we as downstream users should be responsible for the pip bug, but then anything we can do to help our users is always helpful.

[agriyakhetarpal]

The CI failure looks unrelated; I'll debug it.

Edit: it is unrelated – I can see it locally after a refresh of my dependencies 😕

Edit 2: something must have changed and Rich/Click/Typer now sends logger.error() messages to stderr instead of stdout – I've fixed the failing test.

[joerick]

I suppose you'd have to split and take the first entry if you're keeping the following code? constraints might contain two files, so Path(constraints) wouldn't refer to a file.

[ryanking13]

Huh, I didn't know PIP_CONSTRAINT accepts multiple constraint files. Then I am leaning towards following joerick's suggestion: allow spaces in the PIP_CONSTRAINT variable, but consider them as separate files.

Actually, in that case, we can remove some logic in pyodide-build. We have a logic to merge the user-provided PIP_CONSTRAINT with the ones provided by pyodide-build. But instead, we can just append the pyodide-build-provided constraints into PIP_CONSTRAINT.