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

[internal] Cache native client binary in CI #12355

Merged
merged 4 commits into from
Jul 16, 2021

Conversation

Eric-Arellano
Copy link
Contributor

Without this, we end up re-compiling .pants in every CI job, including the lint and test jobs.

Comment on lines -54 to -63
# We expose a safety valve to skip compilation iff the user already has `native_engine.so`. This
# can result in using a stale `native_engine.so`, but we trust that the user knows what
# they're doing.
if [[ "${SKIP_NATIVE_ENGINE_SO_BOOTSTRAP}" == "true" ]]; then
if [[ ! -f "${NATIVE_ENGINE_RESOURCE}" || ! -f "${NATIVE_ENGINE_RESOURCE_PYO3}" ]]; then
die "You requested to override bootstrapping native_engine.so and native_engine_pyo3.so via the env var" \
"SKIP_NATIVE_ENGINE_SO_BOOTSTRAP, but the files do not exist at" \
"${NATIVE_ENGINE_RESOURCE} and ${NATIVE_ENGINE_BINARY_PYO3}. This is not safe to do."
fi
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unused now that we don't have Travis.

Comment on lines -72 to -75
if [[ ! -f "${NATIVE_ENGINE_RESOURCE}" || ! -f \
"${NATIVE_ENGINE_RESOURCE_PYO3}" || ! -f \
"${NATIVE_CLIENT_PATH}" || \
"${engine_version_calculated}" != "${engine_version_in_metadata}" ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inverted the conditional for less nesting.

@Eric-Arellano Eric-Arellano requested review from stuhood and benjyw July 16, 2021 20:11
@@ -132,7 +134,9 @@ jobs:
uses: actions/upload-artifact@v2
with:
name: native_engine.so.${{ matrix.python-version }}.${{ runner.os }}
Copy link
Contributor

Choose a reason for hiding this comment

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

The name is no longer entirely accurate. Maybe should be native_code.${...} (if you change the name on upload you have to change it correspondingly on download).

[ci skip-rust]
[ci skip-build-wheels]
@Eric-Arellano Eric-Arellano merged commit 70cc954 into pantsbuild:main Jul 16, 2021
@Eric-Arellano Eric-Arellano deleted the ci-caching branch July 16, 2021 21:49
@wisechengyi wisechengyi mentioned this pull request Jul 17, 2021
wisechengyi added a commit that referenced this pull request Jul 17, 2021
### Internal

* [internal] Manually fix Black lockfile to handle interpreter constraints ([#12366](#12366))
* Revert "Prefix the entire setup.py chroot. (#12359)" ([#12370](#12370))
* [internal] Cache native client binary in CI ([#12355](#12355))
* Prefix the entire setup.py chroot. ([#12359](#12359))
* [internal] Fix AWS CLI breaking due to Python 2 usage ([#12364](#12364))
* [Internal] Add `git_url()` helper to `docutil.py` ([#12352](#12352))
* [Internal] Refactor how `PythonToolBase` exposes requirements and interpreter constraints ([#12356](#12356))
* [internal] Add experimental per-tool lockfiles with Docformatter as an example ([#12346](#12346))
* Remove Pants's dpeendency on `requests` ([#12348](#12348))
* [internal] Remove `TwoStepPex` abstraction ([#12343](#12343))
* Add psycopg2-binary to default module mapping ([#12339](#12339))
* [internal] Upgrade toolchain pants plugin to 0.13.1 ([#12338](#12338))
* Add an API to coarsen/partition Targets by their cycles ([#12251](#12251))
* Prepare 2.5.1 ([#12329](#12329))
* Bootstrap fewer JVM versions in Coursier/javac tests to hopefully reduce CI flakiness ([#12325](#12325))
* Native client respects `--concurrent`. ([#12324](#12324))
* Add client lib tests. ([#12322](#12322))
* Special case enum option parse failures. ([#12281](#12281))
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