Closed huonw closed 1 year ago
I tried a bisect, but running with PANTS_SOURCE=...
seems to works in all circumstances (e.g. even on the exact commits for release_2.18.0.dev5
or release_2.18.0a0
that definitely fail with that reproducer), so... that revealed no new insight.
Suspiciously, the 2.18.0.dev5 release is when we switched to publishing the platform specific PEXes for scie-pants 0.9+ to read (compare 'assets'):
So, may be this is an issue with the new distribution mechanism?!
Running with scie-pants 0.8.2 instead of 0.9+, to force the use of PyPI-based wheels indeed works:
cd $(mktemp -d)
cat > pants.toml <<EOF
[GLOBAL]
pants_version = "2.18.0.dev5"
backend_packages = ["pants.backend.python"]
EOF
# adjust to your platform:
platform=macos-aarch64
for version in 0.8.2 0.9.0; do
curl -L -o scie-pants-$version https://github.com/pantsbuild/scie-pants/releases/download/v$version/scie-pants-$platform
chmod +x scie-pants-$version
echo "*** With $version"
./scie-pants-$version tailor --check ::
echo "*** Exit: $?"
done
Output:
...
*** With 0.8.2
...
*** Exit: 0
...
*** With 0.9.0
...
Unknown flag --check on global scope
Did you mean --check-only or --tailor-check?
...
*** Exit: 1
i.e. 0.8.2 (using old distribution mechanism) passes, 0.9.0 (using new mechanism) fails.
Why would using the PEX change behaviour here? I'm afraid that's a question I don't yet know the answer to.
More research:
SCIE=doesnt_matter python3.9 pants.cp39-darwin_arm64.pex tailor --check ::
works ✅ dtruss
to work on my Mac to see what scie-pants is doingstrace
to work in docker to find the execve
subprocess calls: strace -v -e trace=execve -e verbose=execve --follow-forks --string-limit=300 ./scie-pants-linux-aarch64-0.9.0 tailor --check :: 2> strace.log
. Full output captured below. The interesting execve
(the one by [pid 436] +++ exited with 1 +++
) is:
[pid 436] execve("/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev5/bin/pants", ["/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev5/bin/pants", "", "tailor", "--check", "::"], ["HOSTNAME=df093749c5ba", "PYTHON_VERSION=3.9.16", "PWD=/code", "PYTHON_SETUPTOOLS_VERSION=58.1.0", "HOME=/root", "LANG=C.UTF-8", "GPG_KEY=E3FF2839C048B25C084DEBE9B26995E310250568", "TERM=xterm", "SHLVL=1", "PYTHON_PIP_VERSION=22.0.4", "PYTHON_GET_PIP_SHA256=394be00f13fa1b9aaa47e911bdb59a09c3b2986472130f30aa0bfaf7f3980637", "PYTHON_GET_PIP_URL=https://github.com/pypa/get-pip/raw/d5cb0afaf23b8520f1bbcfed521017b4a95f5c01/public/get-pip.py", "PATH=/usr/local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "_=/usr/bin/strace", "OLDPWD=/", "SCIE=/code/scie-pants-linux-aarch64-0.9.0", "SCIE_ARGV0=/code/scie-pants-linux-aarch64-0.9.0", "PANTS_BIN_NAME=./scie-pants-linux-aarch64-0.9.0", "PANTS_DEBUG=", "PANTS_BUILDROOT_OVERRIDE=/code", "PANTS_VERSION=2.18.0.dev5", "_PANTS_SERVER_EXE=/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev5/bin/pants"] <unfinished ...>
/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev5/bin/pants "" tailor --check ::
reproduces the failure 🎉 ""
: there's an extra empty argument there. I'd suggest there's two problems here:
""
. High priority.""
as a goal, or something?Here's the similar invocation with dev4, that works:
[pid 672] execve("/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev4/bin/python", ["/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev4/bin/python", "/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev4/bin/pants", "--python-repos-find-links=-['https://wheels.pantsbuild.org/simple/']", "tailor", "--check", "::"], ["HOSTNAME=df093749c5ba", "PYTHON_VERSION=3.9.16", "PWD=/code", "PYTHON_SETUPTOOLS_VERSION=58.1.0", "HOME=/root", "LANG=C.UTF-8", "GPG_KEY=E3FF2839C048B25C084DEBE9B26995E310250568", "TERM=xterm", "SHLVL=0", "PYTHON_PIP_VERSION=22.0.4", "PYTHON_GET_PIP_SHA256=394be00f13fa1b9aaa47e911bdb59a09c3b2986472130f30aa0bfaf7f3980637", "PYTHON_GET_PIP_URL=https://github.com/pypa/get-pip/raw/d5cb0afaf23b8520f1bbcfed521017b4a95f5c01/public/get-pip.py", "PATH=/usr/local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "OLDPWD=/", "_=/usr/bin/strace", "SCIE=/code/scie-pants-linux-aarch64-0.9.0", "SCIE_ARGV0=/code/scie-pants-linux-aarch64-0.9.0", "PANTS_BIN_NAME=./scie-pants-linux-aarch64-0.9.0", "PANTS_DEBUG=", "PANTS_BUILDROOT_OVERRIDE=/code", "PANTS_VERSION=2.18.0.dev4", "_PANTS_SERVER_EXE=/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev4/bin/pants", "__PANTS_BIN_NAME=./scie-pants-linux-aarch64-0.9.0", "PANTS_DAEMON_ENTRYPOINT=pants.pantsd.pants_daemon:launch_new_pantsd_instance", "PYTHONPATH=/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev4/bin:/root/.cache/nce/f629b75ebfcafe9ceee2e796b7e4df5cf8dbd14f3c021afca078d159ab797acf/cpython-3.9.16+20230507-aarch64-unknown-linux-gnu-install_only.tar.gz/python/lib/python39.zip:"...] <unfinished ...>
Difference seems to be --python-repos-find-links
arg. Grep doesn't reveal where this comes from...
Ah, breadcrumbs:
.../bin/pants "" tailor --check ::
, and that explodes: symptoms explained!Now, seems like we could either or both of:
pants tailor "" --check ::
works, it's just pants "" tailor --check ::
)Those are some very helpful breadcrumbs! Thanks a bunch for digging into this.
According to https://github.com/pantsbuild/scie-pants/blob/0a204c5ac816ad24d74b55ecf7424397a07b3c2a/tools/src/scie_pants/pants_version.py#L45 the reason scie-pants
provides this arg is so that PANTS_SHA
invocations will find the right version of Pants.
Considering that PANTS_SHA
support is essentially deprecated and considering that older Pants releases that have been removed from PyPI are now at our new cheeseshop, I'm leaning toward removing this altogether.
The user can, of course, still specify these things manually (albeit a bit of a PITA), use an older version of scie-pants
or revert back to the ./pants
script.
And for full transparency/posterity, I believe this behavior was simply inherited from ./pants
script: https://github.com/pantsbuild/setup/blob/0aaed15895033438f03af88f3714bd804b31993c/pants#L369
Definitely seems like this should be what we want: ensure scie-pants doesn't pass spurious empty args, somehow
Great detective work BTW!
The ./pants
script behaviour is subtlety different: the argument is passed to pip install
unquoted, so an empty string expands to no argument at all. https://github.com/pantsbuild/setup/blob/0aaed15895033438f03af88f3714bd804b31993c/pants#L403
Deprecating and removing the PANTS_SHA
support sounds reasonable. I've sketched something along those lines in https://github.com/pantsbuild/scie-pants/pull/250, which I'll revisit in a while (feel free to take over, though).
I guess it'd also make sense to file a feature request with scie about passing an argument conditionally, which I'll do in a while too.
We can get rid of PANTS_SHA
support, sure, but this seems like a general issue with scie, no?
Filed https://github.com/a-scie/jump/issues/130 for the base scie-jump feature.
To summarise: there's 3 related things that this issue reveals:
PANTS_SHA
support means it needs to sometimes pass an extra arg to pants, and the passing an arg "sometimes" causes issues, but we can work around (and having a work-around isn't so bad if we're planning to remove PANTS_SHA
support) => #250I'd suggest that #250 seems like the appropriate path forward in the short term, unless scie-jump is improved quickly.
The
./pants
script behaviour is subtlety different: the argument is passed topip install
unquoted, so an empty string expands to no argument at all. pantsbuild/setup@0aaed15
/pants#L403Deprecating and removing the
PANTS_SHA
support sounds reasonable. I've sketched something along those lines in #250, which I'll revisit in a while (feel free to take over, though).I guess it'd also make sense to file a feature request with scie about passing an argument conditionally, which I'll do in a while too.
Ah you're right, I think this is the relevant code (although it uses --python-repos-repos
and not --python-repos-find-links
hmm)
I think you may have missed the fact that scie-pants already deals with variable argument lists (which scie-jump does not support). See the pants
vs pants-debug
commands and the dispatch done by the scie-pants rust binary.
Describe the bug In 2.18.0.dev5 and beyond,
pants tailor --check ::
stops working when run under scie-pants 0.9.0 or greater.This invocation is likely to be widespread, as it is suggested by docs and examples. It'd be really good to not break (at least, not without deprecation warnings)
Reproducer:
Changing the version to 2.18.0.dev4 passes (no output).
Pants version 2.18.0.dev5 to 2.18.0a0 at the time of writing
scie-pants 0.9.0, 0.9.3
OS macOS and Linux
Additional info
git bisect
though.~ See first comment: it's the switch to PEX distributions