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

Do not persist MODULE.bazel.lock #145

Merged
merged 2 commits into from
Mar 29, 2024
Merged

Do not persist MODULE.bazel.lock #145

merged 2 commits into from
Mar 29, 2024

Conversation

mmorel-35
Copy link
Contributor

MODULE.bazel.lock is platform dependent so it’s better not to persist it

Signed-off-by: Matthieu MOREL [email protected]

@ejona86 ejona86 requested a review from veblush March 4, 2024 15:44
@mmorel-35
Copy link
Contributor Author

mmorel-35 commented Mar 14, 2024

Anything blocking this ?

@mmorel-35
Copy link
Contributor Author

Another thing, to publish to BCR, there is a bit of configuration, see https://github.com/bazelbuild/bazel-central-registry/blob/main/docs/README.md and also an App, see https://github.com/apps/publish-to-bcr

@ejona86
Copy link
Member

ejona86 commented Mar 14, 2024

@veblush, could you review?

@mmorel-35, we have never done releases of this repo, so there will be things to figure out

@mmorel-35
Copy link
Contributor Author

It seems like it is better to persist it, see https://bazel.build/external/lockfile#best-practices

@mmorel-35 mmorel-35 closed this Mar 24, 2024
@mmorel-35 mmorel-35 deleted the bzlmod branch March 24, 2024 15:56
@ejona86
Copy link
Member

ejona86 commented Mar 25, 2024

Use bazelisk to run Bazel, and include a .bazelversion file in version control that specifies the Bazel version corresponding to the lockfile.

Yeah, we're not doing that. So I think we need to delete the lock file.

@ejona86
Copy link
Member

ejona86 commented Mar 25, 2024

Since this project needs to run with multiple different Bazel versions, it'd also be important to know whether the lock file detects it was used by a different Bazel version.

I also wonder if the lock file could be used for a supply-chain attack; if Bazel trusts the lock file and ignores the files we review, someone could generate a real lock file but then modify it in subtle ways. The file is too large for such a thing to be noticed on human review. That would definitely have been possible for maven_install; I don't know for bzlmod though.

@ejona86
Copy link
Member

ejona86 commented Mar 25, 2024

That would definitely have been possible for maven_install; I don't know for bzlmod though.

This could be detected by a CI, since it seems the output is expected to be stable (generating it twice generates the same bytes).

@mmorel-35 mmorel-35 restored the bzlmod branch March 25, 2024 16:15
@mmorel-35 mmorel-35 reopened this Mar 25, 2024
@ejona86
Copy link
Member

ejona86 commented Mar 25, 2024

@veblush, can you take a look?

@veblush
Copy link
Contributor

veblush commented Mar 29, 2024

LGTM. I'd like to note that bazel.lock file is contained within this repo and shouldn't affect the downstream.

@ejona86 ejona86 merged commit acfb637 into grpc:master Mar 29, 2024
9 checks passed
@mmorel-35 mmorel-35 deleted the bzlmod branch June 17, 2024 21:36
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.

3 participants