Include all sources by default in eclipse-sdk target

[HannesWell]

No description provided.

[merks]

Could you explain this? Do we really want to exclude source of anything in the SDK? Is a default value being serialized? Does this change what is in the target platform? If not, why do we need this change?

[laeubi]

Regarding the source thing, it seems that currently we list source bundle (and features) explicitly in the location (sometimes) so question for me would be if we should remove the explicit ones and enable it in general. Beside that the added values are just the defaults so making it explicit what is used anyways.

[HannesWell]

Is a default value being serialized? Does this change what is in the target platform? If not, why do we need this change?

Initially this just serialized the default attributes that are added by the Target-Editor upon any modification and I wanted to avoid unrelated changes in the future if one uses the Editor.

Beside that the added values are just the defaults so making it explicit what is used anyways.

Yes, exactly.

Regarding the source thing, it seems that currently we list source bundle (and features) explicitly in the location (sometimes) so question for me would be if we should remove the explicit ones and enable it in general.

Good point, I have now enhanced this change to remove all explicitly listed source bundles/features and set the includeSource attribute to true for all IU-locations.

[merks]

I like the removal of explicit source ius. An attribute for that is more convenient. It should be true for all locations though I think. Or?

[merks]

The unrelated charges annoyed me too. Better I can tell it to upgrade without noise.

[laeubi]

The unrelated charges annoyed me too. Better I can tell it to upgrade without noise.

The main problem is that the target file is "handcrafted" especially the comments and formatting/orderings of maven target locations.

PDE has special handling for IU targets to minimize that, but do not make it available to general locations and not always works well.

[merks]

Lately it was literally just a line feed change in the comment about the install location. So mostly not so annoying. 😀

[HannesWell]

I like the removal of explicit source ius. An attribute for that is more convenient. It should be true for all locations though I think. Or?

Yes, I think so as well, but to me it looks like it's enabled for all locations now.
However the build fails saying that org.eclipse.emf.common.source.feature.group, which is required by the eclipse.sdk (which is IMO valid for an SDK), is cannot be found.

[merks]

I wonder if we need the source feature. Isn’t there an include all sources for the product/repository that would include the source bundles?

[HannesWell]

I wonder if we need the source feature. Isn’t there an include all sources for the product/repository that would include the source bundles?

Yes that option exists and AFAICT it's enabled for the Eclipse-SDK repo. So technically we could remove all explicit sources from that feature.
But in the end I think it's a philosophical question if the org.eclipse.sdk feature should include the sources even if the automated inclusion of sources in a consuming target is not enabled. Personally it wouldn't bother me because I usually have includeSource="true". It most of the time very useful and the only drawback I know that it consumes more disk-space and download-bandwidth for my dev-workspace, but nowadays I don't think that's a real problem anymore.
However others might think differently and maybe argue that a SDK-feature should 'actively' pull in the sources.

Besides of what we want to do with respect to the o.e.sdk bundle, I think it's a missing capability of tycho that one cannot reference source-features (maybe also not source-bundles) in a feature if those source-IUs are only included via includeSource="true" in the target.

[HannesWell]

In fact we have multiple sdk features that explicitly include sources:

But the org.eclipse.sdk is the only one 'importing' source-IUs from the TP and including the (generated) source-IUs from the reactor works well.

[laeubi]

But in the end I think it's a philosophical question if the org.eclipse.sdk feature should include the sources even if the automated inclusion of sources in a consuming target is not enabled. Personally it wouldn't bother me because I usually have includeSource="true". It most of the time very useful and the only drawback I know that it consumes more disk-space and download-bandwidth for my dev-workspace, but nowadays I don't think that's a real problem anymore.
However others might think differently and maybe argue that a SDK-feature should 'actively' pull in the sources.

Many things are of course "historical" but I think for this use-case it would be better if a feature.xml would have an attribute "include sources" that the is read by Tycho to include the sources (without a separate source feature), this could also be useful in cases where I have legal requirements to ship the source and don't want to maintain it manually.

So we need a new checkbox in PDE and then Tycho can support it.

In fact we have multiple sdk features that explicitly include sources

Yes that should work without a problem

But the org.eclipse.sdk is the only one 'importing' source-IUs

It might be that this case is special indeed, but I don't see any advantage here in import versus include