Skip to content

Remove SourceFeatureMojo#5137

Draft
laeubi wants to merge 4 commits intoeclipse-tycho:mainfrom
laeubi:clean_up_removal
Draft

Remove SourceFeatureMojo#5137
laeubi wants to merge 4 commits intoeclipse-tycho:mainfrom
laeubi:clean_up_removal

Conversation

@laeubi
Copy link
Copy Markdown
Member

@laeubi laeubi commented Jun 12, 2025

This is marked as deprecated / for removal in

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

Fix #3458

@laeubi laeubi changed the title Cleanup the REMOVAL.MD as it already was performed Remove SourceFeatureMojo Jun 12, 2025
@laeubi laeubi marked this pull request as draft June 12, 2025 05:12
@laeubi
Copy link
Copy Markdown
Member Author

laeubi commented Jun 12, 2025

/request-license-review

@github-actions
Copy link
Copy Markdown

/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

@merks merks self-requested a review June 12, 2025 05:29
merks
merks previously requested changes Jun 12, 2025
Copy link
Copy Markdown
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown
Member Author

laeubi commented Jun 12, 2025

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
Copy link
Copy Markdown

github-actions Bot commented Jun 12, 2025

Test Results

1 035 files  1 035 suites   5h 34m 5s ⏱️
1 359 tests 1 337 ✅ 21 💤 1 ❌
4 077 runs  4 008 ✅ 66 💤 3 ❌

For more details on these failures, see this check.

Results for commit 43e4917.

♻️ This comment has been updated with latest results.

@sratz
Copy link
Copy Markdown
Member

sratz commented Jun 12, 2025

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
Copy link
Copy Markdown
Member Author

laeubi commented Jun 12, 2025

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
Copy link
Copy Markdown
Contributor

merks commented Jun 12, 2025

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
Copy link
Copy Markdown
Member Author

laeubi commented Jun 12, 2025

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
Copy link
Copy Markdown
Contributor

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

@sratz
Copy link
Copy Markdown
Member

sratz commented Jun 12, 2025

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
Copy link
Copy Markdown
Member Author

laeubi commented Jun 12, 2025

@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
Copy link
Copy Markdown
Member

sratz commented Jun 26, 2025

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

@laeubi laeubi added this to the 6.0 milestone Jul 6, 2025
@laeubi laeubi force-pushed the clean_up_removal branch from c54c401 to 64f0a96 Compare July 13, 2025 07:25
@laeubi laeubi force-pushed the clean_up_removal branch from 64f0a96 to 876098f Compare July 27, 2025 15:58
@merks merks self-requested a review July 27, 2025 17:11
Copy link
Copy Markdown
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

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

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.

@merks merks self-requested a review July 27, 2025 17:13
@merks merks dismissed their stale review July 27, 2025 17:13

Abstain.

@laeubi
Copy link
Copy Markdown
Member Author

laeubi commented Jul 28, 2025

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

@laeubi laeubi force-pushed the clean_up_removal branch 3 times, most recently from 632f564 to 8601210 Compare August 30, 2025 15:28
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.

deprecate (and remove) feature-source mojo

5 participants