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

Run all vcs versioning processes locally #21353

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

benjyw
Copy link
Contributor

@benjyw benjyw commented Aug 25, 2024

When using environments we must ensure we run VCS-related
processes in the local environment and not the inherited one,
as a docker environment will not have access to Git state.

In #21188 we made setuptools_scm itself run locally,
but this was not sufficient: this change makes sure we
also run the pex building and git binary discovery
in the local environment.

Fixes #21178

@benjyw benjyw added the category:internal CI, fixes for not-yet-released features, etc. label Aug 25, 2024
@benjyw benjyw requested a review from tobni August 25, 2024 21:08
@benjyw
Copy link
Contributor Author

benjyw commented Aug 25, 2024

I'm looking into writing a test for this.

Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Thanks for this and explaining the git logic.

Just confirming why we need to build the pex locally too: the pex built in the docker environment might not be compatible with the local environment, e.g. if the host is macOS, and the docker environment is linux. Is that right?

@huonw
Copy link
Contributor

huonw commented Aug 25, 2024

Also, does this need cherrypicking to 2.22.x?

@benjyw
Copy link
Contributor Author

benjyw commented Aug 26, 2024

Just confirming why we need to build the pex locally too: the pex built in the docker environment might not be compatible with the local environment, e.g. if the host is macOS, and the docker environment is linux. Is that right?

Correct - we have to build it where we run it.

@benjyw
Copy link
Contributor Author

benjyw commented Aug 26, 2024

Also, does this need cherrypicking to 2.22.x?

The previous change wasn't. I was treating this as more of a feature than a bug, but that is admittedly debatable. I could CP both of them - yeah, might as well.

@benjyw
Copy link
Contributor Author

benjyw commented Aug 26, 2024

OK, this is the CP for the previous part: #21355

Will then auto-CP this one on top of it.

@benjyw benjyw added this to the 2.22.x milestone Aug 26, 2024
@benjyw benjyw merged commit 6a1a938 into pantsbuild:main Aug 26, 2024
25 checks passed
@benjyw benjyw deleted the vcs_version_local2 branch August 26, 2024 20:48
@WorkerPants
Copy link
Member

I tried to automatically cherry-pick this change back to each relevant milestone, so that it is available in those older releases of Pants.

❌ 2.22.x

I was unable to cherry-pick this PR to 2.22.x, likely due to merge-conflicts.

Steps to Cherry-Pick locally

To resolve:

  1. (Ensure your git working directory is clean)
  2. Run the following script to reproduce the merge-conflicts:
    git fetch https://github.com/pantsbuild/pants main \
      && git fetch https://github.com/pantsbuild/pants 2.22.x \
      && git checkout -b cherry-pick-21353-to-2.22.x FETCH_HEAD \
      && git cherry-pick 6a1a938787b7ef0f9ee2d5b14c436a1f84d5589a
  3. Fix the merge conflicts and commit the changes
  4. Run build-support/cherry_pick/make_pr.sh "21353" "2.22.x"

Please note that I cannot re-run CI if a job fails. Please work with your PR approver(s) to re-run CI if necessary.


When you're done manually cherry-picking, please remove the needs-cherrypick label on this PR.

Thanks again for your contributions!

🤖 Beep Boop here's my run link

@WorkerPants WorkerPants added the auto-cherry-picking-failed Auto Cherry-Picking Failed label Aug 26, 2024
benjyw added a commit to benjyw/pants that referenced this pull request Aug 26, 2024
When using environments we must ensure we run VCS-related
processes in the local environment and not the inherited one,
as a docker environment will not have access to Git state.

In pantsbuild#21188 we made setuptools_scm itself run locally,
but this was not sufficient: this change makes sure we
also run the pex building and git binary discovery
in the local environment.

Fixes pantsbuild#21178
benjyw added a commit that referenced this pull request Aug 26, 2024
When using environments we must ensure we run VCS-related processes
in the local environment and not the inherited one, as a docker
environment will not have access to Git state.

In #21188 we made setuptools_scm itself run locally, but this was not
sufficient: this change makes sure we also run the pex building and
git binary discovery in the local environment.

Fixes #21178
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-cherry-picking-failed Auto Cherry-Picking Failed category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vcs_version fails when building pex binary in a docker environment
4 participants