Skip to content

chore(ci): Bom-Content-Test checks local Maven cache first#13641

Merged
lqiu96 merged 5 commits into
mainfrom
fix-bom-content-test-local-check
Jul 6, 2026
Merged

chore(ci): Bom-Content-Test checks local Maven cache first#13641
lqiu96 merged 5 commits into
mainfrom
fix-bom-content-test-local-check

Conversation

@lqiu96

@lqiu96 lqiu96 commented Jul 1, 2026

Copy link
Copy Markdown
Member

Modify BomContentTest to check if an artifact exists in the local Maven repository (.m2) before checking its reachability on Maven Central.

During release PRs, new versions of libraries are built and installed locally (via pre-install.sh and validate-bom canary project) but are not yet published to Maven Central. This change allows the reachability check to pass
for these new versions if they are found in the local cache, while still verifying that other dependencies (which should be on Central) are reachable.

This works with standard GHA runner setups (which use ~/.m2/repository by default, cached by actions/setup-java@v4 with cache: 'maven') and also respects custom local repository locations if maven.repo.local is set. This removes the need for conditional flags in GHA workflows.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces a local cache check to skip Maven Central reachability verification for artifacts that are already present in the local Maven repository cache, which is particularly useful during release PRs. The review feedback suggests defensively handling cases where the user.home system property is null to prevent constructing an invalid path like "null/.m2/repository".

Comment on lines +155 to +159
String localRepository = System.getProperty("maven.repo.local");
if (localRepository == null) {
// Standard default location for Maven local repository.
localRepository = System.getProperty("user.home") + "/.m2/repository";
}

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.

medium

If the user.home system property is not set (which can happen in certain restricted or sandboxed environments), System.getProperty("user.home") will return null. Appending it to a string results in "null/.m2/repository", which will point to an unintended relative path. It is safer to check if userHome is null and handle it defensively.

    String localRepository = System.getProperty("maven.repo.local");
    if (localRepository == null) {
      String userHome = System.getProperty("user.home");
      if (userHome == null) {
        return false;
      }
      // Standard default location for Maven local repository.
      localRepository = userHome + "/.m2/repository";
    }

lqiu96 added 4 commits July 6, 2026 14:55
Modify BomContentTest to check if an artifact exists in the local Maven
repository (.m2) before checking its reachability on Maven Central.

During release PRs, new versions of libraries are built and installed locally
(via pre-install.sh) but are not yet published to Maven Central. This
change allows the reachability check to pass for these new versions if they
are found in the local cache, while still verifying that other dependencies
(which should be on Central) are reachable.

This removes the need for conditional flags in GHA workflows.
@lqiu96
lqiu96 force-pushed the fix-bom-content-test-local-check branch from 719fbe0 to 8fd4ac0 Compare July 6, 2026 14:55
@lqiu96
lqiu96 marked this pull request as ready for review July 6, 2026 14:57
@lqiu96
lqiu96 requested a review from a team as a code owner July 6, 2026 14:57
@lqiu96
lqiu96 requested review from blakeli0 and jinseopkim0 July 6, 2026 14:57
@lqiu96
lqiu96 enabled auto-merge (squash) July 6, 2026 15:11
@lqiu96
lqiu96 merged commit 3a9d5d7 into main Jul 6, 2026
201 of 202 checks passed
@lqiu96
lqiu96 deleted the fix-bom-content-test-local-check branch July 6, 2026 15:43
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