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

cc_toolchain should use compiler_files for CppCompileActions #6927

Closed
hlopko opened this issue Dec 14, 2018 · 2 comments
Closed

cc_toolchain should use compiler_files for CppCompileActions #6927

hlopko opened this issue Dec 14, 2018 · 2 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug

Comments

@hlopko
Copy link
Member

hlopko commented Dec 14, 2018

Currently, cc_toolchain uses all_files attribute for compilation actions, which is unnecessarily big. This effectively makes compiler_files unused.

We should use compiler_files instead.

Note: update cc_toolchain docs after fixing this, docs reference this issue.

@hlopko hlopko added type: bug P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules labels Dec 14, 2018
@benjaminp
Copy link
Collaborator

This issue is about making the following change, right?

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java
index f266feea0e..917c4a43ce 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java
@@ -47,9 +47,7 @@ public class BazelCppSemantics implements AspectLegalCppSemantics {
       FeatureConfiguration featureConfiguration,
       CppCompileActionBuilder actionBuilder) {
     actionBuilder
-        // Because Bazel does not support include scanning, we need the entire crosstool filegroup,
-        // including header files, as opposed to just the "compile" filegroup.
-        .addTransitiveMandatoryInputs(actionBuilder.getToolchain().getAllFiles())
+        .addTransitiveMandatoryInputs(actionBuilder.getToolchain().getCompilerFiles())
         .setShouldScanIncludes(false);
   }

Should we add an --incompatible_ flag for this? I'd like to see it happen.

benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 14, 2019
This option makes C++ compilation actions include only the toolchain's compiler_files artifacts as inputs not all_files.

See bazelbuild#6927.
benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 14, 2019
This option makes C++ compilation actions include only the toolchain's compiler_files artifacts as inputs not all_files.

See bazelbuild#6927.
benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 25, 2019
This option makes C++ compilation actions include only the toolchain's compiler_files artifacts as inputs not all_files.

See bazelbuild#6927.
benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 25, 2019
This option makes C++ compilation actions include only the toolchain's compiler_files artifacts as inputs not all_files.

See bazelbuild#6927.
benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 26, 2019
This option makes C++ compilation actions include only the toolchain's compiler_files artifacts as inputs not all_files.

See bazelbuild#6927.
benjaminp added a commit to benjaminp/bazel that referenced this issue Apr 17, 2019
This option makes
1. C++ compilation actions include only the toolchain's compiler_files artifacts as inputs not all_files.
2. Non-preprocessed assembler actions include only the toolchain's as_files artifacts as inputs not all_files.
3. Archiver link actions include the toolchain's ar_files artifacts as inputs rather than linker_files.

See bazelbuild#6927 and bazelbuild#6928.
benjaminp added a commit to benjaminp/bazel that referenced this issue May 7, 2019
This option makes
1. C++ compilation actions include only the toolchain's compiler_files artifacts as inputs not all_files.
2. Non-preprocessed assembler actions include only the toolchain's as_files artifacts as inputs not all_files.
3. Archiver link actions include the toolchain's ar_files artifacts as inputs rather than linker_files.

See bazelbuild#6927 and bazelbuild#6928.
benjaminp added a commit to benjaminp/bazel that referenced this issue May 8, 2019
This option makes
1. C++ compilation actions include only the toolchain's compiler_files artifacts as inputs not all_files.
2. Non-preprocessed assembler actions include only the toolchain's as_files artifacts as inputs not all_files.
3. Archiver link actions include the toolchain's ar_files artifacts as inputs rather than linker_files.

See bazelbuild#6927 and bazelbuild#6928.
benjaminp added a commit to benjaminp/bazel that referenced this issue May 16, 2019
This option makes
1. C++ compilation actions include only the toolchain's compiler_files artifacts as inputs not all_files.
2. Non-preprocessed assembler actions include only the toolchain's as_files artifacts as inputs not all_files.
3. Archiver link actions include the toolchain's ar_files artifacts as inputs rather than linker_files.

See bazelbuild#6927 and bazelbuild#6928.
@c-mita
Copy link
Member

c-mita commented Nov 23, 2020

I believe this is resolved.

@c-mita c-mita closed this as completed Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

No branches or pull requests

4 participants