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

feat: Add a fully working toolchain #13

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nathaniel-brough
Copy link
Contributor

No description provided.

@nathaniel-brough nathaniel-brough requested a review from tpudlik April 3, 2023 01:03
@nathaniel-brough
Copy link
Contributor Author

Got a little bit more work to clean up the UI. It does currently require libstdc++-9-dev if your on ubuntu, as I've hard coded the
system include paths. I'll admit I'm a bit stuck on some of these compiler errors https://github.com/bazelembedded/modular_cc_toolchains/actions/runs/4591805987/jobs/8108291820#step:5:661. Although some basic compilation/runtime tests I've done locally sort of demonstrate that the toolchain works for more basic applications.

@nathaniel-brough
Copy link
Contributor Author

Sorry for being slow on this front :)

Copy link
Collaborator

@tpudlik tpudlik left a comment

Choose a reason for hiding this comment

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

Regarding the compiler errors: could it be that some cxx_builtin_include_directories are still missing?

FWIW for Fuchsia clang (https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang) I would use,

  "%package(@clang//)%/include/c++/v1/x86_64-unknown-linux-gnu/c++/v1",
  "%package(@clang//)%/include/c++/v1",
  "%package(@clang//)%/lib/clang/15.0.0/include",
  "%package(@clang//)%/lib/clang/15.0.0/share",

I wonder if Fuchsia clang + Fuchsia sysroot (https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/sysroot) would work in CI.

ACTION_NAME_GROUPS.all_cc_compile_actions: [
"-nostdinc",
"-nostdinc++",
"-isystem/usr/include/c++/9",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are these include paths listed both here (in this default-enabled feature) and in cxx_builtin_include_directories?

I guess it's just not clear to me what this feature achieves: you first exclude built-in include paths, but then add them right back in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's mostly a convenience thing to make sure I have consistent includes. Sometimes clang/gcc will change/add include directories depending on other flags. This is usually true for sanitizers. But it gets rather annoying having the compiler "autodetect" different include paths and having them not matching in bazel, not to mention the error messages when this happens are rather cryptic. You end up with a lot of blackbox inconsistencies between environments. But you are probably right, it's probably not 100% necessary here and adds unnecessary indirection.

It's also an intermediate precursor to having fully hermetic libraries as well. But we aren't quite there yet with the modular toolchains.

runfiles = runfiles.merge_all([action_config_set[DefaultInfo].default_runfiles])
transitive_files.append(action_config_set[DefaultInfo].files)

# Deduplicate features
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Deduplicate action_configs

@nathaniel-brough
Copy link
Contributor Author

nathaniel-brough commented Apr 13, 2023

Regarding the compiler errors: could it be that some cxx_builtin_include_directories are still missing?

FWIW for Fuchsia clang (https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang) I would use,

  "%package(@clang//)%/include/c++/v1/x86_64-unknown-linux-gnu/c++/v1",
  "%package(@clang//)%/include/c++/v1",
  "%package(@clang//)%/lib/clang/15.0.0/include",
  "%package(@clang//)%/lib/clang/15.0.0/share",

I wonder if Fuchsia clang + Fuchsia sysroot (https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/sysroot) would work in CI.

Is the %package(@clang//)% syntax documented anywhere I haven't seen this before.

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