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

Fix maven group id to match the actual deployment #737

Merged
merged 1 commit into from
Jan 12, 2025

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Jan 12, 2025

Since a long while equinox is deployed to the org.eclipse.platform group id, but in the P2 maven metadata this is currently recorded as org.eclipse.equinox (and org.eclipse.osgi), this confuses tools like Tycho that try to match P2 to maven metadata (and is not very helpful at all)

This now fixes the issue for the bundles (as features are not deployed at the moment anyways).

Since a long while equinox is deployed to the org.eclipse.platform group
id, but in the P2 maven metadata this is currently recorded as
'org.eclipse.equinox', this confuses tools like Tycho that try to match
P2 to maven metadata (and is not very helpful at all)

This now fixes the issue for the bundles (as features are not deployed
at the moment anyways)
Copy link

Test Results

  666 files  ±0    666 suites  ±0   1h 18m 34s ⏱️ + 2m 43s
2 217 tests ±0  2 170 ✅ ±0   47 💤 ±0  0 ❌ ±0 
6 795 runs  ±0  6 652 ✅ ±0  143 💤 ±0  0 ❌ ±0 

Results for commit 402da06. ± Comparison against base commit a1c46e9.

@laeubi laeubi merged commit cdb5cbc into eclipse-equinox:master Jan 12, 2025
26 of 27 checks passed
@akurtakov
Copy link
Member

Thanks for doing this! Ideally this would happen for all deployed bundles.
@HannesWell Would you please consider enhancing the publishing jobs to have a report showing which bundles change groupId?
IMO if there is not change in groupId reusing Maven poms will far easier.

@laeubi
Copy link
Member Author

laeubi commented Jan 12, 2025

Thanks for doing this! Ideally this would happen for all deployed bundles.

Yes I just need to check for others as well and just noticed that for equinox, but the same might be required for p2, pde, jdt, ...

@HannesWell
Copy link
Member

@HannesWell Would you please consider enhancing the publishing jobs to have a report showing which bundles change groupId?

You mean compared to the information recorded in their original POM? I have to admit that I don't have an immediate idea how to add that in an elegant way.
But I can say that PDE and JDT are deployed to their own groupId:

So it's probably just Equinox/P2 that doesn't get deployed under their 'natural' groupId:

@merks
Copy link
Contributor

merks commented Jan 12, 2025

Note that both these are and will continue to be trivially handled by this rule:

image

So while changing the pom details might be helpful for something, e.g., for consistency, it makes no real/significant difference for the Maven publishing.

Here I copy and change *.aggr to use only the rule that maps a bundle to a POM artifact based on the group ID in the p2 metadata and create a corresponding *.aggran where I can look at the Maven tab to see how artifacts are grouped, with the screen capture below showing a subset of the set of group IDs needing attention:

image

I expect searching for in pom.xml will be helpful in this regard as well and naturally there is a hodgepodge of group IDs, despite assumptions to the contrary.

One unfortunate consequence of changing the group ID is that the same IU will have different before-and-after metadata properties, so no guarantee which version will end up in SimRel.

@laeubi
Copy link
Member Author

laeubi commented Jan 12, 2025

So while changing the pom details might be helpful for something, e.g., for consistency, it makes no real/significant difference for the Maven publishing.

The problem more arise at the inverse case, if one looks at the release repository it has the wrong metadata and if one looks under this in maven central one gets either no match at all or only some very old outdated stuff.

One unfortunate consequence of changing the group ID is that the same IU will have different before-and-after metadata properties, so no guarantee which version will end up in SimRel.

I fear it will take some time to "bubble up" but better we start now than later, another problem is that currently there is always a SNAPSHOT version mentioned while we actually deploy the stuff without a version qualifier... but for this I need to change Tycho first so one can specify a different mapping (what is not completely trivial).

Another option would of course be if we somehow could use the mapping used to deploy to fix the metadata of P2 before publishing the final site, but this does not sound trivial either...

@merks
Copy link
Contributor

merks commented Jan 12, 2025

I can definitely see value in consistently using the same group ID in the build as will be used when the artifact is published! I have no idea why so many different group IDs where used in the first place; probably because it just didn't seem to matter at the time but that's no reason to leave it like this.

As the IU versions evolve, i.e., even just a new qualifier, the newer ones will have the improved metadata so eventually that will all come out in the wash...

merks added a commit to merks/equinox that referenced this pull request Jan 12, 2025
Fix the remaining bundles not addressed by the previous commit.

eclipse-equinox#737
@laeubi
Copy link
Member Author

laeubi commented Jan 12, 2025

Of course choosing a good group id is always hard, in general each project should use its project id, so the equinox choice was not that bad. But with the "global" deployment to central it could have seem easier to just use one group id.

The adding the maven metadata was quite "new" (now some years though) but no one really wanted to touch things as we have maintained many pom.xml... now things have settled and we use pomless mostly it becomes much easier to cleanup such things. And of course things will change/evolve over time as well...

I hope to improve this all with Tycho 5 much more and make it easier to align maven + osgi versions even for the usual deployment options.

@mickaelistria
Copy link
Contributor

While I enjoy seeing an inconsistency resolved, I'm wondering whether a fix from the other end could have been better. Basically, one question can be why us the org.eclipse.platform group being used for eg equinox when the bundles has org.eclipse.osgi declared as groupId in its p2 metadata.
To me, the inconsistency comes more from the process of publishing Maven artifacts which hardcodes different values to one the projects actually maintain in the pom.xml (and p2 metadata).

merks added a commit that referenced this pull request Jan 12, 2025
Fix the remaining bundles not addressed by the previous commit.

#737
@merks
Copy link
Contributor

merks commented Jan 12, 2025

Given the extended period of equinox has been published to that group, changing it now is unlikely to be actually helpful for any existing consumers:

https://repo1.maven.org/maven2/org/eclipse/platform/org.eclipse.equinox.event/

image

Anyway, the comment is quite specific about "match the actual deployment" and that's exactly what it does and who decided to deploy things in this way probably lost to history...

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.

5 participants