Skip to content

Cache built Bazel binaries by commit sha.#144

Open
tjgq wants to merge 1 commit into
bazelbuild:masterfrom
tjgq:store-binary-by-commit
Open

Cache built Bazel binaries by commit sha.#144
tjgq wants to merge 1 commit into
bazelbuild:masterfrom
tjgq:store-binary-by-commit

Conversation

@tjgq

@tjgq tjgq commented Sep 20, 2022

Copy link
Copy Markdown
Collaborator

The commit to build Bazel at can be given as a branch or tag name, which may point to different commits over time. This could cause a previously built Bazel binary to be incorrectly reused.

The commit to build Bazel at can be given as a branch or tag name, which may
point to different commits over time. This could cause a previously built
Bazel binary to be incorrectly reused.
@tjgq tjgq requested a review from joeleba September 20, 2022 12:05
@joeleba

joeleba commented Sep 20, 2022

Copy link
Copy Markdown
Member

The commit to build Bazel at can be given as a branch or tag name

From reading the documentation in the code, it certainly wasn't the intended usage. commit was meant for commits only. Same for the --bazel_commits flag.

If you have a use case for using branch or tag name there, I think the proper thing to do is renaming the --bazel_commits flag to --bazel_identifier--bazel_from_source_at, then do what you're doing in this PR (getting the actual commit for caching the binary). That'd allow us to not overload the term "commit".

Comment thread benchmark.py

Args:
commit: the Bazel commit SHA.
commit: the commit, branch or tag to build Bazel at.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In that case, should this parameter be changed to version_identifier or something, instead of commit?

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