Skip to content

Commit

Permalink
[build] Use bzlmod for development and testing
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jwnimmer-tri committed Dec 12, 2024
1 parent dac7c79 commit a5e94c2
Show file tree
Hide file tree
Showing 14 changed files with 101 additions and 122 deletions.
1 change: 0 additions & 1 deletion BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ exports_files([
exports_files(
[
"MODULE.bazel",
"WORKSPACE",
"WORKSPACE.bzlmod",
],
visibility = ["//tools/workspace:__pkg__"],
Expand Down
9 changes: 2 additions & 7 deletions MODULE.bazel
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
# This file marks a workspace root for the Bazel build system.
# See `https://bazel.build/`.

# This file lists Drake's external dependencies as known to bzlmod.
#
# When bzlmod is disabled, this file is NOT used. Instead, only WORKSPACE is
# used.
#
# When bzlmod is enabled, this file + WORKSPACE.bzlmod are both used, and
# WORKSPACE is ignored.
# This file lists Drake's external dependencies as known to bzlmod. It is used
# in concert with WORKSPACE.bzlmod (which has the workspace-style externals).

module(name = "drake")

Expand Down
39 changes: 6 additions & 33 deletions WORKSPACE
Original file line number Diff line number Diff line change
@@ -1,36 +1,9 @@
# This file marks a workspace root for the Bazel build system.
# See `https://bazel.build/`.
#
# When bzlmod is disabled, only this file is used. The related files
# MODULE.bazel and WORKSPACE.bzlmod are NOT used.
#
# When bzlmod is enabled, this file is ignored.

workspace(name = "drake")

load("//tools/workspace:default.bzl", "add_default_workspace")

add_default_workspace(bzlmod = False)

load("@build_bazel_apple_support//crosstool:setup.bzl", "apple_cc_configure")

apple_cc_configure()

# Add some special heuristic logic for using CLion with Drake.
load("//tools/clion:repository.bzl", "drake_clion_environment")

drake_clion_environment()

load("@bazel_skylib//lib:versions.bzl", "versions")

# This needs to be in WORKSPACE or a repository rule for native.bazel_version
# to actually be defined. The minimum_bazel_version value should match the
# version passed to the find_package(Bazel) call in the root CMakeLists.txt.
versions.check(minimum_bazel_version = "7.1")

# The cargo_universe programs are only used by Drake's new_release tooling, not
# by any compilation rules. As such, we can put it directly into the WORKSPACE
# instead of into our `//tools/workspace:default.bzl` repositories.
load("@rules_rust//crate_universe:repositories.bzl", "crate_universe_dependencies") # noqa
# Building Drake directly (i.e., not as a dependency of a larger project)
# with bzlmod disabled is no longer supported
#
# Consuming Drake as a WORKSPACE dependency is still supported:
# https://github.com/RobotLocomotion/drake-external-examples/tree/main/drake_bazel_external_legacy

crate_universe_dependencies(bootstrap = True)
fail("First-party Drake builds require that bzlmod is enabled.")
16 changes: 2 additions & 14 deletions WORKSPACE.bzlmod
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
# -*- bazel -*-
#
# This file lists Drake's external dependencies as known to bzlmod.
#
# When bzlmod is disabled, this file is NOT used. Instead, only WORKSPACE is
# used.
#
# When bzlmod is enabled, this file + MODULE.bazel are both used, and WORKSPACE
# is ignored.
# This file lists Drake's workspace-style external dependencies. It is used in
# concert with MODULE.bazel (which has the module-style externals).

workspace(name = "drake")

Expand All @@ -25,10 +20,3 @@ load("@bazel_skylib//lib:versions.bzl", "versions")
# to actually be defined. The minimum_bazel_version value should match the
# version passed to the find_package(Bazel) call in the root CMakeLists.txt.
versions.check(minimum_bazel_version = "7.1")

# The cargo_universe programs are only used by Drake's new_release tooling, not
# by any compilation rules. As such, we can put it directly into the WORKSPACE
# instead of into our `//tools/workspace:default.bzl` repositories.
load("@rules_rust//crate_universe:repositories.bzl", "crate_universe_dependencies") # noqa

crate_universe_dependencies(bootstrap = True)
5 changes: 0 additions & 5 deletions cmake/bazel.rc.in
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@ startup --output_base="@BAZEL_OUTPUT_BASE@"
# Inherit Drake's default options.
@BAZELRC_IMPORT@

# By default Drake (currently) opts-out of bzlmod, but for CMake builds we want
# to enable it as a mechanism for covering our bzlmod changes in CI, and also
# to be forward-looking since bzlmod is all Bazel >= 9 will support.
common --enable_bzlmod=true

# Environment variables to be used in repository rules (if any).
common @BAZEL_REPO_ENV@

Expand Down
64 changes: 58 additions & 6 deletions common/find_runfiles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,47 @@

#include <filesystem>

#include "tools/cpp/runfiles/runfiles.h"
#include <fmt/format.h>

#include "drake/common/drake_assert.h"
#include "drake/common/never_destroyed.h"
#include "drake/common/text_logging.h"

// Bazel's API for `class Runfiles` is not sufficient to meet our needs:
// https://github.com/bazelbuild/rules_cc/issues/288
//
// Until that's solved, we need to find some work-around. As best we can tell,
// other than re-implementing the entire library (and its hacky parsers) from
// scratch the only option seems to be to mutate one of its private member
// fields, and the only way to do that is to nerf C++ access control with the
// big hammer: `#define private public`.
//
// Due to the hacky nature of this work-around, `#include ".../runfiles.h"` must
// be the last thing we include. The additional C++ stdlib includes that precede
// it here match the includes that runfiles.h itself mentions, so that the only
// code affected by the `#define ...` will be in runfiles.h, not the stdlib.
//
// clang-format off
#include <functional>
#include <map>
#include <memory>
#include <string>
#include <vector>
#define private public
#include "tools/cpp/runfiles/runfiles.h"
#undef private
// clang-format on

using bazel::tools::cpp::runfiles::Runfiles;

namespace drake {
namespace {

namespace fs = std::filesystem;

// This macro is defined for us by @bazel_tools//tools/cpp/runfiles.
constexpr char kBazelCurrentRepository[] = BAZEL_CURRENT_REPOSITORY;

// Replace `nullptr` with `"nullptr",` or else just return `arg` unchanged.
const char* nullable_to_string(const char* arg) {
return arg ? arg : "nullptr";
Expand All @@ -36,6 +63,9 @@ struct RunfilesSingleton {
std::unique_ptr<Runfiles> runfiles;
// This is the RUNFILES_DIR of `runfiles`; never empty.
std::string runfiles_dir;
// This is the same as `runfiles` but without using the manifest, rather
// only the RUNFILES_DIR.
std::unique_ptr<Runfiles> runfiles_unchecked;
};

// Our singleton latch-initialization logic might fail. This type is akin to
Expand Down Expand Up @@ -121,14 +151,25 @@ RunfilesSingletonOrError Create() {
}
}
if (singleton.runfiles_dir.empty()) {
// This should be effectively unreachable code, but if something went
// terribly wrong internally to Bazel, maybe it could happen.
return wrap_error("RUNFILES_DIR was not provided by the Runfiles object");
}

// We found the runfiles_dir; create the second Runfiles object using it and
// then delete its runfiles manfest in order to force directory-based lookup.
singleton.runfiles_unchecked.reset(Runfiles::Create(
/* argv0 = */ {}, /* runfiles_manifest_file = */ {},
singleton.runfiles_dir, &bazel_error));
if (singleton.runfiles_unchecked == nullptr) {
return wrap_error(bazel_error);
}
const_cast<std::map<std::string, std::string>&>(
singleton.runfiles_unchecked->runfiles_map_)
.clear();

// Sanity check our return value.
DRAKE_DEMAND(singleton.runfiles != nullptr);
DRAKE_DEMAND(!singleton.runfiles_dir.empty());
DRAKE_DEMAND(singleton.runfiles_unchecked != nullptr);

return singleton;
}
Expand All @@ -148,7 +189,8 @@ bool HasRunfiles() {
return std::holds_alternative<RunfilesSingleton>(maybe);
}

RlocationOrError FindRunfile(const std::string& resource_path) {
RlocationOrError FindRunfile(const std::string& resource_path,
const std::string& source_repository) {
const RunfilesSingletonOrError& singleton_or_error =
GetRunfilesSingletonOrError();

Expand All @@ -172,9 +214,19 @@ RlocationOrError FindRunfile(const std::string& resource_path) {
return result;
}

// Map our FindRunfile argument onto the equivalent Rlocation argument.
std::string rlocation_source_repository;
if (resource_path.starts_with("drake/")) {
rlocation_source_repository = kBazelCurrentRepository;
} else {
rlocation_source_repository = source_repository;
}

// Locate the file on the manifest and in the directory.
const std::string by_man = singleton.runfiles->Rlocation(resource_path);
const std::string by_dir = singleton.runfiles_dir + "/" + resource_path;
const std::string by_man =
singleton.runfiles->Rlocation(resource_path, rlocation_source_repository);
const std::string by_dir = singleton.runfiles_unchecked->Rlocation(
resource_path, rlocation_source_repository);
const bool by_man_ok = fs::is_regular_file({by_man});
const bool by_dir_ok = fs::is_regular_file({by_dir});
drake::log()->debug(
Expand Down
16 changes: 10 additions & 6 deletions common/find_runfiles.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

/** @file
This file contains helpers to work with Bazel-declared runfiles -- declared
data dependencies used by C++ code. The functions in this file only succeed
data dependencies used by C++ code. The functions in this file only succeed
when used within a Bazel build.
All source code within Drake should use FindResource() or FindResourceOrThrow()
Expand Down Expand Up @@ -34,10 +34,14 @@ struct RlocationOrError {
std::string error;
};

/** (Advanced.) Returns the absolute path to the given resource_path from Bazel
runfiles, or else an error message when not found. When HasRunfiles() is
false, returns an error. The `resource_path` looks like
`workspace/pkg/subpkg/file.ext`, e.g., "drake/common/foo.txt". */
RlocationOrError FindRunfile(const std::string& resource_path);
/** (Advanced.) Returns the absolute path to the given `resource_path` from
Bazel runfiles, or else an error message when not found. When HasRunfiles()
is false, returns an error. The `resource_path` looks like
`workspace/pkg/subpkg/file.ext`, e.g., "drake/common/foo.txt". When looking
up a non-Drake runfile, the `source_repository` should be set to the
value of the preprocessor definition `BAZEL_CURRENT_REPOSITORY`. */
RlocationOrError FindRunfile(
const std::string& resource_path,
const std::string& source_repository = {});

} // namespace drake
3 changes: 2 additions & 1 deletion common/find_runfiles_stub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ bool HasRunfiles() {
return false;
}

RlocationOrError FindRunfile(const std::string& resource_path) {
RlocationOrError FindRunfile(const std::string& resource_path,
const std::string& source_repository) {
RlocationOrError result;
result.error = "FindRunfile is stubbed out";
return result;
Expand Down
2 changes: 1 addition & 1 deletion common/test/resource_tool_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,5 @@ def test_help_example_is_correct(self):
example_relpath,
], expected_returncode=0)
absolute_path = output.strip()
self.assertTrue(absolute_path.endswith(example_abspath))
self.assertTrue(absolute_path.endswith("/Pendulum.urdf"))
self.assertTrue(os.path.exists(absolute_path))
21 changes: 16 additions & 5 deletions common/test_utilities/drake_py_unittest_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,23 @@ def main():
runfiles, test_package, _, test_name, = match.groups()
test_basename = test_name + ".py"

# Find the test's source file.
runfiles_test_filenames = [
# With bzlmod disabled.
runfiles + "drake/" + test_package + "test/" + test_basename,
# With bzlmod enabled.
runfiles + "_main/" + test_package + "test/" + test_basename,
]
runfiles_test_filename = None
for x in runfiles_test_filenames:
if os.path.exists(x):
runfiles_test_filename = x
break
if runfiles_test_filename is None:
raise RuntimeError("Could not find {} at any of {}".format(
test_basename, runfiles_test_filenames))

# Check the test's source code for a (misleading) __main__.
runfiles_test_filename = (
runfiles + "drake/" + test_package + "test/" + test_basename)
if not os.path.exists(runfiles_test_filename):
raise RuntimeError("Could not find {} at {}".format(
test_basename, runfiles_test_filename))
realpath_test_filename = os.path.realpath(runfiles_test_filename)
with io.open(realpath_test_filename, "r", encoding="utf8") as infile:
for line in infile.readlines():
Expand Down
5 changes: 1 addition & 4 deletions tools/bazel.rc
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
# Don't use bzlmod yet.
# TODO(jwnimmer-tri) When we enable bzlmod by default here, we should nix the
# redundant setting drake/cmake/bazel.rc.in at the same time.
# TODO(#20731) Stop using WORKSPACE (and WORKSPACE.bzlmod).
common --enable_workspace=true
common --enable_bzlmod=false

# Default to an optimized build.
build -c opt
Expand Down
4 changes: 3 additions & 1 deletion tools/lint/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ def find_all_sources(workspace_name):
with open(manifest, "r") as infile:
lines = infile.readlines()
for one_line in lines:
if not one_line.startswith(workspace_name + "/.bazelproject"):
if not one_line.endswith("/.bazelproject\n"):
continue
_, source_sentinel = one_line.split(" ")
workspace_root = os.path.dirname(os.path.realpath(source_sentinel))
if not workspace_root.endswith("/" + workspace_name):
continue
assert workspace_root.startswith("/"), workspace_root
assert os.path.isdir(workspace_root), workspace_root
break
Expand Down
1 change: 0 additions & 1 deletion tools/workspace/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ drake_py_test(
data = [
":default.bzl",
"//:MODULE.bazel",
"//:WORKSPACE",
"//:WORKSPACE.bzlmod",
"//tools/workspace/bazel_skylib:repository.bzl",
"//tools/workspace/build_bazel_apple_support:repository.bzl",
Expand Down
37 changes: 0 additions & 37 deletions tools/workspace/workspace_bzlmod_sync_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,43 +115,6 @@ def test_default_exclude_sync(self):
self.assertEqual(repo_names, self._parse_workspace_already_provided(
self._read("drake/tools/workspace/default.bzl")))

def _canonicalize_workspace(self, content):
"""Given the contents of WORKSPACE or WORKSPACE.bzlmod, returns a
modified copy that:
- strips away comments and blank lines, and
- fuzzes out the `bzlmod = ...` attribute, and
- strips some known workspace-only content.
"""
needle1 = "add_default_workspace(bzlmod = False)"
needle2 = "add_default_workspace(bzlmod = True)"
replacement = "add_default_workspace(bzlmod = ...)"
result = []
for line in content.splitlines():
if "#" in line:
line, _ = line.split("#", maxsplit=1)
line = line.strip()
if not line:
continue
if "apple_cc_configure" in line:
# The apple_cc_configure function should only be used when
# bzlmod is disabled; as such, it should not be in both files.
continue
if line in (needle1, needle2):
line = replacement
result.append(line)
return "\n".join(result) + "\n"

def test_workspace_copies(self):
"""Checks that our WORKSPACE and WORKSPACE.bzlmod are identical,
modulo comments and the `bzlmod = ...` attribute.
"""
workspace1 = self._canonicalize_workspace(
self._read(f"drake/WORKSPACE"))
workspace2 = self._canonicalize_workspace(
self._read(f"drake/WORKSPACE.bzlmod"))
self.maxDiff = None
self.assertMultiLineEqual(workspace1, workspace2)


assert __name__ == '__main__'
unittest.main()

0 comments on commit a5e94c2

Please sign in to comment.