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

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

Merged
merged 2 commits into from
Sep 9, 2021

Conversation

Eric-Arellano
Copy link
Contributor

Improves upon the solution from #12076. There are some args like --manylinux that we need to use both with the lockfile.pex and the final PexFromTargets. But many others like --strip-pex-env are irrelevant for the lockfile.pex, and we only need to set on the final PEX.

WIth this diff:

diff --git a/build-support/bin/BUILD b/build-support/bin/BUILD
index d9751179b..2a6e76478 100644
--- a/build-support/bin/BUILD
+++ b/build-support/bin/BUILD
@@ -17,4 +17,4 @@ pex_binary(name="generate_docs", entry_point="generate_docs.py", dependencies=["
 pex_binary(name="generate_github_workflows", entry_point="generate_github_workflows.py")
 pex_binary(name="generate_user_list", entry_point="generate_user_list.py", dependencies=[":user_list_templates"])
 pex_binary(name="release_helper", entry_point="_release_helper.py")
-pex_binary(name="reversion", entry_point="reversion.py")
+pex_binary(name="reversion", entry_point="reversion.py", always_write_cache=True)

Before:

❯ ./pants --no-process-execution-local-cache --no-remote-cache-read --no-pantsd package build-support/bin:
...
23:50:18.32 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
23:50:18.32 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt

After:

❯ ./pants --no-process-execution-local-cache --no-remote-cache-read --no-pantsd package build-support/bin:
...
23:49:14.07 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
...

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

…le or lockfile

# 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
Copy link
Contributor Author

cc @asherf - this should help fix your find that ./pants package :: is resolving the lockflie multiple times :) Thanks for that report!

@stuhood
Copy link
Sponsor Member

stuhood commented Sep 9, 2021

This seems fine... I'm a little unclear on how this interplays with multiple-user resolves. It seems like "lockfile args" are basically "args to be used in the identity of a resolve", and would be best specified via named resolve config?

Either way: will wait for #12808 to land to review.

@Eric-Arellano
Copy link
Contributor Author

It seems like "lockfile args" are basically "args to be used in the identity of a resolve", and would be best specified via named resolve config?

Ah, good insight! Yes, I do think these args need to be included in the resolve metadata somehow, but we can probably punt for now given that lockfile is being put on pause. This at least gets us closer to the solution. cc @chrisjrn

# Conflicts:
#	src/python/pants/backend/python/util_rules/pex_from_targets.py

# 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]
@@ -104,6 +106,10 @@ def __init__(
interpreter constraints to not be used because platforms already constrain the valid
Python versions, e.g. by including `cp36m` in the platform string.
:param additional_args: Any additional Pex flags.
:param additional_args: Any additional Pex flags that should be used with the lockfile.pex.
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:param additional_args: Any additional Pex flags that should be used with the lockfile.pex.
:param additional_lockfile_args: Any additional Pex flags that should be used with the lockfile.pex.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, missed by 43 seconds! Will fix in a related followup.

@Eric-Arellano Eric-Arellano deleted the extra-args branch September 9, 2021 22:41
@stuhood stuhood added this to the 2.7.x milestone Sep 9, 2021
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this pull request Sep 9, 2021
…le or lockfile (pantsbuild#12807)

Improves upon the solution from pantsbuild#12076. There are some args like `--manylinux` that we need to use both with the lockfile.pex and the final `PexFromTargets`. But many others like `--strip-pex-env` are irrelevant for the lockfile.pex, and we only need to set on the final PEX.

WIth this diff:

```diff
diff --git a/build-support/bin/BUILD b/build-support/bin/BUILD
index d9751179b..2a6e76478 100644
--- a/build-support/bin/BUILD
+++ b/build-support/bin/BUILD
@@ -17,4 +17,4 @@ pex_binary(name="generate_docs", entry_point="generate_docs.py", dependencies=["
 pex_binary(name="generate_github_workflows", entry_point="generate_github_workflows.py")
 pex_binary(name="generate_user_list", entry_point="generate_user_list.py", dependencies=[":user_list_templates"])
 pex_binary(name="release_helper", entry_point="_release_helper.py")
-pex_binary(name="reversion", entry_point="reversion.py")
+pex_binary(name="reversion", entry_point="reversion.py", always_write_cache=True)
```

Before:

```
❯ ./pants --no-process-execution-local-cache --no-remote-cache-read --no-pantsd package build-support/bin:
...
23:50:18.32 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
23:50:18.32 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
```

After:

```
❯ ./pants --no-process-execution-local-cache --no-remote-cache-read --no-pantsd package build-support/bin:
...
23:49:14.07 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
...
```

[ci skip-rust]
[ci skip-build-wheels]
jsirois added a commit to jsirois/pants that referenced this pull request Sep 10, 2021
…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]
Eric-Arellano added a commit that referenced this pull request Sep 10, 2021
…le or lockfile (#12807) (#12836)

Improves upon the solution from #12076. There are some args like `--manylinux` that we need to use both with the lockfile.pex and the final `PexFromTargets`. But many others like `--strip-pex-env` are irrelevant for the lockfile.pex, and we only need to set on the final PEX.

WIth this diff:

```diff
diff --git a/build-support/bin/BUILD b/build-support/bin/BUILD
index d9751179b..2a6e76478 100644
--- a/build-support/bin/BUILD
+++ b/build-support/bin/BUILD
@@ -17,4 +17,4 @@ pex_binary(name="generate_docs", entry_point="generate_docs.py", dependencies=["
 pex_binary(name="generate_github_workflows", entry_point="generate_github_workflows.py")
 pex_binary(name="generate_user_list", entry_point="generate_user_list.py", dependencies=[":user_list_templates"])
 pex_binary(name="release_helper", entry_point="_release_helper.py")
-pex_binary(name="reversion", entry_point="reversion.py")
+pex_binary(name="reversion", entry_point="reversion.py", always_write_cache=True)
```

Before:

```
❯ ./pants --no-process-execution-local-cache --no-remote-cache-read --no-pantsd package build-support/bin:
...
23:50:18.32 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
23:50:18.32 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
```

After:

```
❯ ./pants --no-process-execution-local-cache --no-remote-cache-read --no-pantsd package build-support/bin:
...
23:49:14.07 [INFO] Completed: Installing 3rdparty/python/lockfiles/user_reqs.txt
...
```

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

Successfully merging this pull request may close these issues.

2 participants