gnodet-bot commented on code in PR #13604:
URL: https://github.com/apache/maven/pull/13604#discussion_r4304857674
##########
api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java:
##########
@@ -78,6 +78,26 @@ public interface MavenOptions extends Options {
@Nonnull
Optional<Boolean> suppressSnapshotUpdates();
+ /**
+ * The update policy to apply to artifacts ('always', 'daily', 'never').
+ * The --UA flag overrides this with 'always'.
+ *
+ * @return an {@link Optional} containing the artifacts update policy, or
empty if not specified
+ * @since 4.0.0
+ */
Review Comment:
**Wrong `@since` + missing `interval:N`**
Two issues here:
1. `@since 4.0.0` is incorrect — this PR targets `master` (4.1.0-SNAPSHOT).
Should be `@since 4.1.0`.
2. Resolver 2.x also accepts `interval:N` (minutes) as a valid policy value.
The Javadoc only lists three of the four valid values.
```suggestion
* The update policy to apply to artifacts ('always', 'daily',
'interval:N', 'never').
* The --UA flag overrides this with 'always'.
*
* @return an {@link Optional} containing the artifacts update policy,
or empty if not specified
* @since 4.1.0
*/
```
##########
impl/maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java:
##########
@@ -197,10 +197,22 @@ public SessionBuilder
newRepositorySessionBuilder(MavenExecutionRequest request)
sessionBuilder.setOffline(request.isOffline());
sessionBuilder.setChecksumPolicy(request.getGlobalChecksumPolicy());
- sessionBuilder.setUpdatePolicy(
- request.isNoSnapshotUpdates()
- ? RepositoryPolicy.UPDATE_POLICY_NEVER
- : request.isUpdateSnapshots() ?
RepositoryPolicy.UPDATE_POLICY_ALWAYS : null);
+
+ if (request.getArtifactsUpdatePolicy() == null &&
request.getMetadataUpdatePolicy() == null) {
+ // we go "old" way
+ sessionBuilder.setUpdatePolicy(
+ request.isNoSnapshotUpdates()
+ ? RepositoryPolicy.UPDATE_POLICY_NEVER
+ : request.isUpdateSnapshots() ?
RepositoryPolicy.UPDATE_POLICY_ALWAYS : null);
+ } else {
Review Comment:
⚠️ **High: Silent discard of `-U`/`-nsu` when new-style policy is set**
When the user passes `--artifacts-update-policy daily -nsu`, the `-nsu` flag
is silently ignored — the `else` branch only applies the non-null new-style
policies, completely bypassing `isNoSnapshotUpdates()` and
`isUpdateSnapshots()`. The user has no way to know their old-style flags are
being discarded.
Consider emitting a `WARN` log when old-style snapshot flags are present but
being ignored:
```suggestion
} else {
// New-style per-type policies take precedence; warn if
old-style flags were also set
if (request.isNoSnapshotUpdates() ||
request.isUpdateSnapshots()) {
logger.warn(
"Both new-style update policy options and legacy
snapshot flags (-U/-nsu) were specified. "
+ "The new-style policies take precedence;
the legacy flags are ignored.");
}
if (request.getArtifactsUpdatePolicy() != null) {
sessionBuilder.setArtifactUpdatePolicy(request.getArtifactsUpdatePolicy());
}
if (request.getMetadataUpdatePolicy() != null) {
sessionBuilder.setMetadataUpdatePolicy(request.getMetadataUpdatePolicy());
}
}
```
##########
impl/maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -258,6 +258,28 @@ public interface MavenExecutionRequest {
boolean isNoSnapshotUpdates();
+ // Resolver 2.x artifact and metadata update policies
+
+ /**
+ * @since 4.0.0
+ */
+ MavenExecutionRequest setArtifactsUpdatePolicy(String policy);
Review Comment:
All four new interface methods have only `@since 4.0.0` — should be `@since
4.1.0` (this PR targets master/4.1.0-SNAPSHOT, not maven-4.0.x). They're also
missing `@param` and `@return` Javadoc.
```suggestion
/**
* Sets the update policy to apply to artifact resolution.
* Valid values: {@code always}, {@code daily}, {@code interval:N}
(minutes), {@code never}.
*
* @param policy the update policy string, or {@code null} to use the
legacy snapshot flags
* @return this request
* @since 4.1.0
*/
MavenExecutionRequest setArtifactsUpdatePolicy(String policy);
/**
* Returns the update policy to apply to artifact resolution.
*
* @return the policy string, or {@code null} if the legacy snapshot
flags are used
* @since 4.1.0
*/
String getArtifactsUpdatePolicy();
/**
* Sets the update policy to apply to metadata resolution.
* Valid values: {@code always}, {@code daily}, {@code interval:N}
(minutes), {@code never}.
*
* @param policy the update policy string, or {@code null} to use the
legacy snapshot flags
* @return this request
* @since 4.1.0
*/
MavenExecutionRequest setMetadataUpdatePolicy(String policy);
/**
* Returns the update policy to apply to metadata resolution.
*
* @return the policy string, or {@code null} if the legacy snapshot
flags are used
* @since 4.1.0
*/
String getMetadataUpdatePolicy();
```
##########
compat/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java:
##########
@@ -340,6 +348,27 @@ public CLIManager() {
.longOpt("ignore-transitive-repositories")
.desc("If set, Maven will ignore remote repositories
introduced by transitive dependencies.")
.build());
+ options.addOption(Option.builder(UPDATE_METADATA)
+ .longOpt("update-metadata")
+ .desc(
+ "Forces updates of remote repository metadata (updates
plugin prefixes, version ranges and snapshots)")
+ .build());
+ options.addOption(Option.builder(UPDATE_ARTIFACTS)
+ .longOpt("update-artifacts")
+ .desc("Forces checks for missing artifacts (retries cached
retrieval errors)")
+ .build());
+ options.addOption(Option.builder()
+ .longOpt(ARTIFACTS_UPDATE_POLICY)
+ .hasArg(true)
+ .desc(
+ "The update policy to apply onto artifacts ('always',
'daily', 'never'); The -UA option overrides this option with 'always'")
+ .build());
Review Comment:
CLI help text is missing `'interval:N'` as a valid policy value — same gap
as the Javadoc. Resolver 2.x accepts `interval:N` (minutes).
```suggestion
"The update policy to apply onto artifacts
('always', 'daily', 'interval:N', 'never'); The -UA option overrides this
option with 'always'")
```
##########
api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java:
##########
@@ -78,6 +78,26 @@ public interface MavenOptions extends Options {
@Nonnull
Optional<Boolean> suppressSnapshotUpdates();
+ /**
+ * The update policy to apply to artifacts ('always', 'daily', 'never').
+ * The --UA flag overrides this with 'always'.
+ *
+ * @return an {@link Optional} containing the artifacts update policy, or
empty if not specified
+ * @since 4.0.0
+ */
+ @Nonnull
+ Optional<String> artifactsUpdatePolicy();
+
+ /**
+ * The update policy to apply to metadata ('always', 'daily', 'never').
+ * The --UM flag overrides this with 'always'.
+ *
+ * @return an {@link Optional} containing the metadata update policy, or
empty if not specified
+ * @since 4.0.0
Review Comment:
Same issues: `@since 4.0.0` → `@since 4.1.0`, and missing `'interval:N'` in
the valid values list.
```suggestion
* The update policy to apply to metadata ('always', 'daily',
'interval:N', 'never').
* The --UM flag overrides this with 'always'.
*
* @return an {@link Optional} containing the metadata update policy, or
empty if not specified
* @since 4.1.0
*/
```
##########
compat/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java:
##########
@@ -340,6 +348,27 @@ public CLIManager() {
.longOpt("ignore-transitive-repositories")
.desc("If set, Maven will ignore remote repositories
introduced by transitive dependencies.")
.build());
+ options.addOption(Option.builder(UPDATE_METADATA)
+ .longOpt("update-metadata")
+ .desc(
+ "Forces updates of remote repository metadata (updates
plugin prefixes, version ranges and snapshots)")
+ .build());
+ options.addOption(Option.builder(UPDATE_ARTIFACTS)
+ .longOpt("update-artifacts")
+ .desc("Forces checks for missing artifacts (retries cached
retrieval errors)")
+ .build());
+ options.addOption(Option.builder()
+ .longOpt(ARTIFACTS_UPDATE_POLICY)
+ .hasArg(true)
+ .desc(
+ "The update policy to apply onto artifacts ('always',
'daily', 'never'); The -UA option overrides this option with 'always'")
+ .build());
+ options.addOption(Option.builder()
+ .longOpt(METADATA_UPDATE_POLICY)
+ .hasArg(true)
+ .desc(
+ "The update policy to apply onto metadata ('always',
'daily', 'never'); The -UM option overrides this option with 'always'")
+ .build());
Review Comment:
Same: add `'interval:N'` to the metadata policy help text.
```suggestion
"The update policy to apply onto metadata ('always',
'daily', 'interval:N', 'never'); The -UM option overrides this option with
'always'")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
