Skip to content
New issue

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

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

Already on GitHub? # to your account

Clarify priority of Tycho-mappings #1299

Merged
merged 2 commits into from
Sep 8, 2022

Conversation

HannesWell
Copy link
Member

@HannesWell HannesWell commented Aug 28, 2022

As discussed in #1264, this introduces a well defined priority for all of Tycho's polyglot mappings.
The exact order is:

  1. pom.xml
  2. bundle (i.e. META-INF/MANIFEST.MF)
  3. feature (i.e. feature.xml)
  4. repository (i.e. category.xml and *.product files)
  5. target (i.e. *.target)
  6. aggregator (i.e. a directory with one of the names specified via system property tycho.pomless.aggregator.names)

Furthermore this updates to polyglot-common 4.9 to use takari/polyglot-maven#242 and introduces a PomXMLMapping. The latter is necessary because the default org.sonatype.maven.polyglot.mapping.XmlMapping erroneously claims to handle feature.xml models (because of their ending). Without a dedicated PomXMLMapping we would have to move the XMLMapping as second last in the list above and would have to check in each other Tycho-mapping if the project also has a pom.xml.

@github-actions
Copy link

github-actions bot commented Aug 28, 2022

Test Results

324 files  324 suites   2h 14m 6s ⏱️
290 tests 286 ✔️ 4 💤 0
580 runs  571 ✔️ 9 💤 0

Results for commit 4ac3d1b.

♻️ This comment has been updated with latest results.

@laeubi laeubi changed the title Clarify priority of Tycho-mappings and update to polyglot-common 4.9 Clarify priority of Tycho-mappings Sep 5, 2022
@HannesWell
Copy link
Member Author

I split the this change into two commits.
The first one only introduces the mapping-specific priorities (the actual purpose of this PR), the second one applies some clean-up/code re-ordering.
If you prefer I can open the second commit as separate PR, but the commits can also be reviewed one by one.

@HannesWell HannesWell force-pushed the clearifyMappings branch 2 times, most recently from a83f407 to e5a1aa0 Compare September 6, 2022 20:41
Copy link
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

Just a minor comment before we can merge this, maybe you also want to squash all and use one commit message that covers all changes.

@HannesWell
Copy link
Member Author

Just a minor comment before we can merge this, maybe you also want to squash all and use one commit message that covers all changes.

I would prefer to keep the changes in distinct commits. Especially the last change to NIO Path changes many places and the other changes which would make it harder to find the changes of priorities if one later goes through git.
But the specification of distinct priorities plus method reordering and Set simplifications can be squashed without making the result too complicated.

@laeubi laeubi merged commit 5782054 into eclipse-tycho:master Sep 8, 2022
@HannesWell HannesWell deleted the clearifyMappings branch September 8, 2022 15:07
@laeubi laeubi added this to the 3.0 milestone Sep 21, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants