tycho icon indicating copy to clipboard operation
tycho copied to clipboard

Remove SourceFeatureMojo

Open laeubi opened this issue 6 months ago • 13 comments

This is marked as deprecated / for removal in

  • https://github.com/eclipse-tycho/tycho/pull/5138

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

Fix https://github.com/eclipse-tycho/tycho/issues/3458

laeubi avatar Jun 12 '25 04:06 laeubi

/request-license-review

laeubi avatar Jun 12 '25 05:06 laeubi

/request-license-review

License review requests:

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

github-actions[bot] avatar Jun 12 '25 05:06 github-actions[bot]

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.

laeubi avatar Jun 12 '25 06:06 laeubi

Test Results

1 029 files  1 029 suites   6h 27m 51s ⏱️ 1 347 tests 1 322 ✅ 20 💤 2 ❌ 3 🔥 4 041 runs  3 963 ✅ 63 💤 6 ❌ 9 🔥

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

Results for commit f11b9953.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Jun 12 '25 07:06 github-actions[bot]

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.

sratz avatar Jun 12 '25 08:06 sratz

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).

laeubi avatar Jun 12 '25 08:06 laeubi

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...

merks avatar Jun 12 '25 09:06 merks

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

laeubi avatar Jun 12 '25 09:06 laeubi

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

mickaelistria avatar Jun 12 '25 09:06 mickaelistria

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>.

sratz avatar Jun 12 '25 14:06 sratz

@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.

laeubi avatar Jun 12 '25 14:06 laeubi

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. 👍

sratz avatar Jun 26 '25 14:06 sratz

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

laeubi avatar Jul 28 '25 03:07 laeubi