Skip to content

Commit

Permalink
[build] Use clang-format from bzlmod
Browse files Browse the repository at this point in the history
Remove clang-format prereqs from the OS.

This makes it easier to use an identical version number across all of
our supported development OS's, without relying on their various
package managers to remain consistent.
  • Loading branch information
jwnimmer-tri committed Jan 21, 2025
1 parent a22266d commit 5c43b28
Show file tree
Hide file tree
Showing 14 changed files with 59 additions and 59 deletions.
8 changes: 8 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ spdlog_repository = use_repo_rule(

spdlog_repository(name = "pkgconfig_spdlog")

# Add additional modules we use as tools (not runtime dependencies).

bazel_dep(name = "toolchains_llvm", version = "1.2.0")

llvm = use_extension("@toolchains_llvm//toolchain/extensions:llvm.bzl", "llvm")
llvm.toolchain(llvm_version = "15.0.5")
use_repo(llvm, "llvm_toolchain")

# Load dependencies which are "public", i.e., made available to downstream
# projects.
#
Expand Down
11 changes: 4 additions & 7 deletions doc/_pages/clion.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,18 +176,15 @@ CLion such that the modification may not be immediately apparent. When in doubt,
select away from the target file and back; this will cause the file to refresh
and you can confirm that the file has been modified as expected.

First, make sure you have installed ``clang-format-15``
(see [Tools for Code Style Compliance](/code_style_tools.html)).

### Clang format selected file

Open the ``Edit Tool`` for external tools as outlined above and enter the
following values for the fields:

* **Name:** ``Clang Format Full File``
* **Description:** ``Apply clang-format to the active file``
* **Program:** ``clang-format-15``
* **Arguments:** ``-i $FileName$``
* **Program:** ``bazel``
* **Arguments:** ``run //tools/lint:clang-format -- -i $FileName$``
* **Working directory:** ``$FileDir$``
* **Advanced Options:** Uncheck ``Open console for tool output``

Expand All @@ -200,8 +197,8 @@ following values for the fields:

* **Name:** ``Clang Format Selected Lines``
* **Description:** ``Apply clang-format to the selected lines``
* **Program:** ``clang-format-15``
* **Arguments:** ``-lines $SelectionStartLine$:$SelectionEndLine$ -i $FileName$``
* **Program:** ``bazel``
* **Arguments:** ``run //tools/lint:clang-format -- -lines $SelectionStartLine$:$SelectionEndLine$ -i $FileName$``
* **Working directory:** ``$FileDir$``
* **Advanced Options:** Uncheck ``Open console for tool output``

Expand Down
16 changes: 4 additions & 12 deletions doc/_pages/code_style_tools.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,11 @@ User manuals for the style-checking tools are as follows:

## C/C++: Clang-Format

The [Mandatory platform-specific instructions](/from_source.html#mandatory-platform-specific-instructions)
install Drake's required version of ``clang-format``, depending on the platform
(macOS or Ubuntu).

To run ``clang-format``:

```
clang-format-15 -i -style=file [file name]
cd drake
bazel run /tools/lint:clang-format -- -i -style=file [file name]
```

Using ``clang-format`` will modify the entire file that is specified. As an
Expand All @@ -57,17 +54,12 @@ portions of a file that you have modified. To run ``git clang-format``:

```
# For development on Ubuntu: format a file that has been staged in git
git clang-format-15 --binary=/usr/bin/clang-format-15 -- [file name]
git clang-format --binary=/path/to/drake/bazel-bin/tools/lint/clang-format -- [file name]
# For development on Ubuntu: format a file that has been modified but not staged
git clang-format-15 --binary=/usr/bin/clang-format-15 -f -- [file name]
git clang-format --binary=/path/to/drake/bazel-bin/tools/lint/clang-format -f -- [file name]
```

On macOS, the command to use is
``/opt/homebrew/opt/llvm@15/bin/clang-format``.
Note in particular that ``15`` is part of the directory name, not a suffix on
the program name.

### IDE integration

Most IDEs can run ``clang-format`` automatically.
Expand Down
3 changes: 2 additions & 1 deletion doc/_pages/emacs.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ On Ubuntu ``sudo apt install elpa-magit``.

Use ``(require 'clang-format)`` to enable the ``M-x clang-format-...`` family of
functions. Also check that the customize variable ``clang-format-executable`` is
set to Drake's preferred value ``clang-format-15``.
set to Drake's preferred value
``/path/to/drake/bazel-bin/tools/lint/clang-format``.

<!--- TODO(jwnimmer-tri) Explain 'google-c-style from the styleguide. -->
2 changes: 1 addition & 1 deletion doc/_pages/vscode.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ on code formatting work well. Take note of "Format Document",

In the VS Code Options configuration, check that the option for
``C_Cpp: Clang_format_path`` is set to Drake's preferred value
``clang-format-15``.
``/path/to/drake/bazel-bin/tools/lint/clang-format``.
4 changes: 0 additions & 4 deletions setup/mac/source_distribution/Brewfile-test-only

This file was deleted.

11 changes: 1 addition & 10 deletions setup/mac/source_distribution/install_prereqs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,10 @@

set -euxo pipefail

with_test_only=1

while [ "${1:-}" != "" ]; do
case "$1" in
# Do NOT install prerequisites that are only needed to build and/or run
# unit tests, i.e., those prerequisites that are not dependencies of
# bazel { build, run } //:install.
--without-test-only)
with_test_only=0
# Ignored for backwards compatibility.
;;
*)
echo 'Invalid command line argument' >&2
Expand All @@ -35,7 +30,3 @@ if ! command -v brew &>/dev/null; then
fi

brew bundle --file="${BASH_SOURCE%/*}/Brewfile" --no-lock

if [[ "${with_test_only}" -eq 1 ]]; then
brew bundle --file="${BASH_SOURCE%/*}/Brewfile-test-only" --no-lock
fi
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
clang-format-15
curl
kcov
libstdc++6-10-dbg
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
clang-format-15
curl
libstdc++6-10-dbg
python3-dateutil
Expand Down
15 changes: 14 additions & 1 deletion tools/lint/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,22 @@ drake_py_library(
name = "clang_format",
testonly = 1,
srcs = ["clang_format.py"],
data = ["//:.clang-format"],
data = [
"//:.clang-format",
"@llvm_toolchain//:clang-format",
],
deps = [
":module_py",
"@rules_python//python/runfiles",
],
)

drake_py_binary(
name = "clang-format",
testonly = 1,
srcs = ["clang_format.py"],
deps = [
":clang_format",
],
)

Expand Down
41 changes: 21 additions & 20 deletions tools/lint/clang_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,24 @@
"""

import os
import platform


def get_clang_format_path(version=None):
"""Call with version=None to use Drake's default version.
Otherwise, pass the desired major version as an int.
"""
if version is None:
version = 15
if platform.system() == "Darwin":
if platform.machine() == "arm64":
homebrew = "/opt/homebrew"
else:
homebrew = "/usr/local"
path = f"{homebrew}/opt/llvm@{version}/bin/clang-format"
else:
path = f"/usr/bin/clang-format-{version}"
if os.path.isfile(path):
return path
raise RuntimeError("Could not find required clang-format at " + path)
from pathlib import Path
import sys

from python import runfiles


def get_clang_format_path():
manifest = runfiles.Create()
path = Path(manifest.Rlocation("llvm_toolchain/clang-format"))
if not path.is_file():
raise RuntimeError(f"Could not find required clang-format at {path}")
return path


def _main():
exe = get_clang_format_path()
os.execvp(exe, [exe] + sys.argv[1:])


if __name__ == "__main__":
_main()
3 changes: 2 additions & 1 deletion tools/lint/clang_format_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ def _check_clang_format_idempotence(filename):
if not changes:
return 0
print("ERROR: {} needs clang-format".format(filename))
print("note: fix via {} -style=file -i {}".format(clang_format, filename))
print("note: fix via {} -style=file -i {}".format(
"bazel-bin/tools/lint/clang-format", filename))
return 1


Expand Down
1 change: 1 addition & 0 deletions tools/workspace/default.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,7 @@ REPOS_ALREADY_PROVIDED_BY_BAZEL_MODULES = [
"rules_rust",
"rules_shell",
"spdlog",
"toolchains_llvm",
]

# This is the list of repositories that Drake provides as a module extension
Expand Down
1 change: 1 addition & 0 deletions tools/workspace/workspace_bzlmod_sync_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def test_version_sync(self):

# Don't check modules that are known to be module-only.
del modules["bazel_features"]
del modules["toolchains_llvm"]

# Don't check modules whose repository rule twin is pkgconfig (and thus
# doesn't have a github pinned version that we need to keep in sync).
Expand Down

0 comments on commit 5c43b28

Please sign in to comment.