Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MSHARED-1080] Parent POM 36, Java8, drop legacy. #38

Merged
merged 6 commits into from Jun 11, 2022
Merged

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Jun 8, 2022

Changes:

  • parent POM 36
  • Java8, up locked deps (due Java7)
  • drop maven-shared-util and other dead dependencies

https://issues.apache.org/jira/browse/MSHARED-1080

@cstamas cstamas self-assigned this Jun 8, 2022
@@ -105,8 +110,6 @@ public void copyFile( File from, File to, boolean filtering, List<FileUtils.Filt
}
FileUtils.copyFile( from, to, encoding, new FileUtils.FilterWrapper[0], overwrite );
}

buildContext.refresh( to );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't something used by m2e? or not anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from a repo that is last touched 12 years ago, and is archived. IMHO is dead, parrot dead, but we can tinker about it if you want 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a quick search on github shows this https://github.com/search?q=org%3Aeclipse-m2e+BuildContext&type=code
for sure this very old repo is just interface which doesn't need much changes so that's probably the reason.

disclaimer i'm not using m2e so... 🤣

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing the point: this parrot is dead. Very. Also, whatever it is, this was first attempt to implement incremental build, followed by two other attempts (by same person), that finally materialized in something completely different. So, yes, this may look "stable", the very same way dead parrot looks "stable" (well, not moving, is it?), but is superseded by several other attempts...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe but I can see references of it in the m2e code.
Is it really used or not? I have no idea as I'm not a contributor of m2e.
My opinion is just it worth to ask those folks as we may or not break something.
if not perfect but it worth the courtesy of simply asking.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not a rant. you point to a repo mentioning it's the logical successor.
whereas my point is by removing this you will affect all m2e users which is probably hundreds thousands even million(s) of users.
By pointing the repo I'm just saying m2e has a lot of contribution/maintenance and users compared to the repo you mention.
Even if I agree this was a bad idea/design (and as the guy who started the maven-filtering I didn't like it long time ago neither but it has been done...) but now it's here and use by a lot of people.
so we live with that or we discuss with m2e folks a replacement solution or at least ask them what are the impacts removing this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I get it correctly, what you say is "buildcontext prevails over takari/incrementalbuild as m2e has more commits"? 🦘 LOL, well, I am about to stop here.... 😆

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I get it correctly, what you say is "buildcontext prevails over takari/incrementalbuild as m2e has more commits"? 🦘 LOL, well, I am about to stop here.... 😆

no sorry but you do not read correctly, the most important is the number of users impacted.
and here in m2e is a lot!
what is better it's not the question here.
again I'm just saying the impacted users number is really huge!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

frankly looking at commit history m2e has more maintenance than https://github.com/takari/io.takari.incrementalbuild (last commit almost 2 yo ago).

And who said this then?

Anyway, lets bury the boomerang, sisu build api is raised from the dead

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and my 5 cents to whole this topic: I never saw a "plain" (explained later) Maven project that is fairly more complex than "hello world" and works OOTB in m2e. Work, as in no "tweaks", "m2e profile in POM" needed etc. OTOH, I did saw Maven project using takari lifecycle that WORK perfectly and OOTB in m2e.

But alas, takari lifecycle use the "inferior" incrementalbuild and not buildcontext API. Oh, and it also replaces many things from Maven itself, and even ditches this maven-filtering and replaces it's with it's own to be functional 😄

("plain" as Maven build w/o any extension, while takari-lifecycle is an extension and replaces a LOT from maven, like core copmponets but also shared stuff like this maven-filtering and many plugins as well)

@olamy
Copy link
Member

olamy commented Jun 9, 2022

@fbricon still active m2e? any idea if this BuildContext is still in use? or maybe you can ping someone else? thanks

@olamy
Copy link
Member

olamy commented Jun 9, 2022

@rakus ping

@cstamas cstamas marked this pull request as ready for review June 9, 2022 13:11
@cstamas cstamas requested review from gnodet and michael-o June 9, 2022 13:51
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Show resolved Hide resolved
@cstamas cstamas requested a review from michael-o June 10, 2022 20:03
@cstamas cstamas merged commit 3c92e1b into master Jun 11, 2022
@cstamas cstamas deleted the mshared-1080 branch June 11, 2022 08:15
@cstamas
Copy link
Member Author

cstamas commented Jun 20, 2022

So, given the m2e team response (or better: lack of it), I think that maven-filtering is actually not in use in m2e (is being completely replaced), or, that buildcontext seems totally irrelevant @olamy

@fbricon
Copy link

fbricon commented Jun 21, 2022

Sorry for the late response (I'm not super active on the m2e front), but yeah the buildcontext API is very much used by m2e: https://github.com/eclipse-m2e/m2e-core/search?q=BuildContext
Basically mvn resources:resources is called on each file change, as part of Eclipse's incremental build. So if filtering where to be invoked on all resources instead of what's defined in the build context, we'd see severe performance degradation in Eclipse

@cstamas
Copy link
Member Author

cstamas commented Jun 21, 2022

@fbricon but m2e uses this, very this implementation? Or it depends, as AFAIK when takari-lifecycle used, then this implementation is totally swapped out and unused by m2e.

@fbricon
Copy link

fbricon commented Jun 21, 2022

m2e only uses org.sonatype.plexus.build.incremental.BuildContext.

Looks like the eclipse-takari integration uses its own thing (io.takari.incrementalbuild.BuildContext). @ifedorenko or @jvanzyl would know better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants