Skip to content

Fix issue where MavenPublisher hangs indefinitely#1444

Merged
shs96c merged 3 commits into
bazel-contrib:masterfrom
confluentinc:vinnybod/upstream-publishing-fixes
Oct 6, 2025
Merged

Fix issue where MavenPublisher hangs indefinitely#1444
shs96c merged 3 commits into
bazel-contrib:masterfrom
confluentinc:vinnybod/upstream-publishing-fixes

Conversation

@vinnybod
Copy link
Copy Markdown
Contributor

@vinnybod vinnybod commented Sep 2, 2025

The previous pull request #1260 introduced an issue where publishing could hang indefinitely because it integrated the existing HttpDownloader into MavenPublisher. HttpDownloader contained an EventListener that was not being closed.

This updates the HttpDownloader to be auto-closeable and it closes the listener on close. MavenPublisher now uses it with a try-with-resources block.

There are now some end to end tests for MavenPublisher that tests local and http publishing. To make testing easier, we refactored the MavenPublisher entrypoint so that there is a run function that doesn't need to read from environment variables.

signingMetadata,
executor));

if (!Strings.isNullOrEmpty(extraArtifacts)) {
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.

empty string being passed for extraArtifacts was causing an IndexOutOfBounds exception

Comment on lines +102 to 103
} else if (termAvailable && consoleAvailable || System.getenv("FORCE_ANSI") != null) {
return new AnsiConsoleListener();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wanted to test the AnsiConsoleListener in a test to validate the cleanup; unfortunately Bazel test runner calls System.exit making it difficult to test it. [slack]

credentials,
in_memory_pgp_sign(
item, signingMetadata.signingKey, signingMetadata.signingPassword)));
in_memory_pgp_sign(item, signingMetadata.signingKey, signingMetadata.signingPassword),
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.

Note to self: refactor this method name.

Comment thread private/rules/coursier.bzl
@vinnybod vinnybod requested review from fzakaria and shs96c September 12, 2025 21:03
Copy link
Copy Markdown
Collaborator

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

Approving and merging. Will do a follow up PR to address nits, but that won't block this.

)

java_library(
name = "MavenPublisherLib",
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.

Only binaries get CamelCased names. maven would be a find name for this target

@shs96c shs96c merged commit 5e69c1e into bazel-contrib:master Oct 6, 2025
6 checks passed
@vinnybod vinnybod deleted the vinnybod/upstream-publishing-fixes branch December 16, 2025 20:32
simonresch added a commit to CodeIntelligenceTesting/jazzer that referenced this pull request Dec 22, 2025
bazel-contrib/rules_jvm_external#1444 changed
the command line usage of the MavenPublisher class which now requires a
forth boolean argument to configure `publishMavenMetadata`.
simonresch added a commit to CodeIntelligenceTesting/jazzer that referenced this pull request Dec 22, 2025
bazel-contrib/rules_jvm_external#1444 changed
the command line usage of the MavenPublisher class which now requires a
forth boolean argument to configure `publishMavenMetadata`.
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