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

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

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

Mickael Istria-2
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/>
Reply | Threaded
Open this post in threaded view
|

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

Karl Heinz Marbaise-3
Hi,

On 28/11/18 21:21, Robert Scholte wrote:

> 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.



+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: [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,

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,