mojohaus / versions

Versions Maven Plugin
https://www.mojohaus.org/versions/versions-maven-plugin/
Apache License 2.0
337 stars 267 forks source link

Alter session for version discovery #965

Closed cstamas closed 1 year ago

cstamas commented 1 year ago

Alter session used for discovery, pinpoint ONLY to processed artifact to update ALWAYS. This would fix all the problems reported here https://github.com/mojohaus/versions/issues/959 and here https://issues.apache.org/jira/browse/MRESOLVER-363 just like -U does (and people do it by reflex). But this is more correct way, as -U is global and refreshes too much.

cstamas commented 1 year ago

@slawekjaranowski @slachiewicz @ajarmoniuk ping

jarmoniuk commented 1 year ago

I wish I were a maintainer, but for the time being I can't approve the workflow 😉

EDIT I can actually.

cstamas commented 1 year ago

Am not a maintainer either here, so I will just leave this here for @slawekjaranowski or anyone else to handle. Comment may be removed, I just added it like "head up", but it may happen in near or far future (I cannot tell).

slawekjaranowski commented 1 year ago

I'm not sure if it is good way, after it every execute plugin will cause download every metadata again.

For projects with many dependencies it can be an issue.

cstamas commented 1 year ago

@slawekjaranowski every metadata? only targeted, only artifact that is in call. Maybe vrsions plugin could control, and have "own" -U instead? To reiterate: only plugins (for display-plugin-updates) or dependencies (for display-plugin-dependencies) are affected, nothing else. IMHO, it is up to user, will he invoke this on some top of the reactor, of locally for single module. One thing for sure, this is much-much better than invoking plugin with mvn -U

Point with mvn -U is that it is global, so will download "every metadata maven touches" (so literally EVERY in build), while this change downloads ONLY the metadata for artifact

cstamas commented 1 year ago

@slawekjaranowski or if I misunderstood you, and you mean "will refresh metadata for maven-clean-plugin in EVERY module of the possibly 100 module build? so perform 100 refreshes?" That is not the case, as resolver refreshes once per session, so yes, it will refresh once in first module clean plugin is found, and from then on (in remaining 99 modules) cached results are used.

See https://github.com/apache/maven-resolver/blob/maven-resolver-1.9.10/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java#L267

slawekjaranowski commented 1 year ago

As I see issue from #959 is that we don't have or is wrong property for last modified in properties file. According to https://github.com/apache/maven-resolver/pull/292/files#diff-51ca9aed78d3713d9818b753db3b9e9629743a7e773292e686f93950fca4c2b0R103 in such case default policy for repository is used.

Because default central plugin repository has policy never - https://github.com/apache/maven/blob/6f136ef4d2b4938e939189e9d2e3e0faa11a20e9/maven-model-builder/src/main/resources/org/apache/maven/model/pom-4.0.0.xml#L53 we try to override policy for whole session for all repositories.

Last I did improvement for repository with never policy to use daily #957 - maybe it will be good enough is such case.

slawekjaranowski commented 1 year ago

I try to remove properties from resolver-status.properties

version 2.15.0 - doesn't refresh metadata current master version - refresh once and update resolver-status.properties

cstamas commented 1 year ago

Cool then, was unaware that this plugin overrides configured policy. I am still convinced, that is PR (conditionally, like some -Dforce flag) would be useful for plugin (as aside of daily, user has no other means to say "update now!" than mvn -U which is too much).

But am fine, so if it works as you tested, am fine with it.