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

[MINSTALL-175] Drop MAT #31

Closed
wants to merge 4 commits into from
Closed

[MINSTALL-175] Drop MAT #31

wants to merge 4 commits into from

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Jun 20, 2022

Now that Maven is 3.2.5, no need for MAT anymore.


https://issues.apache.org/jira/browse/MINSTALL-175

Now that Maven is 3.2.5, no need for MAT anymore.
@cstamas cstamas self-assigned this Jun 20, 2022
pom.xml Outdated Show resolved Hide resolved
pom.xml Show resolved Hide resolved
@cstamas cstamas changed the title Drop MAT [MINSTALL-175] Drop MAT Jun 20, 2022
@cstamas cstamas marked this pull request as ready for review June 20, 2022 15:33
@olamy
Copy link
Member

olamy commented Jun 21, 2022

-0
we reached some consensus few years back saying we will use only org.apache,maven namespace to hide everything happened.
And now we’re back on this?
And then to change again back to org.apache,maven namespace

@slachiewicz
Copy link
Member

Let's imagine that we change package to o.a.m for resolver. It's like change like jakarta from javax. How this will look like? If we just change it, all old plugins will be broken. If we use our shared component - yes we can use reflections and our plugins may work. But what about everything else from community?

+1 here to PR - this simplify code and remove access to API via reflections

and maybe let's start discussion how to rename package on mailing list. This will tak years

@cstamas
Copy link
Member Author

cstamas commented Jun 21, 2022

-0 we reached some consensus few years back saying we will use only org.apache,maven namespace to hide everything happened. And now we’re back on this? And then to change again back to org.apache,maven namespace

So I really don't understand your stance: MAT is needed to bridge between sonatype/eclipse package, but as prerequisite for plugin is raised to Maven 3.2.5, there is NO sonatype package in play anymore, hence, no need for MAT and it's non-trivial transitive dependencies (it pulls in bothj sonatype and eclipse resolver, plexus container default etc).

Where are we about to change to, is WIP currently, the new Maven 4 API, but obviously, it does not exists yet in Maven 3.2.5+ 😕

@olamy
Copy link
Member

olamy commented Jun 23, 2022

Let's imagine that we change package to o.a.m for resolver. It's like change like jakarta from javax. How this will look like? If we just change it, all old plugins will be broken. If we use our shared component - yes we can use reflections and our plugins may work. But what about everything else from community?

Correct I definitely like your comparison here javax. to jakarta
But except you missed to add then the next step will back to javax (well maybe javax.foo)
Because this will be case to change again to org.apache.maven once maven-resolver has been migrated.

The problem here is to have to switch to a package we know it will be removed because it's org.eclipse. package coming from a Apache source repo and deployed under the groupId org.apache.maven
The big mess is here, so the goal is to move away from this package because it simply doesn’t belong here.

Imagine if someone told you: we will remove javax. to javax.foo and in the middle for a bit of time there will be jakarta.

sounds like a waste of time?

+1 here to PR - this simplify code and remove access to API via reflections

maybe why not for our plugins (but still 0 as we know this package will be removed)
but for downstream plugin writers that's a waste of time. and keep changing stuff for no real added values (such jetty/jetty.project#8182 )

Why not removing all of this old stuff from m-artifact-transfer so downstream consumers will not pull old dependencies.

and maybe let's start discussion how to rename package on mailing list. This will tak years

no we have a big opportunity with maven 4.x

@slachiewicz
Copy link
Member

@olamy
Copy link
Member

olamy commented Jun 23, 2022

apache/maven-artifact-transfer#20

almost 2 years later maybe it's now finally a good move to do....
I would approve such PR
but hey if you prefer javax. to jakarta. then back to javax just do it.
but frankly to me it just looks like wasting time changing packaging then back again...
Maybe it would be better to spend time adding real user facing improvements....
anyway it's your time guys and not time.

@slachiewicz
Copy link
Member

@cstamas idea was to save us time and with few lines of code - drop m-a-t-p

<javaVersion>7</javaVersion>
<mavenVersion>3.2.5</mavenVersion>
<aetherVersion>1.0.0.v20140518</aetherVersion> <!-- Maven bound -->

Choose a reason for hiding this comment

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

This is effectively a downgrade? From 1.1.0 to 1.0.0.v20140518?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, because it aligns with the Aether version from Maven 3.2.5.

{
File file = artifact.getFile();

// Here, we have a temporary solution to MINSTALL-3 (isDirectory() is true if it went through compile

Choose a reason for hiding this comment

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

This looks odd. We build against Maven 3.2.5 (see pom.xml) but this comment suggests a "proper solution" will be in place with Maven 2.1?

Copy link
Member Author

Choose a reason for hiding this comment

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

I lifted the relevant MAT code here, so this PR does not change "logic", it merely moves it into this plugin (that was before in MAT). So, just like in #15 "minimal change" is in place, that will subsequent PRs refine.

buildingRequest = repositoryManager.setLocalRepositoryBasedir( buildingRequest, localRepositoryPath );

getLog().debug( "localRepoPath: " + repositoryManager.getLocalRepositoryBasedir( buildingRequest ) );
// "clone" session and replace localRepository

Choose a reason for hiding this comment

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

Suggested change
// "clone" session and replace localRepository
// "clone" Maven session and replace localRepository

@@ -71,6 +73,7 @@
public class InstallFileMojo
extends AbstractInstallMojo
{
private static final String LINE_SEP = System.getProperty( "line.separator" );
Copy link
Member

Choose a reason for hiding this comment

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

LS = System.lineSeparator()

@olamy
Copy link
Member

olamy commented Jul 1, 2022

As an Apache project we do not own the package org.eclipse.aether so we cannot promote the use of such package distributed in a org.apache.maven groupId.
This was supposed to be a temporary transitional phase.
It's now almost 7 years the project has been moved here. So as a transition phase we had enough time to change this aberration from our source code.
This usage should be dead already after so many years. It looks to moving backward with such change..
if the goal is remove some old dependencies we don't like (e.g the dependencies pulled by m-a-t) we can certainly exclude:

<artifactId>maven-artifact-transfer-maven-3.0.x</artifactId>

And even drop this module from our source code if we don't want to keep supporting it.
But the history of the source here at Apache is remove the usage of org.eclipse as a package we distribute which again is totally wrong and we cannot keep promoting.

@cstamas
Copy link
Member Author

cstamas commented Jul 3, 2022

As an Apache project we do not own the package org.eclipse.aether so we cannot promote the use of such package distributed in a org.apache.maven groupId. This was supposed to be a temporary transitional phase. It's now almost 7 years the project has been moved here. So as a transition phase we had enough time to change this aberration from our source code. This usage should be dead already after so many years. It looks to moving backward with such change.. if the goal is remove some old dependencies we don't like (e.g the dependencies pulled by m-a-t) we can certainly exclude:

<artifactId>maven-artifact-transfer-maven-3.0.x</artifactId>

And even drop this module from our source code if we don't want to keep supporting it. But the history of the source here at Apache is remove the usage of org.eclipse as a package we distribute which again is totally wrong and we cannot keep promoting.

Stop spreading FUD man, you personally voted +1 on threads "[VOTE] Accept the Aether code into the Maven project" and also on "[IP CLEARANCE] Aether, renamed to Maven Artifact Resolver" on private ML (unsure could I link these as archives for private ML are not public). Hence, due those two before, I bet you got mails from thread "Trademark requests from Apache Maven regarding Aether" as well, also on private ML (same ML as those two before). In short, "distributing org.eclipse.aether" packages IS NOT WRONG, The Eclipse Foundation granted us usage of that namespace.

@gnodet
Copy link
Contributor

gnodet commented Jul 4, 2022

-0 I'm quite hesitant on that one. I don't really see the benefit of dropping MAT now if the goal is to switch to the new API provided by apache/maven#703. If we keep this goal, I don't really see the point of switching to a get rid of MAT to switch to something else in a couple of weeks/months.

@hboutemy
Copy link
Member

hboutemy commented Jul 4, 2022

what I see is that we should enhance MAT index page https://maven.apache.org/shared/maven-artifact-transfer/ to explain the other objective = have a single API mapped over different implementations varying from Maven version to Maven version

@hboutemy
Copy link
Member

hboutemy commented Jul 4, 2022

another aspect I'd like to make explicit: where is MAT used beyond m-install/deploy-p?
which leads to a corollary: why do some plugins require to do this job themselves instead of letting m-install/deploy-p?

@gnodet
Copy link
Contributor

gnodet commented Jul 4, 2022

another aspect I'd like to make explicit: where is MAT used beyond m-install/deploy-p?
which leads to a corollary: why do some plugins require to do this job themselves instead of letting m-install/deploy-p?

I think MAT would be de-facto deprecated whenever the new API is integrated.
MAT does a bit more than install / deploy, I think the main usage for MAT is the dependency collection / resolution.

@cstamas
Copy link
Member Author

cstamas commented Jul 4, 2022

My problem with MAT and reason why I want to get rid of it ASAP is that it's unfinished (and as @gnodet says "does a bit more than install.deploy"). But is unfinished. I just want to cut it in root before it becomes "competing" with new API in any way, to prevent that someone start submitting PRs to "improve" or "finish" MAT. I just want to prevent any resource waste (human resource) by spending any amount of work on MAT, and focus on new Maven API.

@slawekjaranowski
Copy link
Member

another aspect I'd like to make explicit: where is MAT used beyond m-install/deploy-p?

m-invoker-p for install goal

@gnodet
Copy link
Contributor

gnodet commented Jul 4, 2022

another aspect I'd like to make explicit: where is MAT used beyond m-install/deploy-p?

m-invoker-p for install goal

Here's the full list or projects using MAT:

  • maven-archetype-plugin
  • archetype-common
  • maven-assembly-plugin
  • maven-changes-plugin
  • maven-dependency-plugin
  • maven-gpg-plugin
  • maven-invoker-plugin
  • maven-javadoc-plugin
  • maven-pmd-plugin
  • maven-project-info-reports-plugin
  • maven-remote-resources-plugin
  • maven-shade-plugin

@olamy
Copy link
Member

olamy commented Jul 4, 2022

As an Apache project we do not own the package org.eclipse.aether so we cannot promote the use of such package distributed in a org.apache.maven groupId. This was supposed to be a temporary transitional phase. It's now almost 7 years the project has been moved here. So as a transition phase we had enough time to change this aberration from our source code. This usage should be dead already after so many years. It looks to moving backward with such change.. if the goal is remove some old dependencies we don't like (e.g the dependencies pulled by m-a-t) we can certainly exclude:

<artifactId>maven-artifact-transfer-maven-3.0.x</artifactId>

And even drop this module from our source code if we don't want to keep supporting it. But the history of the source here at Apache is remove the usage of org.eclipse as a package we distribute which again is totally wrong and we cannot keep promoting.

Stop spreading FUD man, you personally voted +1 on threads "[VOTE] Accept the Aether code into the Maven project" and also on "[IP CLEARANCE] Aether, renamed to Maven Artifact Resolver" on private ML (unsure could I link these as archives for private ML are not public). Hence, due those two before, I bet you got mails from thread "Trademark requests from Apache Maven regarding Aether" as well, also on private ML (same ML as those two before). In short, "distributing org.eclipse.aether" packages IS NOT WRONG, The Eclipse Foundation granted us usage of that namespace.

I do not see really FUD and I'm prefectly aware of this acceptation of this temporary package naming. My point has nothing to do with the history on accepting some code. Please do not make confusion with my concern.

We created and agreed on m-artifact-transfer to hide all the previous package changes and the future changes.
I just see the package org.eclipse.aether as deprecated already even before m-artifact-transfer was created.
Again the current/previous package implementation is temporary and we will change it by a renaming from org.eclipse.aether to org.apache.maven. and we will provide a new API as well with 4.x
So plugins maintainers do not have to worry about the real internal and obviously m-artifact-transfer will be de-facto deprecated when we will have maven 4.x api and/or resolver package renaming.
If your concern is the possible m-artifact-transfer. Frankly since this has been created there were not many really....
And we can definitely mark it as deprecated.. But I see those changes as just created confusions for plugin maintainers as this will change again.

@cstamas
Copy link
Member Author

cstamas commented Jul 4, 2022

I do not see really FUD and I'm prefectly aware of this acceptation of this temporary package naming. My point has nothing to do with the history on accepting some code. Please do not make confusion with my concern.

Am glad you finally woke up after 7 years, but this is still wrong: please point me at any email from Eclipse Foundation where they state we got temporary [org.eclipse.aether] package namespace grant only.


We need to get rid of old, over-engineered, over-abstracted, half-assed and semi finished projects, to be able to move forward. They are all just burden, many of them are not touched for 10ish years, many are still maven2 stuff (!) and are totally neglected. I'd be really grateful if you'd lend a hand on this "spring cleaning" work instead.

@olamy
Copy link
Member

olamy commented Jul 4, 2022

I do not see really FUD and I'm prefectly aware of this acceptation of this temporary package naming. My point has nothing to do with the history on accepting some code. Please do not make confusion with my concern.

Am glad you finally woke up after 7 years, but this is still wrong: please point me at any email from Eclipse Foundation where they state we got temporary [org.eclipse.aether] package namespace grant only.

not sure why you are mentioning this but this is not my point about a permanent or temporary grant.
I'm just mentioning the fact I find obvious a project hosted at Apache use the corresponding package.
if it takes time to change it fair enough we are all volunteers with different bandwidth to work on this.
But my point is just related to hiding this as this has already changed and will change again.

We need to get rid of old, over-engineered, over-abstracted, half-assed and semi finished projects, to be able to move forward. They are all just burden, many of them are not touched for 10ish years, many are still maven2 stuff (!) and are totally neglected. I'd be really grateful if you'd lend a hand on this "spring cleaning" work instead.

I'm happy to get rid of this once we have finalised the package change or the maven 4.x api.

@cstamas
Copy link
Member Author

cstamas commented Jul 7, 2022

Superseded by #32

@cstamas cstamas closed this Jul 7, 2022
@cstamas cstamas deleted the drop-mat branch July 7, 2022 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants