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

[MNG-7870] Undeprecate G level metadata #1224

Merged
merged 3 commits into from
Sep 7, 2023
Merged

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Sep 5, 2023

The modello generated classes should not be deprecated, and they are in use.


https://issues.apache.org/jira/browse/MNG-7870

@cstamas cstamas requested review from gnodet and hboutemy September 5, 2023 14:56
@cstamas cstamas self-assigned this Sep 5, 2023
@cstamas cstamas added this to the 3.9.5 milestone Sep 5, 2023
~~ 1. MNG-7266: maven-compat will be removed from future Maven version
~~ 2. this will remove the code that updates plugins data: see MNG-7375/MPLUGIN-384 https://maven.apache.org/ref/3.8.4/maven-compat/apidocs/org/apache/maven/artifact/repository/metadata/GroupRepositoryMetadata.html
~~ 3. this will lead to inconsistent data: removing it will be safer/more clear
~~ but this logic still remains to be confirmed by clear consensus of the whole team
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, this is not wrong, but just too early, right ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this is plain wrong: assumes that two distinct things are "same thing" (maven-repository-metadata generated classes and maven-compat Metadata coming from Maven2, pre Aether times).

It blurs by mistake two distinct things, while they are not same (but their purpose WAS same, but latter should not be used anymore, while former is and should be used by resolver-provider ONLY)

Copy link
Member

Choose a reason for hiding this comment

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

can we clarify what fills the (effectively non-deprecated) metadata? And when Maven was fixed to update this metadata?
because this is the part that is not clear to me

Copy link
Member Author

Choose a reason for hiding this comment

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

It was done on master (#555) and ported to maven-3.9.x (#691)

The code is (and should remain) confined in maven-resolver-provider, as it "hooks" into resolver https://github.com/apache/maven-resolver/blob/master/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/MetadataGeneratorFactory.java and does the job by simply inspecting the payload. Also notice that all this code is package protected (again, as this happens by "hooking into resolver, and is not an API that one should tamper/call").

Copy link
Member

@hboutemy hboutemy left a comment

Choose a reason for hiding this comment

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

is it https://issues.apache.org/jira/browse/MNG-7055 ?
I'd prefer keeping a comment in this documentation, because it's hard to track

@cstamas
Copy link
Member Author

cstamas commented Sep 6, 2023

Yes, it was done as part of MNG-7055:

@cstamas
Copy link
Member Author

cstamas commented Sep 6, 2023

My point is, that the related code change simply made G level metadata work the "same way" as GA and GAV level metadata worked: user needs not to manage it, it all happens transparently from user POV, is done by resolver "behind the curtains", without user interaction.

@cstamas
Copy link
Member Author

cstamas commented Sep 6, 2023

And finally, i find this page (rendered as part of maven-repository-metadata site) a bit anemic, and IMHO missing the point: https://maven.apache.org/ref/3.9.4/maven-repository-metadata/index.html

Especially when I put it next to this page:
https://maven.apache.org/repositories/metadata.html

Former page, that is about maven-repository-metadata module (that contains ONLY the modello generated model classes) talks about "local" and "remote" metadata, that is totally irrelevant, and none of those things happens in given module. The "metadata file names" are irrelevant (are defined by layout), and page https://maven.apache.org/repositories/metadata.html IMHO describes much better all of it (from remote repo aspect, it does not mention "locally cached" metadata either).

So, I'd propose to modify page even more, and just keep this:

# Maven Repository Metadata Model

This is strictly the model for Maven Repository Metadata, so really just plain objects.

The following are generated from this model:

* [Java sources](https://maven.apache.org/ref/3.9.4/maven-repository-metadata/apidocs/index.html) with Reader and Writers for the Xpp3 XML parser, to read and write maven-metadata(-*).xml files,
* a [Descriptor Reference](https://maven.apache.org/ref/3.9.4/maven-repository-metadata/repository-metadata.html).

This module really generates JUST the object that are
used to describe G, GA and GAV level maven metadata.

How the file is named, how is locally cached one named
is totally irrelevant (as it is defined by layout and
resolver cache logic).
Copy link
Member

@hboutemy hboutemy left a comment

Choose a reason for hiding this comment

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

keep the description text, but add a link to end-user doc https://maven.apache.org/repositories/metadata.html

maven-repository-metadata/src/site/apt/index.apt Outdated Show resolved Hide resolved
@cstamas
Copy link
Member Author

cstamas commented Sep 6, 2023

ping?

@cstamas cstamas closed this Sep 6, 2023
@cstamas cstamas reopened this Sep 6, 2023
@cstamas
Copy link
Member Author

cstamas commented Sep 6, 2023

Closed by mistake, reopened

@cstamas cstamas merged commit 84ee422 into apache:maven-3.9.x Sep 7, 2023
@cstamas cstamas deleted the MNG-7870 branch September 7, 2023 10:03
cstamas added a commit that referenced this pull request Sep 7, 2023
The modello generated classes should not be deprecated, and they are in use.

Forward port of 84ee422

---

https://issues.apache.org/jira/browse/MNG-7870
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.

3 participants