Skip to content

Conversation

@jschaf
Copy link
Contributor

@jschaf jschaf commented Oct 7, 2024

The default name for the rules_jvm_external.maven rule is "maven". When not set, it defaults to "maven". For root modules also using rules_jvm_external, the name clash causes a warning:

DEBUG: $TMP/external/rules_jvm_external~/private/extensions/maven.bzl:154:14:
The maven repository 'maven' is used in two different bazel modules,
originally in '<my_workspace>' and now in 'protobuf'

Summarizing @shs96c in 1:

The common maven repo name allows rulesets to contribute to the user's JARs.
However, this implies that maven is for the end user, not for transitive
dependencies. If a ruleset needs private dependencies, it should use a custom
namespace rather than the maven namespace.

Since protobuf is not contributing to user's JARs, we'll use a custom namespace. There's precedent for using a custom namespace for library modules:

  • rules_jvm_external uses rules_jvm_external_deps instead of maven.
  • rules_kotlin uses kotlin_rules_maven instead of maven.

Fixes #16839.

The default name for the rules_jvm_external.maven rule is "maven". When not set,
it defaults to "maven". For root modules also using rules_jvm_external, the name
clash causes a warning:

```
DEBUG: $TMP/external/rules_jvm_external~/private/extensions/maven.bzl:154:14:
The maven repository 'maven' is used in two different bazel modules,
originally in '<my_workspace>' and now in 'protobuf'
```

Summarizing @shs96c in [1]:

> The common maven repo name allows rulesets to contribute to the user's JARs.
> However, this implies that maven is for the end user, not for transitive
> dependencies. If a ruleset needs private dependencies, it should use a custom
> namespace rather than the maven namespace.

Since protobuf is not contributing to user's JARs, we'll use a custom namespace.
There's precedent for using a custom namespace for library modules:

- rules_jvm_external uses `rules_jvm_external_deps` instead of `maven`.
- rules_kotlin uses `kotlin_rules_maven` instead of `maven`.

[1]: bazel-contrib/rules_jvm_external#916 (comment)

Fixes protocolbuffers#16839.
@jschaf jschaf requested a review from a team as a code owner October 7, 2024 16:50
@jschaf jschaf requested review from haberman and removed request for a team October 7, 2024 16:50
@haberman haberman requested review from deannagarcia and removed request for haberman October 8, 2024 17:43
Copy link
Member

@deannagarcia deannagarcia left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@deannagarcia deannagarcia added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 8, 2024
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 8, 2024
)

load("@maven//:defs.bzl", "pinned_maven_install")
load("@protobuf_maven//:defs.bzl", "pinned_maven_install")
Copy link
Contributor

@JasonLunn JasonLunn Oct 9, 2024

Choose a reason for hiding this comment

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

Is this change correct? In the error logs for the failing tests we see:

ERROR: Failed to load Starlark extension '@@protobuf_maven//:defs.bzl'.
Cycle in the workspace file detected. This indicates that a repository is used prior to being defined.
The following chain of repository dependencies lead to the missing definition.
- @@protobuf_maven
This could either mean you have to add the '@@protobuf_maven' repository with a statement like `http_archive` in your WORKSPACE file (note that transitive dependencies are not added automatically), or move an existing definition earlier in your WORKSPACE file.
ERROR: Error computing the main repository mapping: cycles detected during computation of main repo mapping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the problem was I forgot to add the name = "protobuf_maven" in the WORKSPACE. I only did it in MODULE.bazel for the first commit.

@JasonLunn JasonLunn added 🅰️ safe for tests Mark a commit as safe to run presubmits over and removed wait for user action labels Oct 10, 2024
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 10, 2024
@ejona86
Copy link
Contributor

ejona86 commented Oct 16, 2024

FYI: this looks broken for protobuf-util, as that does contribute to user's JARs. See my comment on a PR to do something similar in gRPC: grpc/grpc-java#11614 (comment)

@ejona86
Copy link
Contributor

ejona86 commented Oct 16, 2024

Because java_proto_library uses @@protobuf+//:java_toolchain which has a runtime dependency on //java/core, you must use the Bazel-built jars by default:

runtime = ":core",

And if you use Bazel-built "core" then you need Bazel-built "util".

@JasonLunn
Copy link
Contributor

@ejona86 - can you open a new issue for the protobuf-util problem?

@jschaf jschaf deleted the joe/protobuf-maven branch November 7, 2024 18:28
zhangskz pushed a commit that referenced this pull request Dec 2, 2024
The default name for the rules_jvm_external.maven rule is "maven". When not set, it defaults to "maven". For root modules also using rules_jvm_external, the name clash causes a warning:

```
DEBUG: $TMP/external/rules_jvm_external~/private/extensions/maven.bzl:154:14:
The maven repository 'maven' is used in two different bazel modules,
originally in '<my_workspace>' and now in 'protobuf'
```

Summarizing @shs96c in [1]:

> The common maven repo name allows rulesets to contribute to the user's JARs.
> However, this implies that maven is for the end user, not for transitive
> dependencies. If a ruleset needs private dependencies, it should use a custom
> namespace rather than the maven namespace.

Since protobuf is not contributing to user's JARs, we'll use a custom namespace. There's precedent for using a custom namespace for library modules:

