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

incompatible_disable_genrule_cc_toolchain_dependency: remove genrule's implicit dependency on the cc_toolchain #6867

Open
katre opened this Issue Dec 7, 2018 · 7 comments

Comments

Projects
None yet
5 participants
@katre
Copy link
Member

katre commented Dec 7, 2018

Flag: --incompatible_disable_genrule_cc_toolchain_dependency
Available since: 0.22
Will be flipped in: 0.24

Currently, every genrule target has an implicit dependency on the cc_toolchain, caused by the ":cc_toolchain" late-bound attribute. With the recent migration to using the "toolchains" attribute to setting this dependency, the only remaining use of ":cc_toolchains" is to provide the CC_FLAGS Make variable.

In order to remove this dependency, a new rule will be added, cc_flags_supplier, that exists only to make the CC_FLAGS Make variable available to other targets.

Once users have migrated to using the new rule to provide CC_FLAGS, the old behavior will be removed. Also, CcToolchainProvider will stop providing the CC_FLAGS Make variable.

Migration notes:
To migrate, add a new toolchains dependency for any genrule target that accesses CC_FLAGS.

Example old behavior:

genrule(
  name = "display_flags",
  out = ["cc_flags.log"],
  cmd = "echo $(CC_FLAGS) > $@",
  toolchains = ["@bazel_tools//tools/cpp:current_cc_toolchain"],
)

New behavior:

cc_flags_supplier(name = "cc_flags")
genrule(
  name = "display_flags",
  out = ["cc_flags.log"],
  cmd = "echo $(CC_FLAGS) > $@",
  toolchains = [":cc_flags", "@bazel_tools//tools/cpp:current_cc_toolchain"],
)

@katre katre self-assigned this Dec 7, 2018

bazel-io pushed a commit that referenced this issue Dec 11, 2018

Add the new incompatible flag to control genrule's cc toolchain depen…
…dency.

Part of #6867.

Future work will tie this into the actual genrule internals.

PiperOrigin-RevId: 225032807
@hlopko

This comment has been minimized.

Copy link
Contributor

hlopko commented Jan 25, 2019

@aehlig this issue didn't have correct labels, I just added them, FYI.
@katre can you make sure this flag is mentioned in the release notes? Thanks!

@hlopko

This comment has been minimized.

Copy link
Contributor

hlopko commented Jan 25, 2019

I also edited the issue description, the flag is not present in 0.21, only in 0.22. So the flag will be flipped in 0.23.

@katre

This comment has been minimized.

Copy link
Member Author

katre commented Jan 25, 2019

Thanks, I'll take care of the release notes.

@katre

This comment has been minimized.

Copy link
Member Author

katre commented Jan 25, 2019

Added the link to the release notes for 0.22, let me know if more detail is needed.

@philwo

This comment has been minimized.

Copy link
Member

philwo commented Feb 6, 2019

This flag was not flipped in time for the Bazel 0.23.0 release and will thus be postponed to Bazel 0.24.0.

@scottmin0r

This comment has been minimized.

Copy link

scottmin0r commented Feb 6, 2019

This flag doesn't appear in the output of bazel help build - should it?

Output of bazel help build | grep 'incompatible_disable_':

  --[no]incompatible_disable_expand_if_all_available_in_flag_set (a boolean; default: "false")
  --[no]incompatible_disable_legacy_crosstool_fields (a boolean; default: "false")
  --[no]incompatible_disable_runtimes_filegroups (a boolean; default: "false")
  --[no]incompatible_disable_deprecated_attr_params (a boolean; default: "false")
  --[no]incompatible_disable_objc_provider_resources (a boolean; default: "false")

Output of bazel help build | grep 'incompatible_disable_genrule':

<empty>

Output of bazel version:

Build label: 0.22.0
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Mon Jan 28 12:58:08 2019 (1548680288)
Build timestamp: 1548680288
Build timestamp as int: 1548680288
@katre

This comment has been minimized.

Copy link
Member Author

katre commented Feb 6, 2019

I can change it to be documented, but not sure that can be added to the 0.23 release. I'm not really sure it's needed: it's a migration flag to help test the requirement to use @bazel_tools//tools/cpp:cc_flags instead of automatically getting $(CC_FLAGS).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment