Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove unused product definitions #1235

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

HannesWell
Copy link
Member

All other product definitions mentioned in #1195 (comment) seem to be used in some way.
But I have the impression that some of them are only used to build p2-repos or that the build products are not used at all.
For example eclipse-junit-tests is mirrored into the eclipse.platform.repository/pom.xml. So I wonder why the content is not just directly listed in eclipse.platform.repository/category.xml.

And for example equinox-sdk.product or EclipseRTOSGiStarterKit.product, they seem to be build, but does anyone know if they are used anywhere?

@jonahgraham
Copy link
Contributor

And for example equinox-sdk.product

I assume (guess) that is what is used to make equinox-SDK-4.28.zip from https://download.eclipse.org/equinox/ - once upon a time https://download.eclipse.org/equinox/ was linked from https://www.eclipse.org/downloads/ (e.g. this archived version)

@HannesWell HannesWell force-pushed the removeUnusedProducts branch 2 times, most recently from 62638c8 to d51b0d0 Compare July 28, 2023 20:49
@HannesWell
Copy link
Member Author

HannesWell commented Jul 28, 2023

And for example equinox-sdk.product

I assume (guess) that is what is used to make equinox-SDK-4.28.zip from https://download.eclipse.org/equinox/ - once upon a time https://download.eclipse.org/equinox/ was linked from https://www.eclipse.org/downloads/ (e.g. this archived version)

Thanks, yes that seems to be used.

Regarding eclipse-junit-tests, I have the impression that eclipse-junit-tests is the product used to run the nightly I-builds tests. So it makes sense to keep it as a product.

But the rcp.config/config.product seems not to be build. It is co-located to a feature and the pom has packacking eclipse-feature, so the product should not be considered at all. I also cannot find any reference to it, in the build feature. Consequently I believe it can be removed, too (together with its p2.inf and pom.xml file).

@HannesWell HannesWell marked this pull request as ready for review July 28, 2023 21:55
@sravanlakkimsetti
Copy link
Member

I am not sure about removing rcp product. Does this not used in building rcp products.

@merks
Copy link
Contributor

merks commented Jul 31, 2023

All these products are currently in the update site:

image

One cannot say whether or not these are used downstream...

There do seem to be "too many" products though...

@sravanlakkimsetti
Copy link
Member

I can confidently say
Used:

  1. Eclipse Platform
  2. Eclipse SDK
  3. eclipse-junit-tests

Unused:

  1. Eclipse Platform SDK

The other two not sure about the other two

@merks
Copy link
Contributor

merks commented Jul 31, 2023

@sravanlakkimsetti

Very true. We can definitely say which ones we know are used. 👍 It's the unknown we can't know.

The Oomph product catalogs use these IUs:

  • org.eclipse.sdk.ide
  • org.eclipse.platform.ide

I.e., these:

image

@HannesWell
Copy link
Member Author

HannesWell commented Jul 31, 2023

At the moment this PR suggest to remove these four products:

  1. uid org.eclipse.platform.sdk, name Eclipse Platform SDK
  2. uid org.eclipse.rcp.id, Eclipse RCP
  3. uid org.eclipse.rcp.sdk.id, name Eclipse RCP SDK
  4. uid org.eclipse.rcp.configuration

The Oomph product catalogs use these IUs:

* org.eclipse.sdk.ide

* org.eclipse.platform.ide

These are definitely retained, I also see them being build.

I can confidently say Used:

1. Eclipse Platform

2. Eclipse SDK

3. eclipse-junit-tests

Unused:

1. Eclipse Platform SDK

The other two not sure about the other two

The first three you mentioned are retained and the definitively unused one is in the list to be removed.

All these products are currently in the update site:

image

Which repo is this exactly? https://download.eclipse.org/eclipse/updates/4.29-I-builds/?
Nevertheless I wonder how these get included, because AFAICT all the products proposed for removal are not build and one can therefore not be sure if they are even working.
I have to admit I don't even know how one can obtain these products?

@merks
Copy link
Contributor

merks commented Jul 31, 2023

I generate these reports:

https://download.eclipse.org/oomph/archive/reports/index.html

Maybe these got added when "include all requirements" was added...

Let me check an older repository...

@merks
Copy link
Contributor

merks commented Jul 31, 2023

@sravanlakkimsetti
Copy link
Member

@merks these 6 products are there from quite some time all the way going back to 4.4.

I do believe these are also generated as categories in the repositories. I am not able to find the way.
We need to find out whether the rcp product is used while creating rcp application. If not used we can go ahead with the removal of rcp products as well

@HannesWell
Copy link
Member Author

We need to find out whether the rcp product is used while creating rcp application. If not used we can go ahead with the removal of rcp products as well

Do you mean if one selects a product (and not an application) to run in another product definition file?
Those 'products' are defined via extension points in Plugins and would not be affected by this removal.

@HannesWell
Copy link
Member Author

I generate these reports:\n\nhttps://download.eclipse.org/oomph/archive/reports/index.html

Thanks Ed for the links.
I wonder how they are added to a p2 repo?
I didn't even know that one can add more than one product to a repo, I know know this for repos build for one product.

I do believe these are also generated as categories in the repositories. I am not able to find the way.

If the products are used to define categories, we should probably better add them the known way via the category.xml

But yes there seems to be a lot of magic involved.

@mickaelistria
Copy link
Contributor

I believe the "Eclipse SDK" covers "Eclipse Platform SDK" and the "Eclipse RCP SDK" and I never recalled anyone using it or needing it for a while. It's IMO fine to try to remove those. They're more or less minimal Platform or RCP + Sources, which do not make much sense for any common use case:

  • developing Eclipse Platform, or JDT, or PDE: use Eclipse SDK
  • developing an Eclipse RCP product: use Eclipse SDK or EPP product

I think those are remains of older times when the RCP "team" was more distinct from the Platform (JDT, PDE...) team. We're not in need of this separation any longer.

The Eclipse RCP product still makes sense as a basis for RCP producers who want to start with a simple product instead of defining their own one. I would keep it a bit longer.

@vogella
Copy link
Contributor

vogella commented Aug 17, 2023

+1 for removing the RCP products, even if someone is using it, it would be easy for them to update their target platform

@HannesWell
Copy link
Member Author

The Eclipse RCP product still makes sense as a basis for RCP producers who want to start with a simple product instead of defining their own one. I would keep it a bit longer.

Do you mean as a template? Because I have not yet understood how one could use those products that are only defined as meta-data in a p2-repo, if they are not provided as standalone download?
I noticed that I can add them as unit in a InstallableUnit location in a Target, e.g. as
<unit id="org.eclipse.platform.sdk" version="0.0.0"/>
and it resolves.
But what can I do with that then? AFAIK you cannot reference other products in a product. I cannot even find an Editor that contains it, besides the resolution overview of the Target-Editor.
The only use-case I see is Oomph, but as Ed said, the products proposed for removal are not used in the standard product catalogs.
I assume that one could materialize the product with some P2 application only from the metadata, but is anybody doing that?

@merks
Copy link
Contributor

merks commented Aug 19, 2023

Certainly the p2 director application can create products from such things. It seems to me too that in the past, that EPP did reuse the platform's product IU. I found this bug related to that:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=580807

it looks like they were using p2.inf to "require" the product IU.

So I think you are correct that it's not so directly easy to reuse a product IU given it's neither a bundle nor a feature...

@mickaelistria
Copy link
Contributor

Because I have not yet understood how one could use those products that are only defined as meta-data in a p2-repo

You can install them with p2 (and from here one can create a viable workbench product just by using p2 director in scripts to include the RCP product + a few features, no Tycho involved), or reference them as target product with Tycho.

@HannesWell
Copy link
Member Author

Because I have not yet understood how one could use those products that are only defined as meta-data in a p2-repo

You can install them with p2 (and from here one can create a viable workbench product just by using p2 director in scripts to include the RCP product + a few features, no Tycho involved), or reference them as target product with Tycho.

Thanks for the clarification.
But I wonder if this is really done by anyone or if we could just remove them nevertheless?
These products are in general not very complicated. So I assume that everyone that can use the P2-Director is also capable to build a similar product with with a product-file a target and Tycho.
Maybe I'm missing something, but I see no great advantages in using scripts rather than Tycho+Maven and a target-file?
You probably need an Eclipse-Installation to run the P2-Director, don't you? And one also has to define from where the director can gets all the Features and Plugins in the Product to materialize.

@mickaelistria
Copy link
Contributor

But I wonder if this is really done by anyone or if we could just remove them nevertheless?

I used it something like 8 years ago.

So I assume that everyone that can use the P2-Director is also capable to build a similar product with with a product-file a target and Tycho.

I think this assumption is not always true. I also imagine some legacy projects still build with ant and p2.director tasks.

Maybe I'm missing something, but I see no great advantages in using scripts rather than Tycho+Maven and a target-file?

Something like

wget https://download.eclipse.org/eclipse/R...../org.eclipse.rcp_linux.gtk.x86_64.tar.gz
tar xzf org.eclipse.rcp_linux.gtk.x86_64.tar.gz
eclipse/eclipse -noSplash -product org.eclipse.equinox.p2.director -repository http://my.org/myEclipsePlugin/ -installIO my.org.myEclipsePlugin

is much more trivial than Tycho+Maven to assemble a working distribution.
I suspect many people start with this trivial approach when experimenting RCP stuff, and then, once it's demonstrated as viable enough, they go further and consider branding and eventually generate their .product and learn how to build it.
I'm afraid removing the RCP product would cut a low-hanging fruit for newcomers.

@merks
Copy link
Contributor

merks commented Aug 22, 2023

Another question here is whether these old things are actually maintained such that they actually work?

E.g., look at the rather large difference here:

image

I would expect that anyone build anything would be better off using the platform.product and not rcp.product. Further more, if we don't actually test that someone can build a functional product from the rcp.product then we may be worse of keeping it than deleting it.

@mickaelistria
Copy link
Contributor

I think this last comment from @merks is very valid and worth being the decisive argument: if this product is not working and no-one has complained, then we can remove it.
I also believe I was confused between the platform.product and the rcp.product: if RCP doesn't include p2, it's not the one I was referring to earlier, so my former argument is invalid.
Overall, I'm now OK with removal of rcp.product. in worst case, someone complains and contributes its come back.

@akurtakov
Copy link
Member

So what is the current status here?

@HannesWell
Copy link
Member Author

Done with eclipse-equinox/p2#316.

This is merged.

But for the publication part I found nothing and therefore assume it is just added auto-magically and will simply be ignored when removed.

Does anybody have a better idea instead of hoping that nothing needs to be adjusted manually?

Is there any objection to merge this now?
I assume the master is now open (although I didn't receive an official announcement for that) and we could merge this if everybody is fine with this?

@akurtakov
Copy link
Member

Merging would be easiest way to figure if smth breaks

@HannesWell
Copy link
Member Author

HannesWell commented Sep 13, 2023

Merging would be easiest way to figure if smth breaks

Agree.

So unless somebody objects, I'll merge this at Friday evening and it can be fixed at the weekend, if something breaks, without disturbing work-day development.

@HannesWell
Copy link
Member Author

Just compared the metadata of the p2-repo build with this change and for the master and while the repo for the master (like the nightly-build repo) contains all the products located in eclipse.platform.releng.tychoeclipsebuilder/eclipse.platform.repository, those removed with this PR are not in the p2-repo build for this PR anymore.

Therefore I have high hopes that everything just works auto-magically.
I assume Tycho just adds products to a p2-repo if their definition file is co-located to a category.xml file (like in this case). In the past I was already wondering if one can add a product to a repo. Looks like this is a way. As a side-note: It would probably be convenient to be able to add it explicitly and have support in PDE for that.

Furthermore the Product's with uid org.eclipse.rcp.sdk.id and org.eclipse.platform.sdk are part of Eclipse (Platform)'s SimRel contribution and need to be removed for the next milestone.
I'll provide a PR once the simrel repo has been migrated to GH: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/3624

@HannesWell HannesWell merged commit 6d9be33 into eclipse-platform:master Sep 15, 2023
2 checks passed
@HannesWell HannesWell deleted the removeUnusedProducts branch September 15, 2023 18:00
@HannesWell
Copy link
Member Author

Looks like this change caused the failure of tonights I-build:

01:00:03.685  /home/jenkins/agent/workspace/Builds/I-build-4.30/eclipse.platform.releng.aggregator/eclipse.platform.releng.aggregator/cje-production/gitCache/eclipse.platform.releng.aggregator/eclipse.platform.releng.tychoeclipsebuilder/repos/buildAll.xml:9: The following error occurred while executing this line:
01:00:03.685  /home/jenkins/agent/workspace/Builds/I-build-4.30/eclipse.platform.releng.aggregator/eclipse.platform.releng.aggregator/cje-production/gitCache/eclipse.platform.releng.aggregator/eclipse.platform.releng.tychoeclipsebuilder/repos/platformrepo.xml:5: The following error occurred while executing this line:
01:00:03.685  /home/jenkins/agent/workspace/Builds/I-build-4.30/eclipse.platform.releng.aggregator/eclipse.platform.releng.aggregator/cje-production/gitCache/eclipse.platform.releng.aggregator/eclipse.platform.releng.tychoeclipsebuilder/repos/platformrepo.xml:35: Unable to find: Installable Unit [ id=org.eclipse.platform.sdk version= ] 

I investigate and try to fix it.

@merks
Copy link
Contributor

merks commented Sep 16, 2023

It seems to me the "platform" target and the "platformSource" target are a bit inconsistent:

image

I would sort of expect the latter to have "source" features corresponding to the former's features.

At least for this problem I would expect the latter should use org.eclipse.sdk.ide (and should have done so in the past)...

@HannesWell
Copy link
Member Author

I would sort of expect the latter to have "source" features corresponding to the former's features.

At least for this problem I would expect the latter should use org.eclipse.sdk.ide (and should have done so in the past)...

Agree. But I wonder where the org.eclipse.platform-${buildId}.zip respectively org.eclipse.platform.source-${buildId}.zip is made available? I would like to check their current content, but couldn't find it at
https://download.eclipse.org/eclipse/downloads/drops4/I20230915-0750/

@HannesWell
Copy link
Member Author

It seems to me the "platform" target and the "platformSource" target are a bit inconsistent:

There are no org.eclipse.rcp.configuration.source and org.eclipse.equinox.executable.source feature in the current's I-build repo. I assume that's the reason why they are not included.

@merks
Copy link
Contributor

merks commented Sep 16, 2023

This URI will help explore the file system:

https://download.eclipse.org/justj/?file=eclipse/downloads/drops4/I20230915-0750/

@merks
Copy link
Contributor

merks commented Sep 16, 2023

I guess it's these:

image

HannesWell added a commit to HannesWell/eclipse.platform.releng.aggregator that referenced this pull request Sep 16, 2023
... to replace the removed org.eclipse.platform.sdk

Additionally remove the unused
org.eclipse.platform.releng.tychoeclipsebuilder/rcp/pom.xml. From the
history it looks like it was introduced as pom fro the also removed
rcp.product.

Follow-up on
eclipse-platform#1235
HannesWell added a commit to HannesWell/eclipse.platform.releng.aggregator that referenced this pull request Sep 16, 2023
... to replace the removed org.eclipse.platform.sdk

Additionally remove the unused
org.eclipse.platform.releng.tychoeclipsebuilder/rcp/pom.xml. From the
history it looks like it was introduced as pom fro the also removed
rcp.product.

Follow-up on
eclipse-platform#1235
@HannesWell
Copy link
Member Author

HannesWell commented Sep 16, 2023

I guess it's these:

image

Yes, they look like those assembled with that. But I still wonder if they are somewhere presented more prominently, because only there I guess literally nobody will find them 😅

Also the content looks pretty much like a sub-set of the I-builds repo.

HannesWell added a commit to HannesWell/eclipse.platform.releng.aggregator that referenced this pull request Sep 16, 2023
... in order to replace the removed org.eclipse.platform.sdk product
definition.

Additionally remove the unused
org.eclipse.platform.releng.tychoeclipsebuilder/rcp/pom.xml.
From the history it looks like it was introduced as pom for the now also
removed rcp.product.

Follow-up on
eclipse-platform#1235
HannesWell added a commit that referenced this pull request Sep 16, 2023
... in order to replace the removed org.eclipse.platform.sdk product
definition.

Additionally remove the unused
org.eclipse.platform.releng.tychoeclipsebuilder/rcp/pom.xml.
From the history it looks like it was introduced as pom for the now also
removed rcp.product.

Follow-up on
#1235
@HannesWell
Copy link
Member Author

The I-build that included the changes from #1360 succeeded.

Thanks to everyone that participated in this PR to help finding usages of the retained products and to investigate the failure due to this change.

@HannesWell
Copy link
Member Author

Just created eclipse-simrel/simrel.build#4 to remove the product definitions also from SimRel as last step.

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.

7 participants