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

Move more product configuration from p2.inf to product-file #1459

Merged

Conversation

HannesWell
Copy link
Member

@HannesWell HannesWell commented Oct 22, 2023

And remove the unnecessary specification of the property org.eclipse.equinox.p2.type.product=true, which Tycho/P2 already set by
default.
Additionally unify max-memory launcher-arguments of Platform/SDK products

P2 and Tycho 4.0.4 already support names for Product Update repositories with

but I think it is good to not submit this before PDE has Editor support for that as well as described in eclipse-equinox/p2#345.

A similar simplification can then also be applied to all EPP packages.

@akurtakov
Copy link
Member

This looks very promising !

@@ -194,6 +194,13 @@ United States, other countries, or both.
<property name="org.eclipse.update.reconcile" value="false" />
<property name="eclipse.buildId" value="${unqualifiedVersion}.${buildQualifier}" />
<property name="osgi.bundles.defaultStartLevel" value="4" />
<property name="osgi.instance.area.default" value="@user.home/workspace" />
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, EPP uses eclipse-workspace here. Should we also change it for SDK products?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean that the default suggestion for the workspace should be co-located to the installation?
Yes that's something I would also support. I always annoys me if I download an SDK to test something out that I first have to define the test workspace to be somewhere else than in such prominent location.

I'll create a follow-up PR on this so that we can discuss that then.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I don't think he means that. Me means it's specified like this:

[email protected]/eclipse-workspace

I think co-locating it with the installation you have to be careful it's not a read-only installation; maybe you'd co-located it relative to the installation surrogate but might even end up with a read-only location...

And remove the unnecessary specification of the property
org.eclipse.equinox.p2.type.product=true, which Tycho/P2 already set by
default.
@HannesWell HannesWell marked this pull request as ready for review October 28, 2023 11:57
@HannesWell
Copy link
Member Author

I checked the newly build metadata and everything looks good.
As expected the properties and instructions to set the instance.default and available update-sites have been moved to other IUs and the four artifact units (two per product) are gone.

I'll merge this now and check tonight's I-build tomorrow if everything still works as expected.

@HannesWell HannesWell merged commit 5110743 into eclipse-platform:master Oct 28, 2023
4 checks passed
@HannesWell HannesWell deleted the unifyProductConfiguration branch October 28, 2023 12:10
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.

4 participants