Add artifact sink generating module-uri properties#397
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a ChangesModule Properties Sink Operation
Sequence DiagramsequenceDiagram
participant Builder as ArtifactSinkBuilder
participant Sink as ModulePropertiesArtifactSink
participant ModuleExtractor as ModuleDescriptorExtractingSink
participant UriResolver as ArtifactUriSink
participant Checksum as ChecksumArtifactSink
participant Output as Output
Builder->>Builder: recognize modules operation
Builder->>Sink: accept(artifact)
Sink->>ModuleExtractor: extract module name from .jar
Sink->>UriResolver: lookup artifact origin URI
Sink->>Checksum: require SHA-1 and size
Sink->>Sink: store in concurrent map by module name
Sink->>Output: close() writes sorted module=origin#SIZE=...&SHA-1=... lines
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@shared/src/main/java/eu/maveniverse/maven/toolbox/shared/internal/ArtifactSinks.java`:
- Line 631: The code calls Files.write(file, lines) without ensuring the file's
parent directory exists (in ArtifactSinks where Files.write is invoked), which
will fail for non-existent directories; before writing, check if
file.getParent() is non-null and call Files.createDirectories(file.getParent())
to create any missing parent directories so Files.write succeeds.
- Around line 617-621: Guard building Property entries by first verifying
moduleArtifactSink.getModuleDescriptor(artifact) is present (non-null/optional)
and that artifactUriSink.getUri(artifact) and
checksumArtifactSink.checksums(artifact).get("SHA-1") are non-null; if any of
those are missing, skip adding the entry (or return early/log debug) instead of
calling .name() or constructing a Property with nulls. Update the code around
moduleArtifactSink.getModuleDescriptor, artifactUriSink.getUri,
checksumArtifactSink.checksums and the propertyByModuleName.put calls to perform
these null/optional checks before creating new Property.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c09901f8-82b2-4bdc-9f4a-66dfae032b37
📒 Files selected for processing (1)
shared/src/main/java/eu/maveniverse/maven/toolbox/shared/internal/ArtifactSinks.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@shared/src/main/java/eu/maveniverse/maven/toolbox/shared/internal/ArtifactSinks.java`:
- Around line 742-743: The two-line assignment of ModuleDescriptor from
moduleDescriptorExtractingSink.getModuleDescriptor(artifact) violates Spotless
formatting; reformat that statement into a single line or the project's
canonical multi-line style to satisfy Spotless (e.g., align the type, variable
and assignment per project conventions) by editing the ModuleDescriptor
descriptor = moduleDescriptorExtractingSink.getModuleDescriptor(artifact)
statement so it matches surrounding formatting or run mvn spotless:apply to
auto-fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd61417d-8e68-42c3-8b72-79d4c6d7971f
📒 Files selected for processing (1)
shared/src/main/java/eu/maveniverse/maven/toolbox/shared/internal/ArtifactSinks.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@shared/src/main/java/eu/maveniverse/maven/toolbox/shared/internal/ArtifactSinks.java`:
- Around line 121-131: The "modules" branch in ArtifactSinks silently ignores
extra arguments; change it to validate maximum arity and fail fast: after
checking minimum arity in the case "modules" block, add a guard that throws an
IllegalArgumentException if node.getChildren().size() > 2 (since
modules(<output>[, <classifier>]) allows at most two arguments); keep
constructing Path file, classifier, and calling
modulePropertiesArtifactSink(file, classifier, tc.output(), tc) only when size
is 1 or 2, and then clear node.getChildren() as before. Ensure the exception
message clearly states the allowed signature (e.g., "modules requires 1 or 2
arguments: output[, classifier]").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40398c02-8cee-48a2-ad33-b36e70e72bda
📒 Files selected for processing (1)
shared/src/main/java/eu/maveniverse/maven/toolbox/shared/internal/ArtifactSinks.java
|
just curious - what are module-uri properties? I haven't seen that format/style before. |
|
The format/style is described here: https://github.com/sormuras/modules Caveat: that "database" is dormant since summer 2025, due to Sonatype's shift in repository hosting. |
|
gotcha. it was the sha-1 / size approach I hadn't seen before - i dont see that described on that page? and side question - i was wondering where you got the module info from central on; as I was wondering if we could build up a index of jars with main methods - but seems your page is hinting sonatype doesn't expose such metadata anymore? |
Yes, that's a recent addition ... using URI's fragment syntax.
It's partially described the README; https://github.com/sandermak/modulescanner is run by Sonatype internally on "upload events" and fills an AWS S3 bucket with the scanner result. With the new/shifted publish root (storage?) for open source projects, the scanner only sees some old (unshifted) projects. Is that correct, @jorlina? |
Are project-local catalogs, like https://github.com/junit-team/jbang-catalog, not sufficient (for jbang)? |
|
While we are here and already derailed the review thread ... takes some seconds to load and might not be useful; yet, still pretty: |
|
definitely pretty 3d graph :) jbang-catalog is great but thats like 0.1% of jars with main methods that are added there :)
|
3 participants
This change adds an artifact sink that prints and generates a
.propertiesfile with modul-uri entries.Usage example:
Yields:
Summary by CodeRabbit
modulesoperation that writes per-module property files for modular JARs (requires an output path, optional classifier). Each line maps module name to origin URI, file size and SHA-1; non-modular artifacts are skipped. Output is sorted and a completion summary is emitted.