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

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

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

rfscholte
On Wed, 28 Nov 2018 17:13:32 +0100, Mickael Istria <[hidden email]>  
wrote:

> 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

> 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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

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

Mickael Istria-2
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).