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

ci: rm the lock file testing #2430

Merged
merged 6 commits into from
Nov 21, 2024
Merged

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Nov 20, 2024

This was not something that turned out to be useful and it is causing enough
friction that it makes it not worth it.

Summary:

  • remove presubmit checks checking lockfile
  • remove the pre-commit hooks for lock file updating
  • remove the note about the lock file
  • remove the MODULE.bazel.lock

@fmeum
Copy link
Contributor

fmeum commented Nov 20, 2024

@aignas Anything you learned from these checks that we could use to improve the lockfile's usability?

@aignas
Copy link
Collaborator Author

aignas commented Nov 20, 2024

The transitiveDependencies entry for the transitive bzl file hash in the lock file was changing quite often, so that means that GH would constantly tell that the users need to resolve merge conflicts.

Having this is really useful to understand the impact that a change in the bzlmod extension may yield and it is also a good reminder to push as many things to build-time macros instead of having things in the repository rule BUILD.bazel generation time or extension evaluation time.

At the same time, when rules_rust got introduced as a transitive dependency, because they have the arch/os dependent lock file generation on, then it becomes even more painful to keep things up to date.

In general I think the arch/os dependent lock file flag usage is really painful and we should ask users to try to avoid it as much as possible.

@rickeylev rickeylev added this pull request to the merge queue Nov 21, 2024
Merged via the queue into bazelbuild:main with commit fe3f7a4 Nov 21, 2024
4 checks passed
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