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

Use a custom name for the maven repository (maven_protobuf) #17190

Closed
wants to merge 1 commit into from
Closed

Use a custom name for the maven repository (maven_protobuf) #17190

wants to merge 1 commit into from

Conversation

joca-bt
Copy link

@joca-bt joca-bt commented Jun 20, 2024

Addresses #17117.

@google-cla Google CLA
Copy link

google-cla bot commented Jun 20, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@joca-bt joca-bt marked this pull request as ready for review June 21, 2024 12:25
@joca-bt joca-bt requested a review from a team as a code owner June 21, 2024 12:25
@joca-bt joca-bt requested review from haberman and removed request for a team June 21, 2024 12:25
@joca-bt
Copy link
Author

joca-bt commented Jun 21, 2024

Could someone review the "Check for Safety" so I can run the CI pipeline?

@haberman haberman added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jun 21, 2024
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jun 21, 2024
@joca-bt
Copy link
Author

joca-bt commented Jun 26, 2024

Could someone review this PR? cc @haberman

@zhangskz
Copy link
Member

My understanding is that the contents of this WORKSPACE file shouldn't leak the maven repository -- this is a dev dependency in WORKSPACE for packaging protobuf but not actually in protobuf_deps.bzl (that users should transitively depend on).

The rules_jvm_external issue linked seems to use BCR v21.7. I think this would actually need to be patched to MODULE.bazel 21.7.bcr.1 for rules_jvm_external. I think a dev_dependency=True in the use_extension for maven: https://bazel.build/external/migration#specify_dev_dependencies might address this without having to rename.

@joca-bt
Copy link
Author

joca-bt commented Jul 1, 2024 •
edited
Loading

If I manually fix the protobuf version to a more recent version, then the problem seems to go away. I think we can close it for now, will try updating the protobuf dependency version in the relevant projects.

@joca-bt joca-bt closed this Jul 1, 2024
mbland added a commit to EngFlow/bazel_invocation_analyzer that referenced this pull request Jul 2, 2024
@mbland
Explicitly specify protobuf 27.2, repin maven deps
ad2385a 
Resolves these warnings:

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

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

DEBUG: .../external/rules_jvm_external~/coursier.bzl:593:18:
    Found duplicate artifact versions
    com.google.code.gson:gson has multiple versions 2.11.0, 2.8.9
    com.google.guava:guava has multiple versions 33.2.1-jre, 31.1-jre
    com.google.truth:truth has multiple versions 1.4.3, 1.1.2
    org.mockito:mockito-core has multiple versions 5.12.0, 4.3.1
Please remove duplicate artifacts from the artifact list so you do not
get unexpected artifact versions
```

See also:

- Duplicate maven repositories when importing bazel_deps that use
  maven.install
  bazel-contrib/rules_jvm_external#916

- Using maven as the repo name causes duplicate warnings when using
  bzlmod
  protocolbuffers/protobuf#16839

- MODULE.bazel doesn't define @maven repository
  protocolbuffers/protobuf#17176

- Stop including extra artifacts on maven.install()
  bazel-contrib/rules_jvm_external#1168

- Use a custom name for the maven repository (maven_protobuf)
  protocolbuffers/protobuf#17190
@mbland mbland mentioned this pull request Jul 2, 2024
Merged
mbland added a commit to EngFlow/bazel_invocation_analyzer that referenced this pull request Jul 2, 2024
@mbland
Explicitly specify protobuf 27.2, repin maven deps
afd2b9f 
Resolves these warnings:

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

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

DEBUG: .../external/rules_jvm_external~/coursier.bzl:593:18:
    Found duplicate artifact versions
    com.google.code.gson:gson has multiple versions 2.11.0, 2.8.9
    com.google.guava:guava has multiple versions 33.2.1-jre, 31.1-jre
    com.google.truth:truth has multiple versions 1.4.3, 1.1.2
    org.mockito:mockito-core has multiple versions 5.12.0, 4.3.1
Please remove duplicate artifacts from the artifact list so you do not
get unexpected artifact versions
```

See also:

- Duplicate maven repositories when importing bazel_deps that use
  maven.install
  bazel-contrib/rules_jvm_external#916

- Using maven as the repo name causes duplicate warnings when using
  bzlmod
  protocolbuffers/protobuf#16839

- MODULE.bazel doesn't define @maven repository
  protocolbuffers/protobuf#17176

- Stop including extra artifacts on maven.install()
  bazel-contrib/rules_jvm_external#1168

- Use a custom name for the maven repository (maven_protobuf)
  protocolbuffers/protobuf#17190

