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

Expose virtual:original header mappings via CcToolchain variables #21157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rbeasley
Copy link
Contributor

Whenever prefix and/or strip_include_prefix is passed into cc_library rules, Bazel copies the headers into a special "_virtual_includes" directory that allows for such path transformations to occur. Unfortunately, this also has the side-effect of changing how source files are designated within the source binaries: they will refer to the file in the _virtual_includes sandbox directory instead of the actual files in the build tree.

This change allows us to work around this problem by providing a mapping between the "virtual" and where they are in the filesystem as a CcToolchainConfigInfo variable, virtual_to_original_dirs, which ends up being a list of <virtual>=<original> mappings that can be used in rules_cc toolchain features, specifically using -ffile-prefix-map.

Questions

  • Is <virtual>=<original> okay, or do we need a more sophisticated lazy expansion involving subvariables like what libraries_to_link has? E.g., do we imagine another compiler command-line flag format that'd require A:B instead of A=B? (Then again, considering MSVC's ASan is implemented w/ /fsanitize=address, maybe they're on the A=B train now?)

Notes

  • We mostly mimicked coverage's virtual_to_original_headers.

Fixes upstream #11874.

Whenever `prefix` and/or `strip_include_prefix` is passed into
`cc_library` rules, Bazel copies the headers into a special
"_virtual_includes" directory that allows for such path transformations
to occur.  Unfortunately, this also has the side-effect of changing how
source files are designated within the source binaries: they will refer
to the file in the `_virtual_includes` sandbox directory instead of the
actual files in the build tree.

This change allows us to work around this problem by providing a mapping between
the "virtual" and where they are in the filesystem as a
[CcToolchainConfigInfo variable][1], `virtual_to_original_dirs`, which ends up
being a list of `<virtual>=<original>` mappings that can be used in rules_cc
toolchain features, specifically using [-ffile-prefix-map][2].

Questions
=========
- Is `<virtual>=<original>` okay, or do we need a more sophisticated lazy
  expansion involving subvariables like what `libraries_to_link` has?
  E.g., do we imagine another compiler command-line flag format that'd
  require `A:B` instead of `A=B`?  (Then again, considering MSVC's ASan
  is implemented w/ `/fsanitize=address`, maybe they're on the `A=B`
  train now?)

Notes
=====
- We mostly mimicked coverage's `virtual_to_original_headers`.

Fixes upstream bazelbuild#11874.

[1]: https://docs.bazel.build/versions/master/cc-toolchain-config-reference.html#cctoolchainconfiginfo-build-variables
[2]: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-ffile-prefix-map

Co-authored-by: Andrew Psaltis <apsaltis@vmware.com>
@github-actions github-actions bot added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Jan 31, 2024
@lberki lberki requested review from comius and removed request for lberki and oquenchil February 1, 2024 09:09
* context. Unlike {@link virtualToOriginalHeaders}, this is populated even if
* coverage isn't enabled.
*/
private final NestedSet<Tuple> virtualToOriginalDirs;
Copy link
Contributor

Choose a reason for hiding this comment

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

This new field will potentially result in memory regression. There's already virtualToOriginalHeaders, which is probably even larger, containing all header files, however it's only populated with coverage enabled.

I wonder if there's a more efficient way to reconstruct the information from _virtual_includes constant. It would be quite hacky though, but a wrapper script should be able to do it.

@comius comius added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-user-response Awaiting a response from the author team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants