You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Mickael Istria <mi...@redhat.com> on 2018/11/28 16:13:32 UTC
MNG-6530 - Please fix or revert important regression from MNG-6311
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,
--
Mickael Istria
Eclipse IDE <https://www.eclipse.org/downloads/eclipse-packages/>
developer, for Red Hat Developers <https://developers.redhat.com/>
Re: MNG-6530 - Please fix or revert important regression from MNG-6311
Posted by Mickael Istria <mi...@redhat.com>.
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).
Re: MNG-6530 - Please fix or revert important regression from MNG-6311
Posted by Mickael Istria <mi...@redhat.com>.
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,
Re: MNG-6530 - Please fix or revert important regression from
MNG-6311
Posted by Karl Heinz Marbaise <kh...@gmx.de>.
Hi,
On 28/11/18 21:21, Robert Scholte wrote:
> On Wed, 28 Nov 2018 17:13:32 +0100, Mickael Istria <mi...@redhat.com>
> 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.
+1 agreed
Kind regards
Karl Heinz
>
> 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: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: MNG-6530 - Please fix or revert important regression from
MNG-6311
Posted by Robert Scholte <rf...@apache.org>.
On Wed, 28 Nov 2018 17:13:32 +0100, Mickael Istria <mi...@redhat.com>
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: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org