#299: allowAnyUpdates should be ignored #941

andrzejj0 · 2023-04-09T13:12:04Z

This might be seen as a breaking change, but I think it is in line with what most users expect to happen anyway.

So, this change sanctions these expectations -- allowAnyUpdates shall effectively assume the value of false if any of the other modifies are set to false.

Actually, this means that it's redundant, but the fact is that it's getting removed in version 3.0 anyway.

@slawekjaranowski what is your opinion?

andrzejj0 · 2023-04-09T13:14:58Z

Alternatively, we could simply log a warning message if the user sets both "allowAnyUpdates" to true and one of the other modifies to false.

andrzejj0 · 2023-04-10T19:44:55Z

Looks like this is also in line with what users expect – see the latest comments in #299.

…e if any of: allowMajorUpdates, allowMinorUpdates, allowIncrementalUpdates is set to false

slawekjaranowski · 2023-05-18T18:02:41Z

...ions-maven-plugin/src/main/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojo.java

+                && allowAnyUpdates
+                && !(allowMajorUpdates && allowMinorUpdates && allowIncrementalUpdates)) {
+            getLog().warn("Assuming allowAnyUpdates false because one or more other \"allow\" switches is false.");


allowAnyUpdates has default value as true so when user provide only one allow* switch this warning will be always displayed.
As now we ignore at all allowAnyUpdates maybe it is not needed.

As now we ignore at all allowAnyUpdates maybe it is not needed.

The problem is that we do not ignore allowAnyUpdates -- we ignore all the other allow options if allowAnyUpdates is true. Which it is by default.

See:

versions/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojo.java

Line 514 in 0fe821e

? empty()

That is basically the reason why I created this PR.

In other words, if the user only sets allowMajorUpdates=false, that option will be ignored because the user also must use allowAnyUpdates=false together with that option for it to work... Which is flawed in my opinion.

I see that after this change allowAnyUpdates will be ignored - so it will be not important what user set - right?

Yes, with a warning message.

ok, but settings or not allowAnyUpdates will mo have any effect only warning message will be or not displayed
So I think that warning is such case is redundant
I don't see any more usage of allowAnyUpdates ...

I don't see it either.

Ideally, I would remove it.

Should we? Then we could simply use the SegmentUtils method used elsewhere.

Do it. allowAnyUpdates is only used in this goal. Will be more clear.

Okay. How about a small refactoring job?

There's an inconsistency:

SegmentUtils.determineUnchangedSegment will return the first segment that may not change

VersionDetails.restrictionFor expects the first segment that may change.

So let's make parts that rely on these use the same thing and so we can get rid of this incrementing of the segement in some places (DisplayDependencyUpdates). But that will take some time (maybe one day, maybe two). Agreed?

Otherwise we can postpone this to the next release.

agree - we can wait a few days.
I would to release next version at next week - about 25 - 26 of May

...ions-maven-plugin/src/main/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojo.java

andrzejj0 · 2023-05-22T06:04:57Z

While working on the refactoring job, I've discovered a bug around allowAnyUpdates: if allowAnyUpdates is false and all other allows are true, the result differs from when allowAnyUpdates is true.

With allowAnyUpdates for this project:

in this case, update scope is Optional.empty(), all possible updates will be displayed

The following dependencies in Dependency Management have newer versions:
  dom4j:dom4j ................................. 1.6.1 -> 20040902.021138
  org.apache.maven:maven-artifact ............... 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-compat ................. 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-core ................... 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-model .................. 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-plugin-api ............. 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-settings ............... 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven.doxia:doxia-core ................. 1.12.0 -> 2.0.0-M6
  org.apache.maven.doxia:doxia-integration-tools ... 1.11.1 -> 2.0.0-M10
  org.apache.maven.doxia:doxia-sink-api ............. 1.12.0 -> 2.0.0-M6
  org.apache.maven.doxia:doxia-site-renderer ....... 1.11.1 -> 2.0.0-M10
  org.apache.maven.enforcer:enforcer-api ................ 3.2.1 -> 3.3.0
  org.apache.maven.plugin-testing:maven-plugin-testing-harness ...
                                                  3.3.0 -> 4.0.0-alpha-1
  org.apache.maven.plugin-tools:maven-plugin-annotations ...
                                                          3.8.1 -> 3.9.0
  org.apache.maven.reporting:maven-reporting-api ..... 3.1.1 -> 4.0.0-M6
  org.apache.maven.reporting:maven-reporting-impl .... 3.2.0 -> 4.0.0-M8
  org.junit.jupiter:junit-jupiter ................... 5.9.3 -> 5.10.0-M1
  org.junit.jupiter:junit-jupiter-api ............... 5.9.3 -> 5.10.0-M1
  org.junit.jupiter:junit-jupiter-engine ............ 5.9.3 -> 5.10.0-M1
  org.junit.jupiter:junit-jupiter-migrationsupport ...
                                                      5.9.3 -> 5.10.0-M1
  org.junit.jupiter:junit-jupiter-params ............ 5.9.3 -> 5.10.0-M1
  org.junit.platform:junit-platform-commons ......... 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-console ......... 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-engine .......... 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-jfr ............. 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-launcher ........ 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-reporting ....... 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-runner .......... 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-suite ........... 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-suite-api ....... 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-suite-commons ... 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-suite-engine .... 1.9.3 -> 1.10.0-M1
  org.junit.platform:junit-platform-testkit ......... 1.9.3 -> 1.10.0-M1
  org.junit.vintage:junit-vintage-engine ............ 5.9.3 -> 5.10.0-M1
  org.mockito:mockito-inline ........................... 4.11.0 -> 5.2.0
  org.slf4j:slf4j-simple ............................... 1.7.36 -> 2.0.7

The following dependencies in pluginManagement of plugins have newer versions:
  com.puppycrawl.tools:checkstyle ....................... 9.3 -> 10.11.0

The following dependencies in Plugin Dependencies have newer versions:
  com.puppycrawl.tools:checkstyle ....................... 9.3 -> 10.11.0

and without:

in the first case, the updateScope is Optional.of(MAJOR) (which means that only updates
in the MAJOR segment will be displayed)

The following dependencies in Dependency Management have newer versions:
  dom4j:dom4j ................................. 1.6.1 -> 20040902.021138
  org.apache.maven:maven-artifact ............... 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-compat ................. 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-core ................... 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-model .................. 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-plugin-api ............. 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven:maven-settings ............... 3.2.5 -> 4.0.0-alpha-5
  org.apache.maven.doxia:doxia-core ................. 1.12.0 -> 2.0.0-M6
  org.apache.maven.doxia:doxia-integration-tools ... 1.11.1 -> 2.0.0-M10
  org.apache.maven.doxia:doxia-sink-api ............. 1.12.0 -> 2.0.0-M6
  org.apache.maven.doxia:doxia-site-renderer ....... 1.11.1 -> 2.0.0-M10
  org.apache.maven.plugin-testing:maven-plugin-testing-harness ...
                                                  3.3.0 -> 4.0.0-alpha-1
  org.apache.maven.reporting:maven-reporting-api ..... 3.1.1 -> 4.0.0-M6
  org.apache.maven.reporting:maven-reporting-impl .... 3.2.0 -> 4.0.0-M8
  org.mockito:mockito-inline ........................... 4.11.0 -> 5.2.0
  org.slf4j:slf4j-simple ............................... 1.7.36 -> 2.0.7

The following dependencies in pluginManagement of plugins have newer versions:
  com.puppycrawl.tools:checkstyle ....................... 9.3 -> 10.11.0

The following dependencies in Plugin Dependencies have newer versions:
  com.puppycrawl.tools:checkstyle ....................... 9.3 -> 10.11.0

That means that allow... options only display updates from the segment which is the first one which allows updates, but not from any lesser segments.

Strange that we haven't found it with our tests. Well, I'm fixing this now (we'll get rid of allowAnyUpdates), but this will take some time.

Also, adding more tests...

-> #960

will include this refactoring job in a fix for that. I'll keep this PR open until that is done.

slawekjaranowski · 2023-05-23T06:10:43Z

Looks more complicated ... so I have proposition to postpone this and related issues to next release.

andrzejj0 · 2023-05-23T06:12:33Z

Actually, I'd close this PR and I would postpone #961.

On the other hand, #960 is important and almost ready.

andrzejj0 · 2023-05-23T06:25:25Z

On the other hand, #960 is important and almost ready.

Not ready, I see that integration tests are failing, so there's more work needed. Also, I need to see if this is restricted to display-dependency-updates. So, let's postponse that one too.

andrzejj0 mentioned this pull request Apr 10, 2023

allow* parameters not working on display-dependency-updates #299

Closed

andrzejj0 changed the title ~~#299: allowAnyUpdates should be ignored if any of: allowMajorUpdates, allowMinorUpdates, allowIncrementalUpdates is set to false~~ #299: allowAnyUpdates should be ignored Apr 10, 2023

andrzejj0 force-pushed the issue-299 branch from c111aad to 1a4148b Compare April 10, 2023 21:32

mojohaus#299: allowAnyUpdates should be ignored with a warning messag…

dcf704e

…e if any of: allowMajorUpdates, allowMinorUpdates, allowIncrementalUpdates is set to false

andrzejj0 force-pushed the issue-299 branch from 916a932 to dcf704e Compare April 28, 2023 13:47

slawekjaranowski added the enhancement label May 9, 2023

slawekjaranowski added this to the 2.16.0 milestone May 9, 2023

slawekjaranowski reviewed May 18, 2023

View reviewed changes

...ions-maven-plugin/src/main/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojo.java Show resolved Hide resolved

slawekjaranowski reviewed May 18, 2023

View reviewed changes

...ions-maven-plugin/src/main/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojo.java Show resolved Hide resolved

andrzejj0 closed this May 23, 2023

slawekjaranowski removed this from the 2.16.0 milestone May 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#299: allowAnyUpdates should be ignored #941

#299: allowAnyUpdates should be ignored #941

andrzejj0 commented Apr 9, 2023 •

edited

Loading

andrzejj0 commented Apr 9, 2023

andrzejj0 commented Apr 10, 2023

slawekjaranowski May 18, 2023

andrzejj0 May 18, 2023 •

edited

Loading

andrzejj0 May 18, 2023

slawekjaranowski May 18, 2023

andrzejj0 May 18, 2023

slawekjaranowski May 18, 2023

andrzejj0 May 19, 2023 •

edited

Loading

slawekjaranowski May 19, 2023

andrzejj0 May 19, 2023

slawekjaranowski May 19, 2023

andrzejj0 commented May 22, 2023 •

edited

Loading

slawekjaranowski commented May 23, 2023

andrzejj0 commented May 23, 2023

andrzejj0 commented May 23, 2023

#299: allowAnyUpdates should be ignored #941

#299: allowAnyUpdates should be ignored #941

Conversation

andrzejj0 commented Apr 9, 2023 • edited Loading

andrzejj0 commented Apr 9, 2023

andrzejj0 commented Apr 10, 2023

Choose a reason for hiding this comment

andrzejj0 May 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrzejj0 May 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrzejj0 commented May 22, 2023 • edited Loading

slawekjaranowski commented May 23, 2023

andrzejj0 commented May 23, 2023

andrzejj0 commented May 23, 2023

andrzejj0 commented Apr 9, 2023 •

edited

Loading

andrzejj0 May 18, 2023 •

edited

Loading

andrzejj0 May 19, 2023 •

edited

Loading

andrzejj0 commented May 22, 2023 •

edited

Loading