- rules_jvm_external uses `rules_jvm_external_deps` instead of `maven`.
- rules_kotlin uses `kotlin_rules_maven` instead of `maven`.

[1]: bazel-contrib/rules_jvm_external#916 (comment)

Fixes #16839.

Closes #18641

COPYBARA_INTEGRATE_REVIEW=#18641 from jschaf:joe/protobuf-maven bd2c62f
PiperOrigin-RevId: 684625084
zhangskz added a commit that referenced this pull request Dec 2, 2024
The default name for the rules_jvm_external.maven rule is "maven". When not set, it defaults to "maven". For root modules also using rules_jvm_external, the name clash causes a warning:

```
DEBUG: $TMP/external/rules_jvm_external~/private/extensions/maven.bzl:154:14:
The maven repository 'maven' is used in two different bazel modules,
originally in '<my_workspace>' and now in 'protobuf'
```

Summarizing @shs96c in [1]:

> The common maven repo name allows rulesets to contribute to the user's JARs.
> However, this implies that maven is for the end user, not for transitive
> dependencies. If a ruleset needs private dependencies, it should use a custom
> namespace rather than the maven namespace.

Since protobuf is not contributing to user's JARs, we'll use a custom namespace. There's precedent for using a custom namespace for library modules:

- rules_jvm_external uses `rules_jvm_external_deps` instead of `maven`.
- rules_kotlin uses `kotlin_rules_maven` instead of `maven`.

[1]: bazel-contrib/rules_jvm_external#916 (comment)

Fixes #16839.

Closes #18641

COPYBARA_INTEGRATE_REVIEW=#18641 from jschaf:joe/protobuf-maven bd2c62f
PiperOrigin-RevId: 684625084

Co-authored-by: Joe Schafer <[email protected]>
copybara-service bot pushed a commit that referenced this pull request Jul 9, 2025
Fixes #21177

This PR essentially reverts #18641, which claimed

> Since protobuf is not contributing to user's JARs

This is not true since targets like `@com_google_protobuf//:protobuf_java` are meant to be consumed by other projects, therefore protobuf should not use a private maven install namespace. Otherwise, it leads to duplicated maven jars and classpath conflicts. See #21177 and bazel-contrib/rules_jvm_external#916 (comment)

The original warning message caused by multiple modules contributing to `maven` can be suppressed with bazel-contrib/rules_jvm_external#1393, which will be available in rules_jvm_external 6.8.

The PR use a repo mapping trick of `use_repo` to keep BUILD files intact, underlying both `protobuf_maven` and `protobuf_maven_dev` points to the universal `maven` install, while targets in `protobuf_maven_dev` are only available while protobuf is the root module and won't propagate to dependents.

PiperOrigin-RevId: 781031722
copybara-service bot pushed a commit that referenced this pull request Jul 9, 2025
Fixes #21177

This PR essentially reverts #18641, which claimed

> Since protobuf is not contributing to user's JARs

This is not true since targets like `@com_google_protobuf//:protobuf_java` are meant to be consumed by other projects, therefore protobuf should not use a private maven install namespace. Otherwise, it leads to duplicated maven jars and classpath conflicts. See #21177 and bazel-contrib/rules_jvm_external#916 (comment)

The original warning message caused by multiple modules contributing to `maven` can be suppressed with bazel-contrib/rules_jvm_external#1393, which will be available in rules_jvm_external 6.8.

The PR use a repo mapping trick of `use_repo` to keep BUILD files intact, underlying both `protobuf_maven` and `protobuf_maven_dev` points to the universal `maven` install, while targets in `protobuf_maven_dev` are only available while protobuf is the root module and won't propagate to dependents.

PiperOrigin-RevId: 781031722
copybara-service bot pushed a commit that referenced this pull request Jul 9, 2025
Fixes #21177

This PR essentially reverts #18641, which claimed

> Since protobuf is not contributing to user's JARs

This is not true since targets like `@com_google_protobuf//:protobuf_java` are meant to be consumed by other projects, therefore protobuf should not use a private maven install namespace. Otherwise, it leads to duplicated maven jars and classpath conflicts. See #21177 and bazel-contrib/rules_jvm_external#916 (comment)

The original warning message caused by multiple modules contributing to `maven` can be suppressed with bazel-contrib/rules_jvm_external#1393, which will be available in rules_jvm_external 6.8.

The PR use a repo mapping trick of `use_repo` to keep BUILD files intact, underlying both `protobuf_maven` and `protobuf_maven_dev` points to the universal `maven` install, while targets in `protobuf_maven_dev` are only available while protobuf is the root module and won't propagate to dependents.

PiperOrigin-RevId: 781031722
copybara-service bot pushed a commit that referenced this pull request Jul 11, 2025
Fixes #21177

This PR essentially reverts #18641, which claimed

> Since protobuf is not contributing to user's JARs

This is not true since targets like `@com_google_protobuf//:protobuf_java` are meant to be consumed by other projects, therefore protobuf should not use a private maven install namespace. Otherwise, it leads to duplicated maven jars and classpath conflicts. See #21177 and bazel-contrib/rules_jvm_external#916 (comment)

The original warning message caused by multiple modules contributing to `maven` can be suppressed with bazel-contrib/rules_jvm_external#1393, which will be available in rules_jvm_external 6.8.

Closes #22544

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using maven as the repo name causes duplicate warnings when using bzlmod

5 participants