-
Notifications
You must be signed in to change notification settings - Fork 378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support fetching snapshot versions from a Maven registry #1160
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1160 +/- ##
==========================================
+ Coverage 65.86% 65.87% +0.01%
==========================================
Files 168 168
Lines 14076 14116 +40
==========================================
+ Hits 9271 9299 +28
- Misses 4293 4301 +8
- Partials 512 516 +4 ☔ View full report in Codecov by Sentry. |
maven-metadata.xml
from Maven Centralmaven-metadata.xml
from Maven Central
maven-metadata.xml
from Maven CentralThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - I've thought of a few corner cases that can be left as TODOs.
metadata, err := m.getVersionMetadata(ctx, groupID, artifactID, version) | ||
if err != nil { | ||
return maven.Project{}, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it valid for a snapshot version directory to not have a maven-metadata.xml
file? If it is, would maven just look for pkg-1.2.3-SNAPSHOT.pom
?
Conversely, is it possible for a non-snapshot version to have a metadata file with snapshotVersions
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if it is valid for a snapshot version to not have a metadata file.
Based on the link in the comment above, a non-snapshot version should not have a metadata file with snapshotVersion
.
if sv.Extension == "pom" { | ||
// We only look for pom.xml for project metadata. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know what happens if the pom
and jar
versions are mismatched?
e.g. foo-1.0.0-SNAPSHOT
has jar
= 1.0.0-1
and pom
= 1.0.0-2
.
Presumably, maven would download foo-1.0.0-1.jar
for my dependency, but would it look for foo-1.0.0-1.pom
or foo-1.0.0-2.pom
for its dependencies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know exact answer to this question. A snapshot version is the version in development before release and the actual version string is the timestamp + build number - I would expect them to be the same for the same snapshot version.
// GetProject fetches a pom.xml specified by groupID, artifactID and version and parses it to maven.Project. | ||
// For a snapshot version, version level metadata is used to find the extact version value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add some more information on how snapshot versions behave in maven (is there a doc you can link to?)
It took me a while (& looking at your tests & some maven repos) to try understand how it works.
From what I understand, when you depend on [email protected]
, it looks up in the registry pkg/1.2.3-SNAPSHOT/maven-metadata.xml
to determine which file to actually download, and downloads it from pkg/1.2.3-SNAPSHOT/1.2.3-realversion.pom
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes correct.
The closest official documentation I find is https://maven.apache.org/ref/3.9.9/maven-repository-metadata/ and https://maven.apache.org/repositories/metadata.html but the link to V-level metadata is not found...
I added both links to the comment.
good job |
#1127
Maven snapshot versions cannot be requested directly. Instead, we should request version level metadata first to get the exact version value and then request the pom.xml.
This PR supports fetching snapshot versions by: