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] Use bzlmod for development and testing #22269

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jwnimmer-tri
Copy link
Collaborator

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

Towards #20731. See the commit message for details.

Given that there are now two "modes" for how Drake declares its externals (workspace vs bzlmod), it's important to consider how we achieve good CI coverage for both.

(1) As of this PR, Bzlmod mode will be tested in our main CI since it is on by default for all first-party Bazel and CMake builds -- pre-merge jobs will run it, as will Continuous, Nightly, Packaging, Wheel, etc.

(2) Workspace mode will only be tested nightly by the drake_bazel_external_legacy example downstream project. For PRs with acute risk, we can run that project by hand to check for any failures. I anticipate this will be good enough, but if we find lots of bugs leaking through to post-merge failures, we can revisit our CI matrix.


This change is Reviewable

jwnimmer-tri

This comment was marked as outdated.

@jwnimmer-tri jwnimmer-tri force-pushed the bzlmod-migration branch 2 times, most recently from 9be0ffc to db67248 Compare December 10, 2024 17:07
@jwnimmer-tri jwnimmer-tri changed the title [build] Support bzlmod for development and testing [build] Use bzlmod for development and testing Dec 10, 2024
@jwnimmer-tri jwnimmer-tri marked this pull request as ready for review December 10, 2024 19:58
@jwnimmer-tri jwnimmer-tri force-pushed the bzlmod-migration branch 2 times, most recently from b01d0c4 to 6bc63ce Compare December 11, 2024 02:24
@jwnimmer-tri jwnimmer-tri force-pushed the bzlmod-migration branch 2 times, most recently from 06e6f1b to a5e94c2 Compare December 12, 2024 18:04
Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

Alright, this monster is ready to begin feature review. All of the prep PRs ahead of it have landed.

+@rpoyner-tri to for feature review, please. There is no hurry here, sometime later next week is fine.

Reviewed 5 of 6 files at r2, 7 of 13 files at r3, 22 of 23 files at r4, 10 of 10 files at r5, 6 of 6 files at r6, all commit messages.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform), needs at least two assigned reviewers

Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion, LGTM missing from assignee rpoyner-tri(platform), needs at least two assigned reviewers


a discussion (no related file):
Working

For sanity / safety in terms of merge sequencing, I'll block here to allow #22305 to reach master first.

@jwnimmer-tri jwnimmer-tri added release notes: none This pull request should not be mentioned in the release notes and removed release notes: feature This pull request contains a new feature labels Dec 13, 2024
In other words, Drake Developers will now be using bzlmod by default.
(CMake installs were already using it.)

Users who consume Drake as a bazel external must still use workspace
mode (no bzlmod). Even though Drake now uses bzlmod for developers,
it is not yet possible for downstream projects to consume Drake as a
bzlmod module; that remains future work.

The main change required for bzlmod here is that our module name in
runfiles is "_main" now instead of "drake", which mostly affects some
linters and tests that were hard-coded. More generally, there is now a
concept of "repo mapping" where module names in a MODULE.bazel file are
projected to unique names in runfiles, to allow for multiple copies of
modules to co-exist. This means that a runfiles lookup must not only
specify the path it's looking for, but from _whose point of view_ that
path is coming from.

* common: Adjust FindRunfile to know about source_repository. When
  none is given and we're seeking a drake runfile, supply the right
  "point of view" string automatically.

* resource_tool_test: Loosen test condition to be only the file
  basename.

* unittest_main: Adjust our source file scan to allow for either
  the bzlmod or workspace spelling of the test program. (The old
  workspace spelling would only be used by downstream projects.)

* tools: Adjust find_all_sources scan for .bazelproject to check
  against the realpath instead of the runfiles path.
Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-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 7 of 7 files at r7, all commit messages.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform), needs at least two assigned reviewers


a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

For sanity / safety in terms of merge sequencing, I'll block here to allow #22305 to reach master first.

Done and rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low release notes: none This pull request should not be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants