Remove SourceFeatureMojo

[laeubi]

This is marked as deprecated / for removal in

• Deprecate source features mojo and mark for removal #5138

once Tycho 5 is released with this notice, we should remove it from Tycho 6.

Fix #3458

[laeubi]

/request-license-review

[github-actions]

/request-license-review

License review requests:

• maven/mavencentral/org.jspecify/jspecify/1.0.0

After all reviews have concluded, re-run the license-vetting check from the Github Actions web-interface to update its status.

Workflow run (with attached summary files):
https://github.com/eclipse-tycho/tycho/actions/runs/15602213824

[merks]

I don't think the Platform's decision to stop using something (and it's not even clear this will be entirely removed) is a good reason for removing it from Tycho such that the rest of the ecosystem must necessarily follow the Platform's decision. 😱

[laeubi]

The reason for removing it from Tycho is not because Platform don't use it anymore. It is because I don't want to support it anymore. Anyone that has a strong requirement for source features can simply maintain them as a separate artifact, but generating them has always be prone to complications in the build process (e.g. p2 metadata has to be regenerated), people has to carefully review if really everything is included (what requires including third party content possibly) or get annoying errors once in a while (what no one wants).

So I think its time to get rid of this unless someone shows a real compelling use-case for this.

[github-actions]

Test Results

1 005 files   -  3  1 005 suites   - 3   5h 5m 41s ⏱️ - 22m 48s
1 289 tests  -  4  1 264 ✅  -  9  20 💤 ±0  1 ❌ +1   4 🔥 + 4 
3 867 runs   - 12  3 788 ✅  - 27  64 💤 ±0  3 ❌ +3  12 🔥 +12 

For more details on these failures and errors, see this check.

Results for commit 8601210. ± Comparison against base commit 26a38a8.

This pull request removes 4 tests.

org.eclipse.tycho.source.SourceFeatureSkeletonTest ‑ testFeatureLabelDefault
org.eclipse.tycho.source.SourceFeatureSkeletonTest ‑ testFeatureLabelMissingInProperties
org.eclipse.tycho.source.SourceFeatureSkeletonTest ‑ testFeatureWithLabelInPropertiesDefaultSuffix
org.eclipse.tycho.source.SourceFeatureSkeletonTest ‑ testLabelOverriddenInSourceTemplate

♻️ This comment has been updated with latest results.

[sratz]

The reason for removing it from Tycho is not because Platform don't use it anymore. It is because I don't want to support it anymore. Anyone that has a strong requirement for source features can simply maintain them as a separate artifact, but generating them has always be prone to complications in the build process (e.g. p2 metadata has to be regenerated), people has to carefully review if really everything is included (what requires including third party content possibly) or get annoying errors once in a while (what no one wants).

So I think its time to get rid of this unless someone shows a real compelling use-case for this.

😮 I was actually really happy once feature-source was introduced and we were able to throw away all our manually created source features.

The mojo has worked perfectly fine for our use-cases and I'd be sad to see this go.

[laeubi]

The mojo has worked perfectly fine for our use-cases and I'd be sad to see this go.

But beside that you always did it, why do you actually need any source features?

The main reason for source features was to include sources in update sites for what you can simply use https://tycho.eclipseprojects.io/doc/latest/tycho-p2-repository-plugin/assemble-repository-mojo.html#includeAllSources these days, no need for any source feature at all.

If you want sources in your target, simply choose "include sources" there (available since ever).

[merks]

You can't always know what people others are doing and why. I think the net effect of such a decision is that an unknown number of people will get stuck on an old version. Also an unknown number of people will need to do an unknown about of work which one might argue is "for their safety and comfort", or even "for their own good", but can we argue it's strongly necessary. I know there are likely better ways for (almost) all use cases, but still, we make Tycho more consumable if we make it easier to move to new versions with minimal disruption. It's just be sense and my opinion...

[laeubi]

Also an unknown number of people will need to do an unknown about of work which one might argue is "for their safety and comfort", or even "for their own good", but can we argue it's strongly necessary

Every thing comes at a price, so maintaining this feature (especially with the move to Maven 4) will create some efforts. So unless these "unknown" people step out of the dark and start to contribute to the project in any way I don'T see how it is sufficient to put the burden on the project. The same arguments also holds true for upgrading Java, upgrading Maven and any other change.

So for me the deprecation of the feature is somehow a call for action for those, and if someone then presents a good use-case that can't be solved otherwise then one might decide different or improve existing features so solve additional problems. But "we don't know why but we don't want to change" is something that will not help in future evolution of Tycho towards a useful and modern build tool.

So for now the recommendation would be:

1. remove the mojo execution, save some build times and possible headaches
2. enable includeAllSources for your update site
3. See if you actually would miss anything

[mickaelistria]

+1 for removing in Tycho 6 after a (long enough, 6 months?) phase of deprecation.

[sratz]

But beside that you always did it, why do you actually need any source features?

Fair point ;)

The main reason for source features was to include sources in update sites for what you can simply use https://tycho.eclipseprojects.io/doc/latest/tycho-p2-repository-plugin/assemble-repository-mojo.html#includeAllSources these days, no need for any source feature at all.

If you want sources in your target, simply choose "include sources" there (available since ever).

I looked into this and indeed - at least for our indirect repositories - we can probably just use <includeAllSources>.

However, there is one scenario in the toplevel direct shipment repository where we do the following:

updatesite/
  category.xml
    <feature id="foo.bar.feature">

updatesite.internal/
  category.xml
    <feature id="foo.bar.feature.source">
    <feature id="foo.bar.feature.test">
    <feature id="foo.bar.feature.test.source">

and we have configured feature-source with <includeBinaryFeature>false</includeBinaryFeature>.

That way we have a productive (shipment) update site containing only the binaries + a delta update site with the tests and the sources used internally by the developers.

I don't see a trivial way to model such a 'sources + tests only' update site with just <includeAllSources>.

[laeubi]

@sratz regarding the internal site just enable for that site to include all sources and it can be rewritten to:

updatesite.internal/
  category.xml
    <feature id="foo.bar.feature">
    <feature id="foo.bar.feature.test">

If you then add your updatesite as a reference (either in category.xml or pom.xml [see addPomRepositoryReferences]) and use filterProvided you should end up with a minimal site that only includes the delta compared to your regular site.

If that does not work you might provide an integration-test to demonstrate the issue so we can better support the use-case of a delta-update site.

[sratz]

If you then add your updatesite as a reference (either in category.xml or pom.xml [see addPomRepositoryReferences]) and use filterProvided you should end up with a minimal site that only includes the delta compared to your regular site.

That seems to work fine.

But actually there was no real reason for the internal update site to be a delta instead of just being self-contained itself (other than to save a bit of space).

We have now removed all our source-features in our build and it indeed made things easier and less verbose.

So no objection to deprecate/remove the functionality from my side. 👍

[merks]

I'll abstain.

The Platform removal process is completed basically by removing all includes of source bundles and source features from all features and products. Repository assembly, product materialization, and target platform resolution, already support "include all sources" to accomplish pretty much the same effect.

Abstain.

[laeubi]

Platform is now building clean without this feature! We need to adjust some integration tests now.