Skip to content

fix: Do not add coursier opts when run other tools#1531

Merged
shs96c merged 1 commit into
bazel-contrib:masterfrom
honnix:honnix/fix-java-opts
Feb 3, 2026
Merged

fix: Do not add coursier opts when run other tools#1531
shs96c merged 1 commit into
bazel-contrib:masterfrom
honnix:honnix/fix-java-opts

Conversation

@honnix

@honnix honnix commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

_generate_java_jar_command is used in a few other places than calling coursier, so adding COURSIER_OPTS doesn't make sense.

@honnix honnix force-pushed the honnix/fix-java-opts branch from 2aa216c to efa1c3b Compare January 22, 2026 13:56
@honnix honnix force-pushed the honnix/fix-java-opts branch from efa1c3b to 89b7bd9 Compare January 22, 2026 13:57
java_path = _java_path(repository_ctx)

if java_path != None:
cmd = [java_path, "-jar"] + _get_java_proxy_args(repository_ctx) + [jar_path]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure -noverify is still needed as it has been deprecated since Java 13.

OpenJDK 64-Bit Server VM warning: Options -Xverify:none and -noverify were deprecated in JDK 13 and will likely be removed in a future release.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We're an old project, so it's likely this is left over from the early days. However, we do want to continue to support Java 11 onwards, so we should only remove this flag if it won't compromise our ability to run there.

@honnix honnix marked this pull request as ready for review January 22, 2026 14:11

@shs96c shs96c left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Thank you for the PR

java_path = _java_path(repository_ctx)

if java_path != None:
cmd = [java_path, "-jar"] + _get_java_proxy_args(repository_ctx) + [jar_path]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We're an old project, so it's likely this is left over from the early days. However, we do want to continue to support Java 11 onwards, so we should only remove this flag if it won't compromise our ability to run there.

@shs96c shs96c merged commit e43fffe into bazel-contrib:master Feb 3, 2026
6 checks passed
shs96c added a commit to shs96c/rules_jvm_external that referenced this pull request Feb 3, 2026
* master:
  Add presubmit check for prebuilt jars (bazel-contrib#1486)
  Upload artifacts in parallel (address artifactorys "Maven Snapshot Version Behaviour") (bazel-contrib#1524)
  feat: Support COURSIER_SHA256 environment variable (bazel-contrib#1527)
  fix: Do not add coursier opts when run other tools (bazel-contrib#1531)
  fix: add string attributes to `amend_artifact` for explicit unset state (bazel-contrib#1499)
  fix: use forward slash separator in Maven purl format (bazel-contrib#1530)
  Load rules from specific bzl files and add sh_test imports (bazel-contrib#1529)
  Added non-conflicting hash for install files (bazel-contrib#1454)
  Update the maven and coursier resolver tests to create a class index file. (bazel-contrib#1519)
  [ci] Drop Bazel 6 and ensure we run on Bazel 7 and 8 (bazel-contrib#1525)
  Only allow modules specified in known_contributing_modules to contribute artifacts or boms to the root module (bazel-contrib#1523)
  [gradle] Fix false resolution failures when BOM upgrades dependency version (bazel-contrib#1520)
@honnix honnix deleted the honnix/fix-java-opts branch February 3, 2026 12:38
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.

2 participants