RTECO-1646 - Add tests for BuildInfo properties and enhance MavenFlexPack functionality by fluxxBot · Pull Request #408 · jfrog/build-info-go

fluxxBot · 2026-08-03T10:19:55Z

All tests passed. If this feature is not already covered by the tests, I added new tests.
All static analysis checks passed.
This pull request is on the dev branch.
I used gofmt for formatting the code before submitting the pull request.
Appropriate label is added to the PR for auto generate release notes.

Implement TestAppendProperties to verify the behavior of the Append method in BuildInfo, ensuring it fills gaps without clobbering existing properties and initializes nil target properties.
Enhance MavenFlexPack with additional fields for module locations and checksum caching to optimize artifact handling.
Introduce methods for resolving the local Maven repository path and collecting build information from multi-module projects.
Add comprehensive tests for MavenFlexPack, including dependency graph inversion, module dependency collection, and effective POM parsing.
Ensure that deployment repository resolution is robust, handling various scenarios for snapshot and release versions.

…Pack functionality - Implement TestAppendProperties to verify the behavior of the Append method in BuildInfo, ensuring it fills gaps without clobbering existing properties and initializes nil target properties. - Enhance MavenFlexPack with additional fields for module locations and checksum caching to optimize artifact handling. - Introduce methods for resolving the local Maven repository path and collecting build information from multi-module projects. - Add comprehensive tests for MavenFlexPack, including dependency graph inversion, module dependency collection, and effective POM parsing. - Ensure that deployment repository resolution is robust, handling various scenarios for snapshot and release versions.

… version for consistent JSON output

…put handling

agrasth · 2026-08-27T05:56:12Z

+	if targetBuildInfo.Properties == nil {
+		targetBuildInfo.Properties = make(Env)
+	}
+	for key, value := range buildInfo.Properties {


Cross-cutting semantic change on a shared method. Append is used by every builder (Go, npm, Poetry, Ruby, ...), not just Maven. Looks like historically it never touched Properties. Now every caller that partially assembles a BuildInfo and calls Append picks up merged properties, with "first-writer-wins" semantics.

Example: two Append calls both contribute Properties["buildmode"] — the second is silently dropped. That's easy to misread as a bug.

agrasth · 2026-08-27T05:56:12Z

+	return mf.localRepo
+}
+
+func (mf *MavenFlexPack) resolveLocalRepositoryPath() string {


Silent correctness bug. resolveLocalRepositoryPath only inspects ExtraArgs for -Dmaven.repo.local=. It does not honor <localRepository> in ~/.m2/settings.xml or ${M2_HOME}/conf/settings.xml.

Example: a CI runner writes <localRepository>/opt/m2-cache</localRepository> into settings.xml (very common). Maven downloads there, FlexPack looks in ~/.m2/repository, finds nothing → every dependency's checksum comes out empty and the build-info is silently wrong.

Either parse settings.xml, or run mvn help:evaluate -Dexpression=settings.localRepository -q -DforceStdout once and cache the result.

agrasth · 2026-08-27T05:56:12Z

+// dependency:tree goal. The version is pinned because `-DoutputType=json` was only added in
+// maven-dependency-plugin 3.7.0; older versions silently write plain-text output which the JSON
+// parser then rejects. 3.8.1 (current latest) is used to also pick up bug fixes.
+const mavenDependencyPluginTreeGoal = "org.apache.maven.plugins:maven-dependency-plugin:3.8.1:tree"


Pinning maven-dependency-plugin:3.8.1 forces every collection to resolve that exact plugin coordinate from the configured Maven remote. Air-gapped / mirrored setups that haven't uploaded 3.8.1 now fail with "cannot resolve plugin" instead of using whatever the user already had cached.

Consider: try pinned first, fall back to unversioned dependency:tree on resolution failure.

agrasth · 2026-08-27T05:56:12Z

+//     the effective pom, so a reactor whose modules deploy to different repos is handled correctly.
+//
+// Both empty means no deployment repository is configured. Precedence follows Maven (override wins).
+func (mf *MavenFlexPack) GetDeploymentRepositories() (moduleURLs map[string]string, overrideURL string, err error) {


This path can trigger up to three separate mvn invocations per collection (dependency:tree, help:effective-pom, help:effective-settings). Each Maven cold-start is 10–30s on a large reactor.

Combine into one: mvn help:effective-pom help:effective-settings dependency:tree -Doutput=... -DoutputFile=... shares a single JVM/plugin-resolution pass. Real UX difference on big multi-module builds.

agrasth · 2026-08-27T05:56:12Z

+	}
+	for _, profile := range settings.Profiles {
+		if isSnapshot && profile.AltSnapshotDeploymentRepository != "" {
+			return repoURLFromAltValue(profile.AltSnapshotDeploymentRepository), nil


First-match-wins is wrong for Maven profile precedence. Maven's actual rule: later active profiles override earlier ones.

Example: user has profile defaults (sets altDeploymentRepository=A) then profile ci (sets altDeploymentRepository=B). This loop returns A; real Maven deploys to B.

Reverse the iteration or track "last non-empty" and return that.

agrasth · 2026-08-27T05:56:12Z

+	}
+	if dm.Repository.URL != "" {
+		return dm.Repository.URL
+	}


return dm.SnapshotRepository.URL at the tail is unreachable for a release version unless <repository> is empty — in which case it silently returns the snapshot URL for a release deploy. That's a silent wrong-target push.

Return "" here and let the caller decide the fallback.

agrasth · 2026-08-27T05:56:12Z

+// convention and the legacy Maven build-info extractor. A diamond dependency (reached through more
+// than one parent) yields one path per distinct route. A cycle, which a resolved Maven tree should
+// never contain, is broken defensively by terminating the offending path at the repeated node.
+func buildRequestedByPaths(depID string, parents map[string][]string) [][]string {


Worst-case path count is O(product-of-parent-counts). A diamond-heavy Maven tree (BOM inheritance in a monorepo) can produce thousands of RequestedBy paths for a single dep.

Cap at ~15 paths (matches RequestedByMaxLength you already have) and drop the rest with a log.Debug.

agrasth · 2026-08-27T05:56:12Z

+
+// findDependencyTreeFiles walks the working directory and returns every maven-deps.json file, one per
+// reactor module. WalkDir yields lexical order, keeping module ordering deterministic across runs.
+func (mf *MavenFlexPack) findDependencyTreeFiles() ([]string, error) {


findDependencyTreeFiles walks the entire working directory. In a polyglot repo (Maven + JS + Python vendored deps) this becomes a lot of stat syscalls.

You already know the reactor from Maven — combine with the earlier mvn help:evaluate -Dexpression=project.modules (or the combined invocation suggested above) so paths are handed to you directly.

agrasth · 2026-08-27T05:56:12Z

-			checksumMap["sha1"] = sha1
-			checksumMap["sha256"] = sha256
-			checksumMap["md5"] = md5
+		cached, ok := mf.checksumCache[artifactPath]


checksumCache keys by artifactPath. On case-insensitive filesystems (macOS default, Windows) the same file can be reached via different-cased paths and cache-miss.

Normalize the key with filepath.Clean on all platforms; add strings.ToLower on darwin/windows.

agrasth · 2026-08-27T05:56:12Z

 	assert.True(t, results)
 }

+func TestAppendProperties(t *testing.T) {


Given how load-bearing the new Append semantics are, please add these edge cases:

Target Properties["x"] = "" (empty string), source has Properties["x"] = "v" — is empty considered "exists" (source dropped) or "gap" (source wins)? Current impl: exists wins; not asserted.

Chained appends: A.Append(B) then A.Append(C), both set the same key — B wins. Not asserted.

Source Properties is non-nil but empty (Env{}) — early return path at line 121 not exercised.

agrasth

Added comments

fluxxBot added the improvement Automatically generated release notes label Aug 3, 2026

fluxxBot requested a deployment to frogbot August 3, 2026 10:20 — with GitHub Actions Waiting

RTECO-1646 - Enhance MavenFlexPack by pinning maven-dependency-plugin…

71bd354

… version for consistent JSON output

fluxxBot requested a deployment to frogbot August 4, 2026 08:40 — with GitHub Actions Waiting

Update maven-dependency-plugin version to 3.8.1 for improved JSON out…

2e9f7c8

…put handling

fluxxBot had a problem deploying to frogbot August 4, 2026 09:19 — with GitHub Actions Failure

agrasth approved these changes Aug 27, 2026

View reviewed changes

agrasth reviewed Aug 27, 2026

View reviewed changes

Navigation Menu

Uh oh!

RTECO-1646 - Add tests for BuildInfo properties and enhance MavenFlexPack functionality - #408

RTECO-1646 - Add tests for BuildInfo properties and enhance MavenFlexPack functionality#408
fluxxBot wants to merge 3 commits into
mainfrom
RTECO-1646-mvnWorkspace

fluxxBot commented Aug 3, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth Aug 27, 2026

Uh oh!

agrasth left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

fluxxBot commented Aug 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agrasth left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants