MNG-6530 - Please fix or revert important regression from MNG-6311

[Mickael Istria]

Hi,

I'm writing that in the context of some maintenance work in Eclipse m2e.
Eclipse m2e likes to use the latest Maven release, but we identified (and
shared it publicly on the bugtracker before the 3.6.0 release) that
MNG-6311 was a risky change. We unfortunately didn't have the opportunity
to prove our point back then, which probably didn't help our concerns to be
taken into account seriously.
Still, now we have more time for it, and we demonstrated with a JUnit test
that MNG-6311, despite all the shiny effect of a performance improvement in
some cases, actually introduces a severe regression that makes all
downstream adopters keeping a ProjectBuilder instance for a long term usage
in a context where pom files can change (like m2e does) are basically
broken by the change as it will ignore modifications on pom files when
resolving dependencies. The test case I attached to MNG-6350 shows that by
reverting the commit introduced by MNG-6311, everything is back to green
and modifications of pom files are properly taken into account.
On MNG-6350, I've put some notes about what seems easily achievable after
1.5 day of work on this topic (which isn't much, especially for someone who
never wrote a line of code for Maven so far), but I have the impression no
proper fix can be easily reached (at least by me). So I'd like the Maven
community to 1. help finding a solution for MNG-6350 before the next
release, and 2. if no solution seems achievable for next release, consider
reverting MNG-6311 so the downstream community can safely adopt it.


Note that this is part of a longer story of improving the state of m2e in
general, both technically, in term of internal community and in term of
interaction with the Maven project. We are aiming at making m2e able to
work more and more closely from Maven snapshots and staging builds to
report such issues earlier and have the opportunity to prevent regressions
to happen in Maven.
We'll probably share more messages about it in the next weeks/months.

Cheers,

[Robert Scholte]

Post by Mickael Istria
Hi,
I'm writing that in the context of some maintenance work in Eclipse m2e.
Eclipse m2e likes to use the latest Maven release, but we identified (and
shared it publicly on the bugtracker before the 3.6.0 release) that
MNG-6311 was a risky change. We unfortunately didn't have the opportunity
to prove our point back then, which probably didn't help our concerns to be
taken into account seriously.

This is a bit too black/white to me.
Every software change could be considered a risky change and to improve
Maven we must make changes.
MNG-6311 is a response to MNG-6030 and based on the improve performance by
that patch it seemed like we were on the right way.
Should we not apply this patch for a *potential* risk for M2Eclipse?
Considering both pros and cons it does make sense to apply the patch.

Now we know that it did effect m2eclipse we need to move forward.
IMHO reverting is not the right answer.
As proposed in MNG-6530 in the worst case we need to introduce a system
property to control the cache, but I really hope we (Maven team and
Eclipse team) can cooperate to find a better solution.

thanks,
Robert

Post by Mickael Istria
Still, now we have more time for it, and we demonstrated with a JUnit test
that MNG-6311, despite all the shiny effect of a performance improvement in
some cases, actually introduces a severe regression that makes all
downstream adopters keeping a ProjectBuilder instance for a long term usage
in a context where pom files can change (like m2e does) are basically
broken by the change as it will ignore modifications on pom files when
resolving dependencies. The test case I attached to MNG-6350 shows that by
reverting the commit introduced by MNG-6311, everything is back to green
and modifications of pom files are properly taken into account.
On MNG-6350, I've put some notes about what seems easily achievable after
1.5 day of work on this topic (which isn't much, especially for someone who
never wrote a line of code for Maven so far), but I have the impression no
proper fix can be easily reached (at least by me). So I'd like the Maven
community to 1. help finding a solution for MNG-6350 before the next
release, and 2. if no solution seems achievable for next release, consider
reverting MNG-6311 so the downstream community can safely adopt it.
Note that this is part of a longer story of improving the state of m2e in
general, both technically, in term of internal community and in term of
interaction with the Maven project. We are aiming at making m2e able to
work more and more closely from Maven snapshots and staging builds to
report such issues earlier and have the opportunity to prevent
regressions
to happen in Maven.
We'll probably share more messages about it in the next weeks/months.
Cheers,

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-***@maven.apache.org
For additional commands, e-mail: dev-***@maven.apache.org

[Karl Heinz Marbaise]

Hi,

Post by Robert Scholte

Post by Mickael Istria
Hi,
I'm writing that in the context of some maintenance work in Eclipse m2e.
Eclipse m2e likes to use the latest Maven release, but we identified (and
shared it publicly on the bugtracker before the 3.6.0 release) that
MNG-6311 was a risky change. We unfortunately didn't have the opportunity
to prove our point back then, which probably didn't help our concerns to be
taken into account seriously.

This is a bit too black/white to me.
Every software change could be considered a risky change and to improve
Maven we must make changes.
MNG-6311 is a response to MNG-6030 and based on the improve performance
by that patch it seemed like we were on the right way.
Should we not apply this patch for a *potential* risk for M2Eclipse?
Considering both pros and cons it does make sense to apply the patch.
Now we know that it did effect m2eclipse we need to move forward.
IMHO reverting is not the right answer.
As proposed in MNG-6530 in the worst case we need to introduce a system
property to control the cache, but I really hope we (Maven team and
Eclipse team) can cooperate to find a better solution.

+1 agreed

Kind regards
Karl Heinz

Post by Robert Scholte
thanks,
Robert

Post by Mickael Istria
Still, now we have more time for it, and we demonstrated with a JUnit test
that MNG-6311, despite all the shiny effect of a performance
improvement in
some cases, actually introduces a severe regression that makes all
downstream adopters keeping a ProjectBuilder instance for a long term usage
in a context where pom files can change (like m2e does) are basically
broken by the change as it will ignore modifications on pom files when
resolving dependencies. The test case I attached to MNG-6350 shows that by
reverting the commit introduced by MNG-6311, everything is back to green
and modifications of pom files are properly taken into account.
On MNG-6350, I've put some notes about what seems easily achievable after
1.5 day of work on this topic (which isn't much, especially for someone who
never wrote a line of code for Maven so far), but I have the
impression no
proper fix can be easily reached (at least by me). So I'd like the Maven
community to 1. help finding a solution for MNG-6350 before the next
release, and 2. if no solution seems achievable for next release, consider
reverting MNG-6311 so the downstream community can safely adopt it.
Note that this is part of a longer story of improving the state of m2e in
general, both technically, in term of internal community and in term of
interaction with the Maven project. We are aiming at making m2e able to
work more and more closely from Maven snapshots and staging builds to
report such issues earlier and have the opportunity to prevent regressions
to happen in Maven.
We'll probably share more messages about it in the next weeks/months.
Cheers,

---------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-***@maven.apache.org
For additional commands, e-mail: dev-***@maven.apache.org

[Mickael Istria]

Hi guys,

Thanks for your answer. I like the proposal of the system property to
disable this bug. Let's continue discussion on the Jira ticket.

Cheers,

[Mickael Istria]

Hi guys,

Another kind of projects (after IDEs) that may be affected by this issue
and receiving out of date MavenProject from Project.build(...) can be for
example CI systems (
https://github.com/jenkinsci/maven-dependency-update-trigger-plugin/blob/53ba25f12ee37ef221a9cfe81f9841a0270961b9/src/main/java/org/jvnet/hudson/plugins/mavendepsupdate/MavenUpdateChecker.java
) whose lifetime and amount of processed poms could be much bigger than
what IDEs do.
(For those who are not reading the bug, just be informed that Eclipse IDE,
NetBeans and IntelliJ are affected too).