Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pants tailor --check :: fails with Unknown flag --check on global scope with new PEX distribution mechanism #249

Closed
huonw opened this issue Aug 24, 2023 · 13 comments · Fixed by #250
Assignees
Labels
bug Something isn't working

Comments

@huonw
Copy link
Contributor

huonw commented Aug 24, 2023

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)

Unknown flag --check on global scope
Did you mean --check-only or --tailor-check?
Use `pants help` to get help.
13:19:23.55 [ERROR] Unknown flags --check on global scope


Use --print-stacktrace for more error details and/or -ldebug for more logs. 
See https://www.pantsbuild.org/v2.18/docs/troubleshooting for common issues.
Consider reaching out for help: https://www.pantsbuild.org/v2.18/docs/getting-help

Reproducer:

cd $(mktemp -d)

cat > pants.toml <<EOF
[GLOBAL]
pants_version = "2.18.0.dev5"

backend_packages = ["pants.backend.python"]
EOF

pants tailor --check ::

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

@huonw huonw added the bug Something isn't working label Aug 24, 2023
@huonw
Copy link
Contributor Author

huonw commented Aug 24, 2023

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.

@huonw huonw changed the title pants tailor --check :: fails with Unknown flag --check on global scope Did you mean --check-only or --tailor-check? pants tailor --check :: fails with Unknown flag --check on global scope Did you mean --check-only or --tailor-check? with new PEX distributions Aug 24, 2023
@huonw huonw changed the title pants tailor --check :: fails with Unknown flag --check on global scope Did you mean --check-only or --tailor-check? with new PEX distributions pants tailor --check :: fails with Unknown flag --check on global scope with new PEX distribution mechanism Aug 24, 2023
@huonw
Copy link
Contributor Author

huonw commented Aug 24, 2023

More research:

  • running the PEX directly works ✅
  • I couldn't get dtruss to work on my Mac to see what scie-pants is doing
  • I could get strace 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 ...>
    
  • Running /root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev5/bin/pants "" tailor --check :: reproduces the failure 🎉
  • The problem is the "": there's an extra empty argument there. I'd suggest there's two problems here:
    • scie-pants passing "". High priority.
    • pants treating "" as a goal, or something?
execve("./scie-pants-linux-aarch64-0.9.0", ["./scie-pants-linux-aarch64-0.9.0", "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=/"]) = 0
execve("/root/.cache/nce/add1bcb4de419fe4ce982c0d308e696d684a6bc55148c50a6d5e2e721ba2591b/scie-pants.bin", ["/root/.cache/nce/add1bcb4de419fe4ce982c0d308e696d684a6bc55148c50a6d5e2e721ba2591b/scie-pants.bin", "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=./scie-pants-linux-aarch64-0.9.0"]) = 0
execve("/code/scie-pants-linux-aarch64-0.9.0", ["/code/scie-pants-linux-aarch64-0.9.0", "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=./scie-pants-linux-aarch64-0.9.0", "SCIE_BOOT=pants", "PANTS_BIN_NAME=./scie-pants-linux-aarch64-0.9.0", "PANTS_DEBUG=", "PANTS_BUILDROOT_OVERRIDE=/code", "PANTS_VERSION=2.18.0.dev5"]) = 0
execve("/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev5/lib/python3.9/site-packages/pants/bin/native_client", ["/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev5/lib/python3.9/site-packages/pants/bin/native_client", "", "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"]) = 0
strace: Process 437 attached
strace: Process 438 attached
strace: Process 439 attached
[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 ...>
[pid   439] +++ exited with 0 +++
[pid   438] +++ exited with 0 +++
[pid   437] +++ exited with 0 +++
<... execve resumed>)                   = 0
strace: Process 440 attached
strace: Process 441 attached
[pid   441] execve("/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev5/bin/python3.9", ["/root/.cache/nce/8887f456eb0b1ff8356ab345c00198195719ef9320d94846f9110a0043fdadb5/bindings/venvs/2.18.0.dev5/bin/python3.9", "/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", "__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.dev5/bin:/root/.cache/nce/f629b75ebfcafe9ceee2e796b7e4df5cf8dbd14f3c021afca078d159ab797acf/cpython-3.9.16+20230507-aarch64-unknown-linux-gnu-install_only.tar.gz/python/lib/python39.zip:"...] <unfinished ...>
[pid   440] +++ exited with 0 +++
[pid   436] --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=440, si_uid=0, si_status=0, si_utime=0, si_stime=0} ---
[pid   441] <... execve resumed>)       = 0
strace: Process 442 attached
strace: Process 443 attached
strace: Process 444 attached
strace: Process 445 attached
strace: Process 446 attached
strace: Process 447 attached
strace: Process 448 attached
strace: Process 449 attached
strace: Process 450 attached
[pid   448] +++ exited with 0 +++
23:03:24.59 [ERROR] Unknown flags --check on global scope


Use --print-stacktrace for more error details and/or -ldebug for more logs. 
See https://www.pantsbuild.org/v2.18/docs/troubleshooting for common issues.
Consider reaching out for help: https://www.pantsbuild.org/v2.18/docs/getting-help

strace: Process 451 attached
[pid   446] +++ exited with 0 +++
[pid   450] +++ exited with 0 +++
[pid   447] +++ exited with 0 +++
[pid   451] +++ exited with 0 +++
[pid   436] +++ exited with 1 +++
[pid   449] +++ exited with 0 +++
[pid   442] +++ exited with 0 +++
[pid   443] +++ exited with 0 +++
[pid   445] +++ exited with 0 +++
[pid   444] +++ exited with 0 +++
+++ exited with 0 +++

@huonw
Copy link
Contributor Author

huonw commented Aug 24, 2023

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...

@huonw
Copy link
Contributor Author

huonw commented Aug 24, 2023

Ah, breadcrumbs:

Now, seems like we could either or both of:

  • have pants ignore empty args always (e.g. pants tailor "" --check :: works, it's just pants "" tailor --check ::)
  • ensure scie-pants doesn't pass spurious empty args, somehow

@thejcannon
Copy link
Member

Those are some very helpful breadcrumbs! Thanks a bunch for digging into this.

According to

# We only want to add the find-links repo for PANTS_SHA invocations so that plugins can
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.

@thejcannon
Copy link
Member

And for full transparency/posterity, I believe this behavior was simply inherited from ./pants script: https://github.com/pantsbuild/setup/blob/0aaed15895033438f03af88f3714bd804b31993c/pants#L369

@thejcannon thejcannon transferred this issue from pantsbuild/pants Aug 25, 2023
@benjyw
Copy link
Sponsor Contributor

benjyw commented Aug 25, 2023

Definitely seems like this should be what we want: ensure scie-pants doesn't pass spurious empty args, somehow

@benjyw
Copy link
Sponsor Contributor

benjyw commented Aug 25, 2023

Great detective work BTW!

@huonw
Copy link
Contributor Author

huonw commented Aug 25, 2023

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 #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.

@huonw huonw self-assigned this Aug 25, 2023
@benjyw
Copy link
Sponsor Contributor

benjyw commented Aug 25, 2023

We can get rid of PANTS_SHA support, sure, but this seems like a general issue with scie, no?

@huonw
Copy link
Contributor Author

huonw commented Aug 25, 2023

Filed a-scie/jump#130 for the base scie-jump feature.

To summarise: there's 3 related things that this issue reveals:

  1. scie-pants's 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) => Pass --python-repos-find-links always, and deprecate PANTS_SHA #250
  2. scie-jump doesn't have support for directly eliding conditional args like this => Support conditionally setting arguments to a command a-scie/jump#130
  3. Tangentially, pants' handling of empty string arguments is a bit peculiar/inconsistent and could be tightened => Ignore all empty string CLI args pants#19666

I'd suggest that #250 seems like the appropriate path forward in the short term, unless scie-jump is improved quickly.

huonw added a commit that referenced this issue Aug 25, 2023
Currently, the `scie-pants` launcher sometimes needs to pass an
additional argument to the underlying pants invocation, to be able to
support `PANTS_SHA` and its custom wheel location. Effectively turning
an invocation like `pants goal --arg` into `pants
--pants-repos-find-links=+["..."] goal --arg` when appropriate:
https://github.com/pantsbuild/scie-pants/blob/0a204c5ac816ad24d74b55ecf7424397a07b3c2a/package/scie-pants.toml#L57-L60

However, when not using `PANTS_SHA`, the pants command becomes `pants ""
goal --arg`, and the spurious empty string results in some commands
failing, e.g. `pants "" tailor --check` thinks `--check` is a global
argument, and this is ambiguous.

To fix this issue, this PR does two things:

1. avoid passing an empty string, via a no-op workaround: always set the
`PANTS_SHA_FIND_LINKS` variable (to a no-op like
`--python-repos-find-links=-[]`), so that scie-pants never passes an
empty variable to the pants command, since that makes commands like
`pants tailor --check ::` explode:
2. deprecates `PANTS_SHA`, so that the hack above doesn't feel so bad:
we're on the way to removing the need for `PANTS_SHA_FIND_LINKS`
entirely, thus sidestepping the limitation and the need for the
workaround (2)

This could also be fixed via a-scie/jump#130.

Fixes #249
@thejcannon
Copy link
Member

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. pantsbuild/setup@0aaed15/pants#L403

Deprecating 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)

@jsirois
Copy link
Contributor

jsirois commented Aug 25, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants