Changing PATH in CIBW_ENVIRONMENT can cause wrong Python to be used

[YannickJadoul]

See samuelcolvin/rtoml#8 (comment): if someone changes PATH in CIBW_ENVIRONMENT, the newly installed Python (from nuget on Windows, but I'm guessing the same is possible on macOS or Linux; still need to check) is not used, but some other Python version of the system is used.

So I think we should add some kind of check after applying CIBW_ENVIRONMENT that makes sure we're using the python executable we think we're using.

(I'm happy to actually do it myself, actually, if no one else is eager to do this; but I currently don't have time to do it immediately, so I thought I'd make sure to report it.)

[Czaki]

on linux there is not such problem because of different order of command execution. Before build is evaluated before extending path by python location.

MacOS has same command order like windows.

I think that solution from linux is best.

if [ ! -z {before_build} ]; then
    PATH="$PYBIN:$PATH" sh -c {before_build}
fi

# Build the wheel
rm -rf /tmp/built_wheel
mkdir /tmp/built_wheel
PATH="$PYBIN:$PATH" "$PYBIN/pip" wheel . -w /tmp/built_wheel --no-deps {build_verbosity_flag}
built_wheel=(/tmp/built_wheel/*.whl)

[YannickJadoul]

on linux there is not such problem because of different order of command execution. Before build is evaluated before extending path by python location.

Hmmmm, that's not consistent. I thought we'd made different platforms the same before.

I think that solution from linux is best.

I don't know about that. It might be good to have access to the python that will be used when evaluating environment variables (e.g., PYTHON_VERSION=$(python --version) or so)? @joerick Any thoughts on the (implicitly) assumed order, here?

[YannickJadoul]

Actually; something weirder is going on: CIBW_ENVIRONMENT is only evaluated once (per architecture/manylinux image) on Linux, but it's reevaluated each time on macOS and Windows.

The documentation isn't clear on what's happening; it just says

A space-separated list of environment variables to set during the build.

I would argue to do it for every build, after python is added to PATH, máybe after pip has been added (?), and before CIBW_BEFORE_BUILD. I would argue that the location of python is already part of the environment you're adapting.

But ... I don't really know. Maybe there's better arguments, so some input would be nice.

Afterthought: why do we do it before CIBW_BEFORE_BUILD? Actually, if you need environment variables in CIBW_BEFORE_BUILD, you can still set set them in there. The main reason we need CIBW_ENVIRONMENT is because CIBW_BEFORE_BUILD is run in a subshell. So would it make sense if in some use case CIBW_BEFORE_BUILD prepares a dependency, after which CIBW_ENVIRONMENT adds it to the PATH/environment?

[joerick]

I would argue to do it for every build, after python is added to PATH, máybe after pip has been added (?), and before CIBW_BEFORE_BUILD. I would argue that the location of python is already part of the environment you're adapting.

I think I agree with you here @YannickJadoul, in fact just today I noticed that linux has different behaviour - by coincidence, that is fixed in #256 as a side effect of my changes.

It's important for the correct python and pip to be accessible at the top of the PATH, because user scripts or build systems might be using it. So while the old linux 'belt and braces' approach seems more reliable, it could be masking errors that'll cause weirder errors down the line.

So I'm in favour of your original suggestion - asserting that python and pip are correct after CIBW_ENVIRONMENT is evaluated. And once #256 is merged, the behaviour should be consistent between platforms.

[YannickJadoul]

Thanks, @joerick. I'll make a PR; shouldn't take too long. If #256 gets merged before, it should be easy enough to rebase :-)