[build] Pass a more specific compiler from CMake to Bazel #23117

tyler-yankee · 2025-06-20T13:01:54Z

Add a Bazel flag to opt in to tweaking compiler-specific flags, so that CMake can use it when checking against CMAKE_<LANG>_COMPILER_ID. This allows Linux CMake users to inherit Bazel's cc_toolchain logic without having to specify CMAKE_<LANG>_COMPILER (notably, when the compiler is 'cc'/'c++').

This change is

tyler-yankee

Reviewable status: 2 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 324 at r1 (raw file):

    //tools/cc_toolchain:print_compiler_config)

  execute_process(COMMAND ${Bazel_EXECUTABLE} ${COMPILER_CONFIG_ARGS}

Working this call breaks if you haven't yet run Drake's install_prereqs_user_environment.sh, which happens when you consume Drake as a CMake external.

CMakeLists.txt line 325 at r1 (raw file):

  execute_process(COMMAND ${Bazel_EXECUTABLE} ${COMPILER_CONFIG_ARGS}
    WORKING_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}"

Working CMAKE_CURRENT_SOURCE_DIR might be cleaner, but this should be fine too.

tyler-yankee · 2025-06-24T16:40:42Z

CMakeLists.txt line 324 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Working this call breaks if you haven't yet run Drake's install_prereqs_user_environment.sh, which happens when you consume Drake as a CMake external.

@jwnimmer-tri Do you have thoughts here? I'm using bazel run --config={gcc|clang} //tools/cc_toolchain:print_compiler_config (as CI does) when the compiler is cc/c++ to fetch the appropriate gcc or clang version.

Having CMake call the user env prereq script seems wrong, but so does not using the Bazel tool we already have.

jwnimmer-tri

Reviewable status: 3 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 324 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

@jwnimmer-tri Do you have thoughts here? I'm using bazel run --config={gcc|clang} //tools/cc_toolchain:print_compiler_config (as CI does) when the compiler is cc/c++ to fetch the appropriate gcc or clang version.

Having CMake call the user env prereq script seems wrong, but so does _not _ using the Bazel tool we already have.

Because (at least on "NOT APPLE") we are going to hard-code the environment variables below ...

    " --repo_env=CC=${CMAKE_C_COMPILER}"
    " --repo_env=CXX=${CMAKE_CXX_COMPILER}"

... it doesn't help anything to call print_compiler_config. All that program does it regurgitate the environment variables.

CMakeLists.txt line 291 at r1 (raw file):

endif()

set_property(GLOBAL PROPERTY BAZEL_REPO_ENV)

BTW Do we really want this? We have drake/cmake/bazel.rc.in at our fingertops. Maybe it would be simpler to have e.g. common --repo_env=CXX=@CMAKE_CXX_COMPILER@ in that file? Or common --repo_env=CXX=@DRAKE_CXX_COMPILER@ in case we need to tweak it to be slightly different that CMake's default.

tyler-yankee

Reviewable status: 3 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes

CMakeLists.txt line 291 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

BTW Do we really want this? We have drake/cmake/bazel.rc.in at our fingertops. Maybe it would be simpler to have e.g. common --repo_env=CXX=@CMAKE_CXX_COMPILER@ in that file? Or common --repo_env=CXX=@DRAKE_CXX_COMPILER@ in case we need to tweak it to be slightly different that CMake's default.

That's fair. My thought was that we currently keep all of our common Bazel flags in BAZEL_REPO_ENV in this file, so doing it this way keeps it consistent.

CMakeLists.txt line 324 at r1 (raw file):

All that program does it regurgitate ...

Not quite? E.g. It turns --config=clang into /usr/bin/clang-15 tagging on that version and finding the path.

jwnimmer-tri

Reviewable status: 2 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 291 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

That's fair. My thought was that we currently keep all of our common Bazel flags in BAZEL_REPO_ENV in this file, so doing it this way keeps it consistent.

Ah. I guess anything that's conditional can't be done in the way I propose, and we don't set CXX on APPLE, so maybe we can't make this change anyway.

CMakeLists.txt line 324 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

All that program does it regurgitate ...

Not quite? E.g. It turns --config=clang into /usr/bin/clang-15 tagging on that version and finding the path.

Sure, for bazel users it does that. CMake users don't say --config=clang, so for the purposes of this file that doesn't matter.

tyler-yankee

Reviewable status: 2 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes

CMakeLists.txt line 324 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Sure, for bazel users it does that. CMake users don't say --config=clang, so for the purposes of this file that doesn't matter.

Would we not support the combo of clang/CMake/Linux with cc pointing to clang?

jwnimmer-tri

Reviewable status: 2 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 324 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Would we not support the combo of clang/CMake/Linux with cc pointing to clang?

Yes, CMake users can use Clang. They don't pass --config=clang to do that. The --config=clang is a shortcut for Drake developers to opt-in to our CI-pinned default version of clang by setting the environment variables:

drake/tools/ubuntu-jammy.bazelrc

Lines 3 to 8 in 9752aff

    
           common:clang --repo_env=CC=clang-15 
        
           common:clang --repo_env=CXX=clang++-15 
        
           build:clang --action_env=CC=clang-15 
        
           build:clang --action_env=CXX=clang++-15 
        
           build:clang --host_action_env=CC=clang-15 
        
           build:clang --host_action_env=CXX=clang++-15

(It is not some Bazel upstream flag.)

tyler-yankee · 2025-06-24T17:17:37Z

CMakeLists.txt line 324 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Yes, CMake users can use Clang. They don't pass --config=clang to do that. The --config=clang is a shortcut for Drake developers to opt-in to our CI-pinned default version of clang by setting the environment variables:

drake/tools/ubuntu-jammy.bazelrc

Lines 3 to 8 in 9752aff

common:clang --repo_env=CC=clang-15

common:clang --repo_env=CXX=clang++-15

build:clang --action_env=CC=clang-15

build:clang --action_env=CXX=clang++-15

build:clang --host_action_env=CC=clang-15

build:clang --host_action_env=CXX=clang++-15

(It is not some Bazel upstream flag.)

Okay, that makes sense. So this script needs its own logic to determine the version of clang pointed to by /usr/bin/cc, likewise for gcc, and it need not depend on //tools/cc_toolchain/print_compiler_config.

jwnimmer-tri

Reviewable status: 2 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 324 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Okay, that makes sense. So this script needs its own logic to determine the version of clang pointed to by /usr/bin/cc, likewise for gcc, and it need not depend on //tools/cc_toolchain/print_compiler_config.

I think CMake has it built-in? At least, until recently, Drake had a C program based on the CMake logic:

https://github.com/RobotLocomotion/drake/blob/3704a57b329c7a56c730ed36031b6a6fc71b5620/tools/workspace/cc/identify_compiler.cc

tyler-yankee

Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 324 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I think CMake has it built-in? At least, until recently, Drake had a C program based on the CMake logic:

https://github.com/RobotLocomotion/drake/blob/3704a57b329c7a56c730ed36031b6a6fc71b5620/tools/workspace/cc/identify_compiler.cc

I'm not entirely sure what feature/module(?) you're referring to in CMake, but I don't know of anything that is able to parse cc/c++ other than CMAKE_<LANG>_COMPILER_ID, which is already part of the logic I'm using (and is already being used in Drake's CMake). My new proposal based on which <compiler>-<version> with a fallback to cc/c++ seems probably sufficient.

jwnimmer-tri

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 324 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

I'm not entirely sure what feature/module(?) you're referring to in CMake, but I don't know of anything that is able to parse cc/c++ other than CMAKE_<LANG>_COMPILER_ID, which is already part of the logic I'm using (and is already being used in Drake's CMake). My new proposal based on which <compiler>-<version> with a fallback to cc/c++ seems probably sufficient.

(I'll open a new discussion.)

CMakeLists.txt line 311 at r2 (raw file):

  if(CMAKE_${lang}_COMPILER_ID STREQUAL GNU)
    if(lang STREQUAL "C")
      set(compiler "gcc")

No, this is totally wrong. The user might have passed CXX=gcc-17 which now we've broken and use CXX=gcc instead.

I can see two valid solution paths:

(1) Fix "@rules_cc//cc/compiler:gcc" to actually work (by shelling out to the compiler and checking it it's GCC). I haven't looked how easy or hard this would be.

(2) In this file, ask CMake if we're GCC ("STREQUAL GNU") and if yes, then (2) set a magic drake-specific bazel flag that says "it's really gcc, no fooling" that we "or" with "@rules_cc//cc/compiler:gcc" at the point(s) of use so that if either rule_cc or CMake detects GCC, that we will enable the GCC-specific compiler flags.

jwnimmer-tri

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 311 at r2 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

No, this is totally wrong. The user might have passed CXX=gcc-17 which now we've broken and use CXX=gcc instead.

I can see two valid solution paths:

(1) Fix "@rules_cc//cc/compiler:gcc" to actually work (by shelling out to the compiler and checking it it's GCC). I haven't looked how easy or hard this would be.

(2) In this file, ask CMake if we're GCC ("STREQUAL GNU") and if yes, then (2) set a magic drake-specific bazel flag that says "it's really gcc, no fooling" that we "or" with "@rules_cc//cc/compiler:gcc" at the point(s) of use so that if either rule_cc or CMake detects GCC, that we will enable the GCC-specific compiler flags.

Or maybe it's only GCC=gcc=17.1 or GCC=/not/on/the/path/gcc-17 that's broken?

In any case, the invariant is that whatever exact path the user told CMake to use, we must pass that unchanged down to Bazel. We must never set it to something else.

tyler-yankee

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 311 at r2 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Or maybe it's only GCC=gcc=17.1 or GCC=/not/on/the/path/gcc-17 that's broken?

In any case, the invariant is that whatever exact path the user told CMake to use, we must pass that unchanged down to Bazel. We must never set it to something else.

Yeah, I incorporated the major version below. This function is only called when the compiler is cc; otherwise we implement the existing logic to pass CMAKE_<LANG>_COMPILER directly to Bazel. Perhaps the code could be restructured to make that more clear, but I am trying to make this change narrowly-scoped. If I'm not mistaken, if you call cmake -DCMAKE_C_COMPILER=/path/to/gcc (which is being passed directly to Bazel), that Bazel will already work with that.

jwnimmer-tri

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 311 at r2 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Yeah, I incorporated the major version below. This function is only called when the compiler is cc; otherwise we implement the existing logic to pass CMAKE_<LANG>_COMPILER directly to Bazel. Perhaps the code could be restructured to make that more clear, but I am trying to make this change narrowly-scoped. If I'm not mistaken, if you call cmake -DCMAKE_C_COMPILER=/path/to/gcc (which is being passed directly to Bazel), that Bazel will already work with that.

Whatever other circumstances exist, we must ensure that we end up with ...

    " --repo_env=CC=${CMAKE_C_COMPILER}"
    " --repo_env=CXX=${CMAKE_CXX_COMPILER}"

... set exactly that way, never changed to anything that is "probably" the same.

It is not good that /non/path/bin/cc could be automatically rewritten to /usr/bin/gcc-15. What if /non/path/bin/cc was a custom build of GCC 15 that was not added to the path?

tyler-yankee · 2025-06-30T18:51:58Z

CMakeLists.txt line 311 at r2 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Whatever other circumstances exist, we must ensure that we end up with ...
    " --repo_env=CC=${CMAKE_C_COMPILER}"
    " --repo_env=CXX=${CMAKE_CXX_COMPILER}"
... set exactly that way, never changed to anything that is "probably" the same.

It is not good that /non/path/bin/cc could be automatically rewritten to /usr/bin/gcc-15. What if /non/path/bin/cc was a custom build of GCC 15 that was not added to the path?

ping @jwnimmer-tri from f2f: What is the best way to add a Bazel flag here?

The compiler version is already provided by @drake//tools/cc\_toolchain:compiler\_major, so all we need is probably two booleans, for "really gcc" and "really clang" (though those would need to be mutually exclusive....). Not sure if a string is less clunky?

jwnimmer-tri

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

CMakeLists.txt line 311 at r2 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

ping @jwnimmer-tri from f2f: What is the best way to add a Bazel flag here?

The compiler version is already provided by @drake//tools/cc\_toolchain:compiler\_major, so all we need is probably two booleans, for "really gcc" and "really clang" (though those would need to be mutually exclusive....). Not sure if a string is less clunky?

Ah yes, that cc_toolchain/BUILD.bazel file is a great place for this flag.

I don't have an intuition for whether string_flag or bool_flag is going to be most natural. I'm OK if you just play with it and pick the one you like best.

jwnimmer-tri

feature.

Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: 3 unresolved discussions, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

tools/cc_toolchain/BUILD.bazel line 24 at r3 (raw file):

)

# In our developer builds and CMake installs, we use the following flags as

nit Make it epsilon more clear exactly which parts of the BUILD file this paragraph is covering.

Suggestion:

following two flags

tools/cc_toolchain/BUILD.bazel line 33 at r3 (raw file):

# TODO(jwnimmer-tri) Ideally, rules_cc would provide either or both of these
# as toolchain attributes that we could query, but it doesn't seem to exist
# there yet.

nit This TODO is slightly imprecise. For the major version number, it doesn't provide it at all so the TODO is okay. However, for the compiler flavor, rules_cc does provide it ("@rules_cc//cc/compiler:gcc") but the implementation apparently only looks at the filename instead of running it to see what's up. We should make that second part more clear here.

CMakeLists.txt line 299 at r3 (raw file):

  )
  # Pass a Bazel flag which enforces the underlying compiler so that
  # compiler- and version-specific logic (e.g. warnings) is enforced in the

typo

Suggestion:

(e.g., warnings)

tyler-yankee

Reviewable status: 2 unresolved discussions, needs at least two assigned reviewers, missing label for release notes (waiting on @tyler-yankee)

tools/cc_toolchain/BUILD.bazel line 33 at r3 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit This TODO is slightly imprecise. For the major version number, it doesn't provide it at all so the TODO is okay. However, for the compiler flavor, rules_cc does provide it ("@rules_cc//cc/compiler:gcc") but the implementation apparently only looks at the filename instead of running it to see what's up. We should make that second part more clear here.

Good clarification, thanks. Let me know how you like the new language.

tools/skylark/pybind.bzl line 8 at r4 (raw file):

EXTRA_PYBIND_COPTS = select({
    "@drake//tools/cc_toolchain:clang": [

I missed this on the first go-around, and then DEE's CI failed. I believe this is correct now.

tyler-yankee · 2025-07-02T13:10:14Z

DEE CI with -DCMAKE_C_COMPILER=/usr/bin/gcc (likewise for g++) removed and warnings gone: https://drake-jenkins.csail.mit.edu/view/External%20Examples/job/linux-noble-unprovisioned-external-examples/8/

jwnimmer-tri

+@ggould-tri for platform review per schedule, please (after the holiday).

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: LGTM missing from assignee ggould-tri(platform)

Add a Bazel flag to opt in to tweaking compiler-specific flags, so that CMake can use it when checking against CMAKE_<LANG>_COMPILER_ID. This allows Linux CMake users to inherit Bazel's cc_toolchain logic without having to specify CMAKE_<LANG>_COMPILER (notably, when the compiler is 'cc'/'c++').

tyler-yankee added the status: do not review label Jun 20, 2025

tyler-yankee force-pushed the cmake-compiler branch 3 times, most recently from 35f7e53 to 58beb50 Compare June 24, 2025 16:15

tyler-yankee commented Jun 24, 2025

View reviewed changes

jwnimmer-tri reviewed Jun 24, 2025

View reviewed changes

tyler-yankee commented Jun 24, 2025

View reviewed changes

jwnimmer-tri reviewed Jun 24, 2025

View reviewed changes

tyler-yankee commented Jun 24, 2025

View reviewed changes

jwnimmer-tri reviewed Jun 24, 2025

View reviewed changes

tyler-yankee force-pushed the cmake-compiler branch from 58beb50 to af89e92 Compare June 24, 2025 19:26

tyler-yankee commented Jun 24, 2025

View reviewed changes

jwnimmer-tri reviewed Jun 24, 2025

View reviewed changes

tyler-yankee commented Jun 24, 2025

View reviewed changes

jwnimmer-tri reviewed Jun 24, 2025

View reviewed changes

jwnimmer-tri reviewed Jun 30, 2025

View reviewed changes

tyler-yankee force-pushed the cmake-compiler branch from af89e92 to cd9f2e3 Compare June 30, 2025 20:35

tyler-yankee removed the status: do not review label Jun 30, 2025

tyler-yankee marked this pull request as ready for review June 30, 2025 20:36

jwnimmer-tri self-assigned this Jun 30, 2025

tyler-yankee force-pushed the cmake-compiler branch from cd9f2e3 to ef080c3 Compare July 1, 2025 12:58

tyler-yankee mentioned this pull request Jul 1, 2025

[drake_cmake_external] Remove explicit gcc in CI RobotLocomotion/drake-external-examples#415

Open

jwnimmer-tri reviewed Jul 1, 2025

View reviewed changes

tyler-yankee force-pushed the cmake-compiler branch from ef080c3 to aa0912d Compare July 2, 2025 12:14

tyler-yankee commented Jul 2, 2025

View reviewed changes

tyler-yankee added the release notes: fix This pull request contains fixes (no new features) label Jul 2, 2025

jwnimmer-tri assigned ggould-tri Jul 2, 2025

jwnimmer-tri reviewed Jul 2, 2025

View reviewed changes

tyler-yankee force-pushed the cmake-compiler branch from aa0912d to 97465f7 Compare July 2, 2025 14:36

	common:clang --repo_env=CC=clang-15
	common:clang --repo_env=CXX=clang++-15
	build:clang --action_env=CC=clang-15
	build:clang --action_env=CXX=clang++-15
	build:clang --host_action_env=CC=clang-15
	build:clang --host_action_env=CXX=clang++-15

[build] Pass a more specific compiler from CMake to Bazel #23117

Are you sure you want to change the base?

[build] Pass a more specific compiler from CMake to Bazel #23117

Uh oh!

Conversation

tyler-yankee commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tyler-yankee left a comment

Choose a reason for hiding this comment

Uh oh!

tyler-yankee commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

tyler-yankee left a comment

Choose a reason for hiding this comment

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

tyler-yankee left a comment

Choose a reason for hiding this comment

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

tyler-yankee commented Jun 24, 2025

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

tyler-yankee left a comment

Choose a reason for hiding this comment

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

tyler-yankee left a comment

Choose a reason for hiding this comment

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

tyler-yankee commented Jun 30, 2025

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

tyler-yankee left a comment

Choose a reason for hiding this comment

Uh oh!

tyler-yankee commented Jul 2, 2025

Uh oh!

jwnimmer-tri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tyler-yankee commented Jun 20, 2025 •

edited

Loading

tyler-yankee commented Jun 24, 2025 •

edited

Loading