Signed-off-by: Mike Bland <mbland@engflow.com>
mbland added a commit to EngFlow/bazel_invocation_analyzer that referenced this pull request Jul 3, 2024
@mbland
Bazel, module, and JAR upgrades (#182)
ddc353f 
Updates as much as possible except rules_jvm_external.

The latest rules_jvm_external v6.1 breaks this project, both under the
previous Bazel version (7.0.2) and the new one (7.2.1). I've filed
bazel-contrib/rules_jvm_external#1189, which uses this repo and this PR as
an example.

See the last section below for details.

---

Bazel update:
- Bazel v7.2.1
  https://github.com/bazelbuild/bazel/releases/tag/7.2.1

Bazel module updates:
- bazel-skylib v1.7.1
  https://github.com/bazelbuild/bazel-skylib/releases/tag/1.7.1
- platforms v0.0.10
  https://github.com/bazelbuild/platforms/releases/tag/0.0.10
- rules_proto v6.0.2
  https://github.com/bazelbuild/rules_proto/releases/tag/6.0.2

JAR updates:
- com.google.code.gson:gson:2.11.0
- com.google.guava:guava:33.2.1-jre
- commons-cli:commons-cli:1.8.0

Test JAR updates:
- com.google.googlejavaformat:google-java-format:1.22.0
- com.google.truth:truth:1.4.3
- com.google.truth.extensions:truth-java8-extension:1.4.3
- org.mockito:mockito-core:5.12.0

---

Added explicit module spec and repinned the maven deps for:
- protobuf v27.2
  https://github.com/protocolbuffers/protobuf/releases/tag/v27.2

Adding protobuf explicitly resolves these warnings:

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

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

DEBUG: .../external/rules_jvm_external~/coursier.bzl:593:18:
    Found duplicate artifact versions
    com.google.code.gson:gson has multiple versions 2.11.0, 2.8.9
    com.google.guava:guava has multiple versions 33.2.1-jre, 31.1-jre
    com.google.truth:truth has multiple versions 1.4.3, 1.1.2
    org.mockito:mockito-core has multiple versions 5.12.0, 4.3.1
Please remove duplicate artifacts from the artifact list so you do not
get unexpected artifact versions
```

See also:

- Duplicate maven repositories when importing bazel_deps that use
  maven.install
  bazel-contrib/rules_jvm_external#916

- Using maven as the repo name causes duplicate warnings when using
  bzlmod
  protocolbuffers/protobuf#16839

- MODULE.bazel doesn't define @maven repository
  protocolbuffers/protobuf#17176

- Stop including extra artifacts on maven.install()
  bazel-contrib/rules_jvm_external#1168

- Use a custom name for the maven repository (maven_protobuf)
  protocolbuffers/protobuf#17190

---

rules_jvm_external v6.1 somehow creates duplicate `jvm_import` rules for
binary and source jars, instead of using the `srcjar` attribute:

```txt
ERROR: Traceback (most recent call last):
  File ".../external/rules_jvm_external~~maven~maven/BUILD",
    line 34, column 11, in <toplevel>
      jvm_import(

Error in jvm_import: jvm_import rule
  'com_google_auto_value_auto_value_annotations' in package ''
  conflicts with existing jvm_import rule, defined at
  .../external/rules_jvm_external~~maven~maven/BUILD:9:11
```

The content of rules_jvm_external~~maven~maven/BUILD at lines 9 and 34:

```bzl
jvm_import(
  name = "com_google_auto_value_auto_value_annotations",
  jars = ["com/google/auto/value/auto-value-annotations/1.10.1/auto-value-annotations-1.10.1.jar"],
```

```bzl
jvm_import(
  name = "com_google_auto_value_auto_value_annotations",
  jars = ["com/google/auto/value/auto-value-annotations/1.10.1/auto-value-annotations-1.10.1-sources.jar"],
```

This pattern repeats for all the JAR targets.

The BUILD contents from v5.3, which builds successfully both before and
after applying the PR changes:

```bzl
jvm_import(
  name = "com_google_auto_value_auto_value_annotations",
  jars = ["com/google/auto/value/auto-value-annotations/1.10.1/auto-value-annotations-1.10.1.jar"],
  srcjar = "com/google/auto/value/auto-value-annotations/1.10.1/auto-value-annotations-1.10.1-sources.jar",
```

---------

Signed-off-by: Mike Bland <mbland@engflow.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haberman haberman Awaiting requested review from haberman haberman is a code owner automatically assigned from protocolbuffers/protobuf-btr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@joca-bt @zhangskz @haberman