Prefix the entire setup.py chroot. #12372
Conversation
pantsbuild#12370)" This reverts commit 9f5ff78. [ci skip-rust]
benjyw
force-pushed
the
prefix_setup_py_again
branch
from
b239be2 to
d0fd7df
Compare
benjyw
changed the title
| @@ -28,7 +28,7 @@ python_distribution( | |||
| # used when creating the wheel, which is a good thing. We should be setting this ABI to ensure | |||
| # consumers of pantsbuild.pants are using a compatible interpreter. | |||
| # TODO(7344): the tuple syntax for ext_modules is deprecated. Use Extension once we support it. | |||
| ext_modules=[('native_engine', {'sources': ['src/pants/dummy.c']})], | |||
| ext_modules=[('native_engine', {'sources': ['pants/dummy.c']})], | |||
There was a problem hiding this comment.
This is different from #12359
| description=f"Run setuptools for {req.exported_target.target.address}", | ||
| level=LogLevel.DEBUG, | ||
| ), | ||
| ) | ||
| # Note that output_digest paths are relative to the working_directory. |
There was a problem hiding this comment.
This is different from #12359 - the error there was that I tried to strip "chroot/dist" when it should have just been "dist" (turns out output files are not just specified relative to the working dir, but the output digest is as well).
| @@ -74,11 +75,20 @@ def test_env(rule_runner: RuleRunner) -> None: | |||
| assert b"VAR2=VAL" in result.stdout | |||
|
|
|||
|
|
|||
| def test_output_digest(rule_runner: RuleRunner) -> None: | |||
| @pytest.mark.parametrize("working_directory", ["", "subdir"]) | |||
There was a problem hiding this comment.
This is different from #12359 - it tests that the behavior we rely on above (output digests are relative to the working directory) is in fact true.
|
Note that this PR did trigger wheel building, to prove that it now works. |
| # We prefix the entire chroot, and run with this prefix as the cwd, so that we can capture any | ||
| # changes setup made within it (e.g., when running 'develop') without also capturing other | ||
| # artifacts of the pex process invocation. | ||
| chroot_prefix = "chroot" |
There was a problem hiding this comment.
It seems like we should be doing this more generally. Afaict no good ever comes of mixing binaries in with the sources they operate on at the top level in sandboxes.
More specifically:
Add a prefix to the chroot. This prefix only exists
at build time, and does not affect the built dist,
but allows us to capture setuptools output from input
dirs without capturing the entire sandbox.
No longer prefix just the sources within the chroot.
This was never particularly necessary, and I originally
implemented that somewhat arbitrarily. Note that this means
that from now on dists will have a different internal structure,
but one that is more usual and simpler, so probably better.
Run setup.py with its directory as the cwd. Custom setup.pys
will expect that.
This will be useful when we run the setuptools develop command
on setup.py scripts that build native code and write the .so or
other output files into the input dirs.
This is a replay of #12359 with a fix for the issue that caused it
to be reverted.
[ci skip-rust]