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

Ignore transitive cppmap files from dotd files #21832

Closed

Conversation

keith
Copy link
Member

@keith keith commented Mar 27, 2024

When clang generates dotd files when using -fmodule-map-file any extern module directives in the modulemap are included in the dotd file if they exist. The result of this was that with sandboxing disabled the dotd file included transitive cppmap files that weren't in its input set, resulting in build failures. This change excludes those instead since they're not required as evidence by the fact that with sandboxing enabled they are not part of the input set.

Fixes #21592

@keith
Copy link
Member Author

keith commented Mar 27, 2024

I'm curious if someone who knows this area better has a better solution in mind for this!

@keith keith marked this pull request as ready for review April 25, 2024 22:30
@keith keith requested a review from lberki as a code owner April 25, 2024 22:30
@github-actions github-actions bot added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Apr 25, 2024
@keith
Copy link
Member Author

keith commented Apr 25, 2024

@lberki curious to hear your thoughts here. an alternative would be to always propagate all cppmap files, but I would think that would be undesirable for other reasons like the command line exploding

@keith keith force-pushed the ks/ignore-transitive-cppmap-files-from-dotd-files branch from bf8a7eb to 887ed5d Compare May 14, 2024 19:28
@keith
Copy link
Member Author

keith commented May 14, 2024

@lberki can you take a look or recommend someone else who should review?

@keith
Copy link
Member Author

keith commented Jun 4, 2024

@pzembrod could you take a look? 🙏

@keith
Copy link
Member Author

keith commented Jun 25, 2024

@pzembrod can you help direct this one? this blocks llvm's upgrade to 7.x

@keith
Copy link
Member Author

keith commented Jul 9, 2024

@lberki @pzembrod can you help route

@trybka trybka self-requested a review July 25, 2024 16:12
Copy link
Contributor

@trybka trybka left a comment

Choose a reason for hiding this comment

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

This seems fine. I will try running some presubmits internally, too.

@fmeum
Copy link
Collaborator

fmeum commented Jul 25, 2024

@bazel-io fork 7.3.0

@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Jul 26, 2024
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Jul 26, 2024
When clang generates dotd files when using `-fmodule-map-file` any `extern module` directives in the modulemap are included in the dotd file if they exist. The result of this was that with sandboxing disabled the dotd file included transitive cppmap files that weren't in its input set, resulting in build failures. This change excludes those instead since they're not required as evidence by the fact that with sandboxing enabled they are not part of the input set.

Fixes bazelbuild#21592

Closes bazelbuild#21832.

PiperOrigin-RevId: 656382428
Change-Id: I4bc9802884ce1bc66ceda65a602db8dffbd1d9ea
@keith keith deleted the ks/ignore-transitive-cppmap-files-from-dotd-files branch July 26, 2024 15:36
github-merge-queue bot pushed a commit that referenced this pull request Jul 26, 2024
When clang generates dotd files when using `-fmodule-map-file` any
`extern module` directives in the modulemap are included in the dotd
file if they exist. The result of this was that with sandboxing disabled
the dotd file included transitive cppmap files that weren't in its input
set, resulting in build failures. This change excludes those instead
since they're not required as evidence by the fact that with sandboxing
enabled they are not part of the input set.

Fixes #21592

Closes #21832.

PiperOrigin-RevId: 656382428
Change-Id: I4bc9802884ce1bc66ceda65a602db8dffbd1d9ea

Commit
ad53147

Co-authored-by: Keith Smiley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

layering_check doesn't work without sandboxing
3 participants