Upgrade to Pex 2.1.48 and leverage packed layout. #12808

jsirois · 2021-09-09T12:57:44Z

Pex 2.1.48 brings --layout {packed,loose} alternate layouts for PEXes.
These are both more friendly to remote caching, leading to smaller
artifacts to cache and greater cache hit ratios in the face of
requirement changes. Since the loose layout still does not perform well
with the local CAS scheme, we use packed for now.

Fixes #12548
Fixes #12688
Fixes #12803

[ci skip-rust]
[ci skip-build-wheels]

…omposition. (pantsbuild#12736)" This reverts commit 10ed875. [ci skip-rust] [ci skip-build-wheels]

This reverts commit 30a1459. [ci skip-rust] [ci skip-build-wheels]

Pex 2.1.48 brings `--layout {packed,loose}` alternate layouts for PEXes. These are both more friendly to remote caching, leading to smaller artifacts to cache and greater cache hit ratios in the face of requirement changes. Since the loose layout still does not perform well with the local CAS scheme, we use packed for now. [ci skip-rust] [ci skip-build-wheels]

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

jsirois · 2021-09-09T13:36:56Z

I could really use eyes on the revert of #12675 in 3e1f3bc. That was not a clean revert. I don't want to stomp the subsequent lock file work that tangled the clean revert.

I'll update with a re-run of the local CAS space savings and timing tests I ran earlier this week and add that report here, but the numbers should be exactly the same.

Eric-Arellano

🎉

Eric-Arellano · 2021-09-09T13:38:19Z

src/python/pants/backend/python/util_rules/pex_test.py

+
+
+@dataclass(frozen=True)
+class PexData:


Eric-Arellano · 2021-09-09T13:40:33Z

src/python/pants/init/plugin_resolver.py

@@ -51,9 +51,7 @@ async def resolve_plugins(
    `named_caches` directory), but consequently needs to disable the process cache: see the


FYI changes to this file can be reverted. apply_requirement_constraints defaults to False. (Not blocking)

I'm going to leave Stu's comment and explicit False in in the follow-up since it looks like it marks danger in a useful way.

Eric-Arellano · 2021-09-09T13:48:14Z

src/python/pants/backend/python/util_rules/pex.py

@@ -100,39 +99,28 @@ class ToolCustomLockfile(Lockfile, _ToolLockfileMixin):
 @dataclass(unsafe_hash=True)
 class PexRequirements:
    req_strings: FrozenOrderedSet[str]
-    apply_constraints: bool


I could really use eyes on the revert of #12675 in 3e1f3bc. That was not a clean revert. I don't want to stomp the subsequent lock file work that tangled the clean revert.

I think it was better modeling to define apply_constraints on PexRequirements than PexRequest because it's better namespaced. But either approach is valid - not blocking.

Not a big deal either way because I believe this current plan* is to deprecate [python-setup].requirement_constraints in favor of the new lockfile feature, meaning this code will be deleted.

*I know you've mentioned constraints files could still be useful to pin certain transitive deps, which I agree. But we co-opted them too much with [python-setup].resolve_all_constraints (on by default) that to get people to migrate to lockfiles, deprecating constraints files seems the clearest path. And then maybe we retcon the mechanism after it was removed to behave how you intend. (This plan is open to change.)

The other reason it was moved here was that the two LockFile cases never use the constraints / are "for" the constraints, and so this field was only relevant to directly specified constraints (afaict). But I should have done it as prework, and as Eric says: not a big deal.

Yeah, I'd prefer to keep a revert a revert and follow up with the re-improvement post cherry pick if that makes sense.

Noting I still intend to do this but my stack got deeper here temporarily with the bug this upgrade exposed.

src/python/pants/backend/python/target_types.py

stuhood · 2021-09-09T16:50:02Z

src/python/pants/backend/python/util_rules/pex.py

@@ -100,39 +99,28 @@ class ToolCustomLockfile(Lockfile, _ToolLockfileMixin):
 @dataclass(unsafe_hash=True)
 class PexRequirements:
    req_strings: FrozenOrderedSet[str]
-    apply_constraints: bool


The other reason it was moved here was that the two LockFile cases never use the constraints / are "for" the constraints, and so this field was only relevant to directly specified constraints (afaict). But I should have done it as prework, and as Eric says: not a big deal.

stuhood

Thanks!

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

src/python/pants/backend/python/target_types.py

Eric-Arellano

Thanks! I caught one more place that needs updating:

pants/src/python/pants/backend/python/util_rules/pex_from_targets.py

Line 146 in 19fc101

zip_safe: bool = False,

src/python/pants/backend/python/target_types.py

Co-authored-by: Eric Arellano <[email protected]>

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Eric-Arellano

I'm glad to see the zip_safe arg go :)

Pex 2.1.48 brings `--layout {packed,loose}` alternate layouts for PEXes. These are both more friendly to remote caching, leading to smaller artifacts to cache and greater cache hit ratios in the face of requirement changes. Since the loose layout still does not perform well with the local CAS scheme, we use packed for now. Fixes pantsbuild#12548 Fixes pantsbuild#12688 Fixes pantsbuild#12803 [ci skip-rust] [ci skip-build-wheels]

…uild#12778)) * Add python-docx to the module mapping dictionary ([pantsbuild#12775](pantsbuild#12775)) * Add python-pptx to the module mapping dictionary ([pantsbuild#12776](pantsbuild#12776)) * Add `opencv-python` to the default Python module mapping ([pantsbuild#12777](pantsbuild#12777)) * Add `PyMuPDF` to the default Python module mapping ([pantsbuild#12774](pantsbuild#12774)) * Deprecate `--list-provides` option. ([pantsbuild#12759](pantsbuild#12759)) * Upgrade default `isort` to latest `isort==5.9.3` ([pantsbuild#12756](pantsbuild#12756)) * `OutputPathField.value_or_default()` no longer has an `Address` argument ([pantsbuild#12837](pantsbuild#12837)) * Properly include file dependencies in docker build context ([pantsbuild#12758](pantsbuild#12758)) * DigestSubset should not short-circuit when there are ignores involved. ([pantsbuild#12648](pantsbuild#12648)) * Improve cache reuse for `./pants package` when using a constraints file or lockfile ([pantsbuild#12807](pantsbuild#12807)) * Upgrade to Pex 2.1.48 and leverage packed layout. ([pantsbuild#12808](pantsbuild#12808)) * Fix backports of std lib modules like `dataclasses` not working with dependency inference ([pantsbuild#12818](pantsbuild#12818)) * Warn if `[python-repos]` is set during lockfile generation ([pantsbuild#12800](pantsbuild#12800)) * Add `version` to lockfile metadata headers ([pantsbuild#12788](pantsbuild#12788)) * jvm: add missing space to avoid two words running together ([pantsbuild#12793](pantsbuild#12793)) * Fix a markdown issue in a help string. ([pantsbuild#12766](pantsbuild#12766)) [ci skip-rust]

benjyw · 2021-09-10T22:29:08Z

According to git bisect this PR causes the following test failures on MacOS:

$ ./pants test src/python/pants/backend/python/goals/run_pex_binary_integration_test.py  -- -k test_no_strip_pex_env_issues_12057
18:27:10.12 [ERROR] Completed: test - src/python/pants/backend/python/goals/run_pex_binary_integration_test.py:run_pex_binary_integration failed (exit code 1).
============================= test session starts ==============================
collected 6 items / 5 deselected / 1 selected

src/python/pants/backend/python/goals/run_pex_binary_integration_test.py F [100%]

=================================== FAILURES ===================================
______________________ test_no_strip_pex_env_issues_12057 ______________________

    def test_no_strip_pex_env_issues_12057() -> None:
        sources = {
            "src/app.py": dedent(
                """\
                import os
                import sys
    
    
                if __name__ == "__main__":
                    exit_code = os.environ.get("PANTS_ISSUES_12057")
                    if exit_code is None:
                        os.environ["PANTS_ISSUES_12057"] = "42"
                        os.execv(sys.executable, [sys.executable, *sys.argv])
                    sys.exit(int(exit_code))
                """
            ),
            "src/BUILD": dedent(
                """\
                python_library(name="lib")
                pex_binary(entry_point="app.py")
                """
            ),
        }
        with setup_tmpdir(sources) as tmpdir:
            args = [
                "--backend-packages=pants.backend.python",
                f"--source-root-patterns=['/{tmpdir}/src']",
                "run",
                f"{tmpdir}/src/app.py",
            ]
            result = run_pants(args)
>           assert result.exit_code == 42, result.stderr
E           AssertionError: 18:27:06.61 [INFO] Initializing scheduler...
E             18:27:06.92 [INFO] Scheduler initialized.
E             18:27:06.94 [WARN] Please either set `enabled = true` in the [anonymous-telemetry] section of pants.toml to enable sending anonymous stats to the Pants project to aid development, or set `enabled = false` to disable it. No telemetry sent for this run. An explicit setting will get rid of this message. See https://www.pantsbuild.org/v2.8/docs/anonymous-telemetry for details.
E             Traceback (most recent call last):
E               File "/Users/benjyw/.cache/pants/named_caches/pex_root/unzipped_pexes/0eae43951aa63dc71d105f08c91e74441802b295/.bootstrap/pex/pex.py", line 475, in execute
E                 self.activate()
E               File "/Users/benjyw/.cache/pants/named_caches/pex_root/unzipped_pexes/0eae43951aa63dc71d105f08c91e74441802b295/.bootstrap/pex/pex.py", line 139, in activate
E                 self._activated_dists = self._activate()
E               File "/Users/benjyw/.cache/pants/named_caches/pex_root/unzipped_pexes/0eae43951aa63dc71d105f08c91e74441802b295/.bootstrap/pex/pex.py", line 126, in _activate
E                 activated_dists.extend(env.activate())
E               File "/Users/benjyw/.cache/pants/named_caches/pex_root/unzipped_pexes/0eae43951aa63dc71d105f08c91e74441802b295/.bootstrap/pex/environment.py", line 288, in activate
E                 self._activated_dists = self._activate()
E               File "/Users/benjyw/.cache/pants/named_caches/pex_root/unzipped_pexes/0eae43951aa63dc71d105f08c91e74441802b295/.bootstrap/pex/environment.py", line 632, in _activate
E                 resolved = self.resolve()
E               File "/Users/benjyw/.cache/pants/named_caches/pex_root/unzipped_pexes/0eae43951aa63dc71d105f08c91e74441802b295/.bootstrap/pex/environment.py", line 468, in resolve
E                 self._resolved_dists = self.resolve_dists(all_reqs)
E               File "/Users/benjyw/.cache/pants/named_caches/pex_root/unzipped_pexes/0eae43951aa63dc71d105f08c91e74441802b295/.bootstrap/pex/environment.py", line 474, in resolve_dists
E                 self._update_candidate_distributions(self._load_internal_cache())
E               File "/Users/benjyw/.cache/pants/named_caches/pex_root/unzipped_pexes/0eae43951aa63dc71d105f08c91e74441802b295/.bootstrap/pex/environment.py", line 245, in _update_candidate_distributions
E                 ranked_dist = self._can_add(dist)
E               File "/Users/benjyw/.cache/pants/named_caches/pex_root/unzipped_pexes/0eae43951aa63dc71d105f08c91e74441802b295/.bootstrap/pex/environment.py", line 255, in _can_add
E                 filename, ext = os.path.splitext(os.path.basename(dist.location))
E             AttributeError: 'NoneType' object has no attribute 'location'
E             
E           assert 1 == 42
E            +  where 1 = PantsResult(command=['/Users/benjyw/.cache/pants/named_caches/pex_root/venvs/short/452dd9f9/bin/python3.7', '-m', 'pan...e/var/folders/q4/pv4jnnm944b_ywr3tnp_b1300000gn/T/process-executionFw6qBm/.pants.d/tmp/tmpg2x4nb71.pants.d', pid=40685).exit_code

src/python/pants/backend/python/goals/run_pex_binary_integration_test.py:129: AssertionError
----------------------------- Captured stdout call -----------------------------
pants.log +++ 
pants.log >>> 18:27:06.56 [INFO] handling request: `--no-pantsrc --pants-workdir=/private/var/folders/q4/pv4jnnm944b_ywr3tnp_b1300000gn/T/process-executionFw6qBm/.pants.d/tmp/tmpg2x4nb71.pants.d --print-stacktrace=True --pantsd --pants-config-files=[] --backend-packages=pants.backend.python --source-root-patterns=['/tmpjc9pr3c_/src'] run tmpjc9pr3c_/src/app.py`
pants.log >>> 18:27:09.80 [INFO] request completed: `--no-pantsrc --pants-workdir=/private/var/folders/q4/pv4jnnm944b_ywr3tnp_b1300000gn/T/process-executionFw6qBm/.pants.d/tmp/tmpg2x4nb71.pants.d --print-stacktrace=True --pantsd --pants-config-files=[] --backend-packages=pants.backend.python --source-root-patterns=['/tmpjc9pr3c_/src'] run tmpjc9pr3c_/src/app.py`
pants.log --- 
=========================== short test summary info ============================
FAILED src/python/pants/backend/python/goals/run_pex_binary_integration_test.py::test_no_strip_pex_env_issues_12057
======================= 1 failed, 5 deselected in 6.80s ========================



𐄂 src/python/pants/backend/python/goals/run_pex_binary_integration_test.py:run_pex_binary_integration failed.

All the tests in src/python/pants/backend/python/goals/run_pex_binary_integration_test.py fail, I singled one out above for simplicity.

stuhood · 2021-09-10T22:34:51Z

According to git bisect this PR causes the following test failures on MacOS:

This was fixed in PEX as pex-tool/pex#1443, and should be released soon.

In the meantime, you can fix it by (re)moving ~/.cache/pants/pex_root.

benjyw · 2021-09-10T22:53:40Z

In the meantime, you can fix it by (re)moving ~/.cache/pants/pex_root.

That doesn't appear to fix it.

stuhood · 2021-09-10T23:51:01Z

Oh, shoot... ~/.cache/pants/named_caches/pex_root.

The refactor was a step forward, it was just all the remaining code in the PR that was target of the revert. Follow-up to pantsbuild#12808. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

The refactor was a step forward, it was just all the remaining code in the PR that was target of the revert. Follow-up to #12808.

Pex 2.1.48 brings `--layout {packed,loose}` alternate layouts for PEXes. These are both more friendly to remote caching, leading to smaller artifacts to cache and greater cache hit ratios in the face of requirement changes. Since the loose layout still does not perform well with the local CAS scheme, we use packed for now. Fixes pantsbuild#12548 Fixes pantsbuild#12688 Fixes pantsbuild#12803 (cherry picked from commit 433a4dd) # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

…3a4dd 0d36002 7ba06a5) (#13078) This is a three-part-cherry-pick: 1. Upgrade to Pex 2.1.48 and leverage packed layout. (#12808) Pex 2.1.48 brings `--layout {packed,loose}` alternate layouts for PEXes. These are both more friendly to remote caching, leading to smaller artifacts to cache and greater cache hit ratios in the face of requirement changes. Since the loose layout still does not perform well with the local CAS scheme, we use packed for now. Fixes #12548 Fixes #12688 Fixes #12803 (cherry picked from commit 433a4dd) 2. Upgrade to Pex 2.1.49. (#12853) This fixes a bug activating previously `--not-zip-safe` dependency-only PEXes, which Pants used extensively before the Pex 2.1.48 upgrade. (cherry picked from commit 0d36002) 3. Upgrade to Pex 2.1.50. (#12888) This fixes a bug executing PEX zipapps that do not have the execute bit set, which is the case for the Pex PEX we download and run in the release script. (cherry picked from commit 7ba06a5) With a fourth commit that applies `./build-support/bin/generate_all_lockfiles.sh`. Fixes #13075

jsirois added 5 commits September 8, 2021 20:43

Revert "Improve descriptions of the process executions for PEX_PATH c…

c96287e

…omposition. (pantsbuild#12736)" This reverts commit 10ed875. [ci skip-rust] [ci skip-build-wheels]

Revert "Decompose requirements pexes (pantsbuild#12675)"

3e1f3bc

This reverts commit 30a1459. [ci skip-rust] [ci skip-build-wheels]

Remove "unzip" PEX-INFO probe.

c3956cf

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Simplify and fix setup-py-runner.pex execution.

c154d9a

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

jsirois marked this pull request as ready for review September 9, 2021 13:33

jsirois requested review from stuhood and Eric-Arellano September 9, 2021 13:33

Eric-Arellano approved these changes Sep 9, 2021

View reviewed changes

Eric-Arellano reviewed Sep 9, 2021

View reviewed changes

stuhood reviewed Sep 9, 2021

View reviewed changes

stuhood approved these changes Sep 9, 2021

View reviewed changes

This was referenced Sep 9, 2021

Improve cache reuse for ./pants package when using a constraints file or lockfile #12807

Merged

[internal] Simplify PEX setup for ./pants run #12806

Merged

stuhood added the needs-cherrypick label Sep 9, 2021

stuhood added this to the 2.7.x milestone Sep 9, 2021

Propagate Pex 2.1.48 deprecations to the associated Pants fields.

5f324e5

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

jsirois commented Sep 9, 2021

View reviewed changes

src/python/pants/backend/python/target_types.py Show resolved Hide resolved

Eric-Arellano reviewed Sep 9, 2021

View reviewed changes

src/python/pants/backend/python/target_types.py Outdated Show resolved Hide resolved

jsirois and others added 2 commits September 9, 2021 13:51

Burn a tree to fix a typo.

3e26afd

Co-authored-by: Eric Arellano <[email protected]>

Kill internal use of --not-zip-safe.

8a3a09e

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Eric-Arellano approved these changes Sep 9, 2021

View reviewed changes

jsirois mentioned this pull request Sep 9, 2021

Deprecate Pex options now deprecated in Pex 2.1.48 #12803

Closed

jsirois merged commit 433a4dd into pantsbuild:main Sep 9, 2021

jsirois deleted the pex/packed branch September 9, 2021 21:37

stuhood removed the needs-cherrypick label Sep 9, 2021

jsirois mentioned this pull request Sep 14, 2021

Restore refactor of apply_requirement_constraints in #12675. #12883

Merged

jsirois added a commit that referenced this pull request Sep 14, 2021

Restore refactor of apply_requirement_constraints in #12675. (#12883)

1dacea9

The refactor was a step forward, it was just all the remaining code in the PR that was target of the revert. Follow-up to #12808.

jsirois mentioned this pull request Oct 1, 2021

Upgrade to Pex 2.1.50 and leverage packed layout. (Cherry picks of 433a4dd 0d36002 7ba06a5) #13078

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Pex 2.1.48 and leverage packed layout. #12808

Upgrade to Pex 2.1.48 and leverage packed layout. #12808

jsirois commented Sep 9, 2021 •

edited

Loading

jsirois commented Sep 9, 2021

Eric-Arellano left a comment

Eric-Arellano Sep 9, 2021

Eric-Arellano Sep 9, 2021 •

edited

Loading

jsirois Sep 14, 2021

Eric-Arellano Sep 9, 2021

stuhood Sep 9, 2021

jsirois Sep 9, 2021

jsirois Sep 11, 2021

stuhood Sep 9, 2021

stuhood left a comment

Eric-Arellano left a comment

Eric-Arellano left a comment

benjyw commented Sep 10, 2021

stuhood commented Sep 10, 2021

benjyw commented Sep 10, 2021

stuhood commented Sep 10, 2021

		@@ -51,9 +51,7 @@ async def resolve_plugins(
		`named_caches` directory), but consequently needs to disable the process cache: see the

Upgrade to Pex 2.1.48 and leverage packed layout. #12808

Upgrade to Pex 2.1.48 and leverage packed layout. #12808

Conversation

jsirois commented Sep 9, 2021 • edited Loading

jsirois commented Sep 9, 2021

Eric-Arellano left a comment

Choose a reason for hiding this comment

Eric-Arellano Sep 9, 2021

Choose a reason for hiding this comment

Eric-Arellano Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

jsirois Sep 14, 2021

Choose a reason for hiding this comment

Eric-Arellano Sep 9, 2021

Choose a reason for hiding this comment

stuhood Sep 9, 2021

Choose a reason for hiding this comment

jsirois Sep 9, 2021

Choose a reason for hiding this comment

jsirois Sep 11, 2021

Choose a reason for hiding this comment

stuhood Sep 9, 2021

Choose a reason for hiding this comment

stuhood left a comment

Choose a reason for hiding this comment

Eric-Arellano left a comment

Choose a reason for hiding this comment

Eric-Arellano left a comment

Choose a reason for hiding this comment

benjyw commented Sep 10, 2021

stuhood commented Sep 10, 2021

benjyw commented Sep 10, 2021

stuhood commented Sep 10, 2021

jsirois commented Sep 9, 2021 •

edited

Loading

Eric-Arellano Sep 9, 2021 •

edited

Loading