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 Pex releases broke w/ venv mode #11954

Closed
Eric-Arellano opened this issue Apr 21, 2021 · 2 comments · Fixed by #14819
Closed

Pants Pex releases broke w/ venv mode #11954

Eric-Arellano opened this issue Apr 21, 2021 · 2 comments · Fixed by #14819
Assignees
Labels

Comments

@Eric-Arellano
Copy link
Contributor

They originally worked when added for 2.4.0.dev0 in #11591, but broke somewhere in between there and 2.4.0.0rc1 (no PEX releases between then, which was unintentional).

❯ wget https://github.com/pantsbuild/pants/releases/download/release_2.4.0rc1/pants.2.4.0rc1.pex
❯ mv pants.2.4.0rc1.pex dist
❯ cd dist
❯ ./pants.2.4.0rc1.pex --print-stacktrace
15:46:51.05 [INFO] initializing scheduler...
15:46:51.14 [ERROR] no path specified
Traceback (most recent call last):
  File "/data/home/eric/.pex/venvs/short/d02a7ea7/lib/python3.7/site-packages/pants/bin/daemon_pants_runner.py", line 125, in single_daemonized_run
    scheduler, options_initializer = self._core.prepare(options_bootstrapper, complete_env)
  File "/data/home/eric/.pex/venvs/short/d02a7ea7/lib/python3.7/site-packages/pants/pantsd/pants_daemon_core.py", line 129, in prepare
    self._initialize(options_fingerprint, options_bootstrapper, env)
  File "/data/home/eric/.pex/venvs/short/d02a7ea7/lib/python3.7/site-packages/pants/pantsd/pants_daemon_core.py", line 104, in _initialize
    raise e
  File "/data/home/eric/.pex/venvs/short/d02a7ea7/lib/python3.7/site-packages/pants/pantsd/pants_daemon_core.py", line 98, in _initialize
    self._services = self._services_constructor(bootstrap_options_values, self._scheduler)
  File "/data/home/eric/.pex/venvs/short/d02a7ea7/lib/python3.7/site-packages/pants/pantsd/pants_daemon.py", line 91, in _setup_services
    bootstrap_options,
  File "/data/home/eric/.pex/venvs/short/d02a7ea7/lib/python3.7/site-packages/pants/option/global_options.py", line 1386, in compute_pantsd_invalidation_globs
    glob_relpath = os.path.relpath(glob, buildroot)
  File "/data/home/eric/.pyenv/versions/3.7.9/lib/python3.7/posixpath.py", line 457, in relpath
    raise ValueError("no path specified")
ValueError: no path specified

I time boxed myself investigating this, so am not sure why this is happening yet. However, using --unzip mode fixes the issue.

--

Beyond the underlying Pex issue this might reveal, it shows the need for our release process to add a smoke test to building PEXes, something as simple as running --version.

@jsirois
Copy link
Member

jsirois commented Apr 21, 2021

The issue is a sys.path entry of "" - i.e.: cwd.

# Globs calculated from the sys.path and other file-like configuration need to be sanitized
# to relative globs (where possible).
potentially_absolute_globs = (
*sys.path,
*bootstrap_options.pythonpath,
*bootstrap_options.pants_config_files,
)
for glob in potentially_absolute_globs:
# NB: We use `relpath` here because these paths are untrusted, and might need to be
# normalized in addition to being relativized.
glob_relpath = os.path.relpath(glob, buildroot)

Changing the relpath line to glob_relpath = os.path.relpath(glob or os.getcwd(), buildroot) fixes.

@jsirois
Copy link
Member

jsirois commented Apr 21, 2021

More generally, the relpath should only be attempted for globs that are absolute paths in the first place.

Eric-Arellano added a commit that referenced this issue Apr 21, 2021
#11955)

Works around #11954. `--unzip` should give still good enough of performance.

[ci skip-rust]
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this issue Apr 21, 2021
pantsbuild#11955)

Works around pantsbuild#11954. `--unzip` should give still good enough of performance.

[ci skip-rust]
Eric-Arellano added a commit that referenced this issue Apr 21, 2021
More progress on #11952. There is nothing left in Bash except for the processing of options, which then pipes to Python; and setting the Python interpreter to run the Python script. `release.sh` will be removed in a followup.

This adds lightweight validation to our PEX build process, which would have caught #11954.

Certainly, we should be building the PEX via `./pants package` instead of directly via PEX. But this is an incremental improvement from before.

[ci skip-rust]
@benjyw benjyw removed infra labels Sep 9, 2021
@Eric-Arellano Eric-Arellano removed their assignment Dec 7, 2021
@Eric-Arellano Eric-Arellano self-assigned this Mar 16, 2022
@stuhood stuhood assigned stuhood and unassigned Eric-Arellano Mar 16, 2022
stuhood added a commit that referenced this issue Mar 17, 2022
stuhood added a commit to stuhood/pants that referenced this issue Mar 17, 2022
…b calculation. (pantsbuild#14819)

Fixes pantsbuild#11954.

[ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
Eric-Arellano added a commit that referenced this issue Mar 17, 2022
stuhood added a commit that referenced this issue Mar 17, 2022
…b calculation. (cherrypick of #14819) (#14822)

Fixes #11954.

[ci skip-rust]
stuhood added a commit to stuhood/pants that referenced this issue Mar 30, 2022
…b calculation. (pantsbuild#14819)

Fixes pantsbuild#11954.

[ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
stuhood added a commit that referenced this issue Mar 31, 2022
…b calculation. (cherrypick of #14819) (#14961)

Fixes #11954.

[ci skip-rust]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants