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.

Sign up for GitHub

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

Jump to bottom

Add --incompatible_use_specific_tool_files #7725

Closed
wants to merge 1 commit into from
Closed

Add --incompatible_use_specific_tool_files #7725

wants to merge 1 commit into from

Conversation

benjaminp
Copy link
Collaborator

@benjaminp benjaminp commented Mar 14, 2019 •
edited
Loading

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 #6927 and #6928.

@benjaminp benjaminp requested a review from lberki as a code owner March 14, 2019 15:57
@lberki lberki requested review from hlopko and removed request for lberki March 14, 2019 15:59
@aiuto aiuto added the team-Rules-CPP Issues for C++ rules label Mar 18, 2019
@benjaminp benjaminp force-pushed the use-compiler-files branch 3 times, most recently from bb52419 to cf44a22 Compare March 26, 2019 15:19
@hlopko hlopko requested a review from scentini March 26, 2019 16:46
@hlopko
Copy link
Member

hlopko commented Mar 26, 2019

I'm sorry Benjamin, I thought I assigned @scentini to this PR a while ago. I didn't :)

@hlopko
Copy link
Member

hlopko commented Mar 28, 2019

In general I like it, thank you for working on it. I'd want to make sure that other attributes are used when they should be (linker_files for linking, ar_files for archiving, as_files for assembly), and if not, have one big incompatible flag for all. @scentini can you take a look please?

@benjaminp
Copy link
Collaborator Author

#6928 suggests there's indeed more work to do.

@hlopko hlopko removed their request for review April 17, 2019 14:49
@benjaminp benjaminp changed the title Add --incompatible_use_compiler_files_for_cc_compilation. Add --incompatible_use_specific_tool_files. Apr 17, 2019
scentini
scentini reviewed May 7, 2019
View reviewed changes
Copy link
Contributor

@scentini scentini left a comment

Choose a reason for hiding this comment

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

In general this LGTM. Can we have tests covering the new behavior either in
https://github.com/bazelbuild/bazel/blob/db0392174ac1d85e79c54f6ed8b31accc161090f/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
or
https://github.com/bazelbuild/bazel/blob/db0392174ac1d85e79c54f6ed8b31accc161090f/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java ?

src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java Outdated Show resolved Hide resolved
@benjaminp
Add --incompatible_use_specific_tool_files.
2619f37 
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
Copy link
Collaborator Author

Any further comments? It'd be nice to make 0.27.

@scentini scentini changed the title Add --incompatible_use_specific_tool_files. incompatible_use_specific_tool_files: Use appropriate artifacts from cc_toolchain as inputs to C++ actions. May 31, 2019
@scentini scentini changed the title incompatible_use_specific_tool_files: Use appropriate artifacts from cc_toolchain as inputs to C++ actions. Add --incompatible_use_specific_tool_files May 31, 2019
@scentini
Copy link
Contributor

I created #8531 to track this incompatible change, as per https://www.bazel.build/breaking-changes-guide.html.

@bazel-io bazel-io closed this in b691915 May 31, 2019
irengrig pushed a commit to irengrig/bazel that referenced this pull request Jun 18, 2019
@benjaminp @irengrig
Add --incompatible_use_specific_tool_files.
674c1a1 
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#8531.

Closes bazelbuild#7725.

PiperOrigin-RevId: 250880419
irengrig pushed a commit to irengrig/bazel that referenced this pull request Jul 15, 2019
@benjaminp @irengrig
Add --incompatible_use_specific_tool_files.
678e750 
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#8531.

Closes bazelbuild#7725.

PiperOrigin-RevId: 250880419
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scentini scentini scentini left review comments

Assignees

@scentini scentini

Labels
cla: yes team-Rules-CPP Issues for C++ rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@benjaminp @hlopko @scentini @aiuto @googlebot