Edit path when building sdist package

[OlenaYefymenko]

This patch changes the working directory from the temp to the project when building sdist package. This resolves issues with relative paths in configuration files.

Resolves #1683

[joerick]

Nice one, thanks! Could you also add an integration test for this? It might be possible to add an assert to an existing test in

https://github.com/pypa/cibuildwheel/blob/main/test/test_from_sdist.py

[webknjaz]

@joerick TBH it's not exactly obvious what kind of check could be added into cibuildwheel_from_sdist_run. Any clues? The internal CWD is only accessible from within a subprocess, right? Wouldn't that require something like an in-tree build back-end that would save that dir somewhere accessible externally?

[joerick]

CIBW_BEFORE_BUILD runs from the CWD, so we could check we are in the project dir by doing something like CIBW_BEFORE_BUILD=python3 -c 'import os; assert os.path.exists("setup.py")'.

[webknjaz]

UPD: adding tests may take a while since Olena is a beginner and needs to learn a thing or two about the ecosystem. I was helping her out to figure out how to run tests locally and learned that following the CONTRIBUTING instructions doesn't work — the way the tests are being executed requires something like GITHUB_ACTIONS=true nox -s tests to pretend the env is a CI. Otherwise, the underlying invocation of cibuildwheel in tests errors out with a return code of 2 and an error message saying that it couldn't detect a CI.

@joerick have you considered synchronizing the CIs with the way the invocations are supposed to run locally to avoid such discrepancies? As in run nox everywhere the same (ish) way? One interesting approach I saw in some other projects was having a dedicated "dev env" CI job that essentially runs the tools in a way the contributors would do it locally.

[joerick]

Apologies, that doc does probably need a note about this. I wouldn't recommend setting the GITHUB_ACTIONS like you mention, rather what I do is set CIBW_PLATFORM e.g. CIBW_PLATFORM=macos or CIBW_PLATFORM=linux (both run locally on my Mac, because the linux one runs in docker).

I don't use the nox integration personally, because the test suite takes a while to run, so I generally want to be more specific. I will generally do pytest unit_test or if I want to run an integration test, I might do CIBW_PLATFORM=linux pytest test -k <test_name>.

Just one caveat to the above - for local runs with CIBW_PLATFORM=macos, your system might be missing some versions of Python - you can install them from Python.org, the error message should include the link to download.

[joerick]

@joerick have you considered synchronizing the CIs with the way the invocations are supposed to run locally to avoid such discrepancies? As in run nox everywhere the same (ish) way? One interesting approach I saw in some other projects was having a dedicated "dev env" CI job that essentially runs the tools in a way the contributors would do it locally.

Interesting idea... though really the problem here is that the docs are incomplete, I don't think we can test for that.

[webknjaz]

I don't use the nox integration personally, because the test suite takes a while to run, so I generally want to be more specific. I will generally do pytest unit_test or if I want to run an integration test, I might do CIBW_PLATFORM=linux pytest test -k <test_name>.

@joerick I checked the noxfile.py and it does allow you to pass arbitrary args to pytest. So you can do nox -s tests -- unit_test or nox -s tests -- test -k <test_name>. This is a commonly known approach that is usually implemented with tox too. Could you try it out? This should eliminate the need for you to run pytest directly.

[webknjaz]

@joerick have you considered synchronizing the CIs with the way the invocations are supposed to run locally to avoid such discrepancies? As in run nox everywhere the same (ish) way? One interesting approach I saw in some other projects was having a dedicated "dev env" CI job that essentially runs the tools in a way the contributors would do it locally.

Interesting idea... though really the problem here is that the docs are incomplete, I don't think we can test for that.

The idea with workflows tools like tox / nox is that you absolutely can make the invocations nearly the same locally and in CI without having to compromise the ability to invoke the underlying tools with customizations. This is why the posargs supplied after -- are usually bypassed to those integrated invocations in the respective configs.

Note that both tools have ways to list the configured sessions. They are tox -av or nox --list — and it's a convenient centralized place to document what different sessions implements. For tox, there's a dedicated setting for the env description, and in the case of nox, these are taken from docstrings.

If the contributing doc says to check out the sessions in the workflow tool, with a few more concrete examples, and the CI runs the same, that's usually the best way of making sure there's no difference between what different people run.

A more sophisticated solution could be auto-generating a portion of the contributing doc from the nox file.

[henryiii]

FYI, you can also use CIBW_PLATFORM=linux nox -Rs tests -- test -k <test_name> which will avoid reinstalling anything and be just as fast as running it manually (at least the second time it runs).

[joerick]

I've just pushed a PR #1698 with an improvement to this page. My opinion is that the direct pytest invocation is better, as I'm nearly always using some pytest-specific option, so it's extra level of indirection and mental overhead to also think about the nox layer, and a little confusing to thing about passing options to nox that behind the scenes get passed through to pytest. Also, that's the method I always use, so I'm more likely to notice if it changes.

Regarding using nox in CI, it's complicated by the fact that we do a few tricks in CI (like CI-specific flags for docker/podman checks, parallel execution) that we probably wouldn't pass on to the end-user. Perhaps those features from bin/run_tests.py could be added to the nox task. That might be okay, but I'm more of the opinion that the problem here is just missing documentation.

[webknjaz]

@joerick while I understand your sentiment, people are quite used to passing arguments to the wrapped commands. This is quite idiomatic in the tox world (and by extension, nox too). The whole idea is that this ends up always using the correct environment that is set up properly and the same way as in the CI. Tox also has --devenv for exposing disconnected envs, not sure nox has the same.
Either way, you stated that youdo things this way, which is fine because you're proficient and know enough implementation details. But it's hugely different for an arbitrary contributor. This is why, their starting point should be as close to the CI as possible. This doesn't prevent folks from doing advanced stuff but lowers the entry barrier for the beginners, which I think is important.

As for the custom CLI options, a better place for them is pytest. I can take a look into that, if that's something you'd like to explore.

[webknjaz]

@henryiii @joerick approve the GHA run?

[webknjaz]

No CI failures so far 🤞 Waiting for the completion of the rest of the jobs.

[webknjaz]

@joerick @henryiii the CI is fully green. Would you like some commits to be polished before merging?

[henryiii]

Nah, squash and merge is fine unless files both move and change.

[joerick]

Thanks all for this!