Skip to content
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

[MRESOLVER-377] Introduce metadataUpdatePolicy #308

Merged
merged 6 commits into from
Oct 13, 2023

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Jun 29, 2023

Split the policies applied in case of data and metadata, allowing greater control of their updates. Still, work "as before" in case of applications that migrate from 1.x resolver (they should receive no behavior change).

The crux of this change is really just to introduce two policies (to co-exist in parallel), but if application using resolver decides to use same policy for both, the original behavior of resolver returns and will seemingly behave as before (ie. like 1.9.x does).

Maven relies on metadata in these cases ONLY:

  • resolving maven plugin prefix to maven plugin groupId (G level md)
  • resolving snapshot version to timestamped version (V level md)

Resolver OTOH provides one more use case:

  • "discovery" of existing versions for given GA (this use case is NOT used in Maven Core) (A level md)

Now, while Maven Core itself does NOT use 3rd use case, some plugins does, most notably the versions-maven-plugin. Today, everyone on Earth use this plugin along with -U Maven switch to pick up new stuff from remote repository, but this is total overkill, as it refreshes everything (yes, even the immutable release artifacts!). The -U is simply a "must", to make Maven "refresh metadata" (as well, along with all the Artifacts) to pick up new versions on remote. Versions plugin did implement a "hack" to overcome this limitation, that is still a hack, as it still applies to everything. Proper solution is to have means to express -U but for metadata only. This PR makes this possible on resolver side.


https://issues.apache.org/jira/browse/MRESOLVER-377

Split the policies applied in case of data and metadata
application.

---

https://issues.apache.org/jira/browse/MRESOLVER-377
@cstamas cstamas self-assigned this Jun 29, 2023
@@ -94,7 +94,7 @@ public interface RepositorySystemSession {
String getChecksumPolicy();

/**
* Gets the global update policy. If set, the global update policy overrides the update policies of the remote
* Gets the global data update policy. If set, the global update policy overrides the update policies of the remote
Copy link
Member

Choose a reason for hiding this comment

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

data or artifact?

Copy link
Member Author

Choose a reason for hiding this comment

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

IMHO, "data", as we call consequently the other "metadata" everywhere. Yes, in resolver, "data" is "artifact", but the point is, it does not have to be only that. Hence I went with generic "data" and "metadata".

@cstamas cstamas added this to the 1.9.14 milestone Jun 29, 2023
@gnodet
Copy link
Contributor

gnodet commented Jun 30, 2023

Shouldn't that be targeted for 1.10 ?

@cstamas
Copy link
Member Author

cstamas commented Jun 30, 2023

Really does not matter, as if you do updatePolicy == metadataUpdatePolicy, you are back to "old behaviour", like this PR did not exist even. But agree, it could go to 1.10 as well...

@cstamas cstamas modified the milestones: 1.9.14, 1.10.0 Jun 30, 2023
@cstamas cstamas requested a review from gnodet October 13, 2023 15:36
@cstamas
Copy link
Member Author

cstamas commented Oct 13, 2023

The crux of this change is really just to introduce two policies (in parallel), but if application using resolver decides to use same policy for both, the original behavior of resolver returns and will behave as before (ie. like 1.9.x does)

Instead, make javadoc clear this ctor enables "old" behaviour,
as maybe that is some wants. Also, migrated code will not get
deprecation warnings like this, as using this ctor is really
"legal" in a sense, is still okay to use it IF this is what
you want.
@cstamas cstamas merged commit aa3945e into apache:master Oct 13, 2023
10 checks passed
@cstamas cstamas deleted the MRESOVER-377 branch October 13, 2023 20:38
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.

4 participants