Skip to content

bazel: make environment compatible with more operating systems.#7086

Merged
maliberty merged 1 commit intoThe-OpenROAD-Project:masterfrom
hzeller:feature-20250409-fix-rulescc
Apr 10, 2025
Merged

bazel: make environment compatible with more operating systems.#7086
maliberty merged 1 commit intoThe-OpenROAD-Project:masterfrom
hzeller:feature-20250409-fix-rulescc

Conversation

@hzeller
Copy link
Collaborator

@hzeller hzeller commented Apr 10, 2025

When running the compilation on NixOS, it is not possible currently to use the llvm_toolchain mentioned in the MODULE.bazel, but necessary to run the local toolchain (clangStdenv). That in turn is strict and only allows to compile C files when invoked with clang and requires clang++ or clang -xc++ to compile C++ files - unlike a typical clang installation that attempts to auto-detect. But Bazel is lazy and invokes c++ compilation with clang ...

Another issue: the latest rules_cc version has hardcoded paths to /bin/bash while the correct way to use that on Posix systems is to call it via /usr/bin/env.

So, this PR solves both these issues:

When running the compilation on NixOS, it is not possible currently
to use the llvm_toolchain mentioned in the MODULE.bazel, but necessary
to run the local toolchain (`clangStdenv`). That in turn is strict and
only allows to compile C files when invoked with `clang` and
requires `clang++` or `clang -xc++` to compile C++ files - unlike a typical clang
installation that attempts to auto-detect. But Bazel is lazy and
invokes c++ compilation with `clang` ...

Another issue: the latest rules_cc version has hardcoded paths to
`/bin/bash` while the correct way to use that on Posix systems is to
call it via `/usr/bin/env`.

So, this PR solves both these issues:

  * Add `-xc++` to the compiler invocation for cxx compilation.

  * Use a rules_cc that has the fix to use /usr/bin/env
    (bazelbuild/rules_cc#306), but that
    is not released yet on BCR (once 0.1.2 is out we can probably
    use that directly).

Signed-off-by: Henner Zeller <hzeller@google.com>
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"


# The current rules_cc 0.1.1 hardcodes script locations to `#!/bin/bash`
# instead of using `#!/usr/bin/env bash`.
# This is fixed by https://github.com/bazelbuild/rules_cc/pull/306 but
Copy link
Member

Choose a reason for hiding this comment

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

This is closed as of three weeks ago. Is it going to be released?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it is in the repo, but they have not cut a release yet.

Copy link
Member

Choose a reason for hiding this comment

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

Weird - it says closed not merged.

@maliberty
Copy link
Member

I'm curious why nix can't use the toolchain compiler. Is it supposed to be self-contained.

@hzeller
Copy link
Collaborator Author

hzeller commented Apr 10, 2025

I'm curious why nix can't use the toolchain compiler. Is it supposed to be self-contained.

The llvm toolchain repo actually is only self-contained on the surface. It has some shell script that attempts to match the operating system and then downloads the relevant binary from the llvm download site. This mostly works for some more common OSes.

However, it doesn't have a match for NixOS. And even if it would download, say the 'debian' version, the binaries are not self-contained as they are dynamically linked which makes it hard to run on Nix because it is very strict what and where libraries are.

A proper self-containment would be if it would download llvm sources itself, then boostrap compile it first with the local compiler then again with itself...
Or, it would already help if it downloaded a statically linked Linux binary for the compiler because then it would work on any linux distro, including Nix.

@maliberty
Copy link
Member

I didn't realize it isn't statically linked, that is a hole in the method but I suppose it works on most OSes

@maliberty maliberty merged commit 0755d1d into The-OpenROAD-Project:master Apr 10, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants