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

Stop exporting language modules #16295

Closed
wants to merge 7 commits into from

Conversation

comius
Copy link
Contributor

@comius comius commented Sep 19, 2022

No description provided.

private static final ImmutableSet<PackageIdentifier> allowedRepositories =
ImmutableSet.of(
PackageIdentifier.createUnchecked("_builtins", ""),
PackageIdentifier.createUnchecked("rules_android", ""),
Copy link
Member

Choose a reason for hiding this comment

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

just FYI, this won't work with Bzlmod. The module rules_android won't have the repo name rules_android, but something like rules_android~1.2.3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Very helpful!

@brentleyjones
Copy link
Contributor

After this, how does one call cc_common.merge_cc_infos or cc_common.get_memory_inefficient_command_line (methods that my ruleset for Xcode IDE integration depends on, as well as Tulsi depends on)?

@comius
Copy link
Contributor Author

comius commented Sep 19, 2022

After this, how does one call cc_common.merge_cc_infos or cc_common.get_memory_inefficient_command_line (methods that my ruleset for Xcode IDE integration depends on, as well as Tulsi depends on)?

Via load("@rules_cc/cc:defs.bzl", "cc_common"). I just noticed my PR didn't get properly exported into rules_cc :(

@brentleyjones
Copy link
Contributor

Ahh, I didn't see that rules_cc was being revived. Thanks!

Does rules_cc have all of the changes to the crosstool/toolchain that have been made to Bazel recently? I recall that when people used rules_cc in the past they would get old macOS crosstool/toolchain behaviors, and we've been recommending they remove the use: envoyproxy/envoy#19760.

@comius
Copy link
Contributor Author

comius commented Sep 19, 2022

Ahh, I didn't see that rules_cc was being revived. Thanks!

I've unclogged the pipelines!
https://github.com/bazelbuild/rules_cc/blob/main/cc/defs.bzl#L178

Does rules_cc have all of the changes to the crosstool/toolchain that have been made to Bazel recently? I recall that when people used rules_cc in the past they would get old macOS crosstool/toolchain behaviors, and we've been recommending they remove the use: envoyproxy/envoy#19760.

I don't know... feel free to push the changes...

@brentleyjones
Copy link
Contributor

I don't know... feel free to push the changes...

cc: @keith

Will rules_cc become the canonical location? The issue we had before was it was hard to keep both up to date (going through review on both, dealing with versioning issues, etc).

@comius
Copy link
Contributor Author

comius commented Sep 19, 2022

Will rules_cc become the canonical location? The issue we had before was it was hard to keep both up to date (going through review on both, dealing with versioning issues, etc).

It will, however that will take time.

The first thing that's coming is ProtoInfo, JavaInfo and CcInfo in Starlark (most likely in this order). I tried different migration paths, and the easiest migration from "builtin Starlark code" to "pure Starlark code in repos" seems to be first force users to use @rules_proto, @rules_java and @rules_cc. It makes it possible to have some deps on providers still in Bazel.

@comius comius force-pushed the stop-exporting-modules branch from 2bb4ed3 to 5b949a6 Compare September 21, 2022 18:58
@comius comius force-pushed the stop-exporting-modules branch from 5b949a6 to d84a216 Compare September 21, 2022 19:21
@comius comius force-pushed the stop-exporting-modules branch from d84a216 to cbd6873 Compare September 22, 2022 06:30
@comius comius closed this Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants