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

[build] Disable autoloaded (built-in) bazel rules #22305

Merged

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Dec 12, 2024

Towards #20731.

The autoloading behavior will become deprecated and eventually go away in future bazel versions. This gets out ahead of the problem by opting-out of the autoloading. Note that the opt-out in bazel.rc is not inherited by downstream projects; it is only for Drake.

Loading the specific rules versions also helps make it crystal clear to us which versions of starlark rules we are running. We definitely do not want to be mixing and matching versions in different places. With autoloading turned off, we know for sure what we're running, based on either the MODULE or WORKSPACE file (depending on which mode we're using).


This change is Reviewable

@jwnimmer-tri jwnimmer-tri added priority: low status: single reviewer ok https://drake.mit.edu/reviewable.html release notes: fix This pull request contains fixes (no new features) labels Dec 12, 2024
@jwnimmer-tri jwnimmer-tri force-pushed the bzlmod-no-autoload branch 2 times, most recently from 4762d59 to 8326231 Compare December 13, 2024 15:35
@jwnimmer-tri
Copy link
Collaborator Author

FYI @ggould-tri @rpoyner-tri @xuchenhan-tri this is ready for reviews now, if anyone wants to sign up.

@rpoyner-tri rpoyner-tri self-assigned this Dec 13, 2024
Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

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

+@rpoyner-tri for reviews.

Reviewable status: LGTM missing from assignee rpoyner-tri(platform)

Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

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

Reviewed 29 of 29 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee rpoyner-tri(platform)


tools/skylark/cc.bzl line 5 at r1 (raw file):

For all code built by Drake (both first-party code, and built-from-source
externals), we should be using this file's macros. We should never call
`native.cc_foo()` from anywhere other than this file, and any BUILD file

nit Does this comment need updating, now that we are avoiding native rules for c++ altogether?

Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 1 unresolved discussion

The release-notable changes here are the addition of rules_java and
rules_shell as new dependencies and the bump of the minimum supported
bazel version up to 7.4.

As part of uniform loading of rules we also refactor to provide a single
point of control for shell and java, like we had previously done for cc
and python and tidy up some missing pieces.
@jwnimmer-tri jwnimmer-tri merged commit d819ff2 into RobotLocomotion:master Dec 13, 2024
8 of 10 checks passed
@jwnimmer-tri jwnimmer-tri deleted the bzlmod-no-autoload branch December 13, 2024 23:25
RussTedrake pushed a commit to RussTedrake/drake that referenced this pull request Dec 15, 2024
)

The release-notable changes here are the addition of rules_java and
rules_shell as new dependencies and the bump of the minimum supported
bazel version up to 7.4.

As part of uniform loading of rules we also refactor to provide a single
point of control for shell and java, like we had previously done for cc
and python and tidy up some missing pieces.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low release notes: fix This pull request contains fixes (no new features) status: single reviewer ok https://drake.mit.edu/reviewable.html
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants