eclipse-packaging / packages

Eclipse IDE product definitions.
Eclipse Public License 2.0
4 stars 11 forks source link

remove root features in p2.inf because Tycho can now generate this info automatically #111

Closed jonahgraham closed 5 months ago

jonahgraham commented 7 months ago

The p2.inf has some manually maintained entries that with an upcoming (or recently released) version of Tycho is no longer needed. These steps in releasing become redundant and all the corresponding parts of p2.inf can be removed:

https://github.com/eclipse-packaging/packages/blob/fb243661dfa23c2d99731c9cec803eb54578a89e/RELEASING.md#L50-L52

See https://github.com/eclipse-equinox/p2/issues/423#issuecomment-1884456597

jonahgraham commented 6 months ago

I had meant to look at this for 2024-03 M3, but because of the loss of build stability my time available for this went away. I can look at this for 2024-06 M1 next.

merks commented 6 months ago

I think this will be after the 4.31 release and then after the next Tycho release afterwards.

jonahgraham commented 6 months ago

Thanks for that extra info.

merks commented 5 months ago

@jonahgraham

I investigated this over the long weekend. Firstly I need to update the build to use Tycho 4.0.7. Then I noticed all the version updates for 4.32 are not there and now I have a whole big whack of changes:

image

Then I ran into problems with Tycho 4.0.7 using a hard coded version for advice parsing:

https://github.com/eclipse-tycho/tycho/pull/3722

I can work around that for now by removing in the update.id advice from the features.

Then I noticed that the mirror mojo mirrors every requirement of the product regardless of the state of the filter on that requirement. I see there are already hacks to remove IUs that are mirrored when not desired:

https://github.com/eclipse-packaging/packages/blob/cad2b5baca0c29cc0e547f609f12e4d3ab5979b2/releng/org.eclipse.epp.config/tools/remove-justj-from-p2.xml#L4-L21

To address this, I experimented with using the CBI p2 Aggregator to do the mirroring.

image

With the new flag, it aggregates everything (as normal because nothing is specified for the mapped repository which implies map everything) but then it excludes everything that is available in one of the validation repositories. So for a build of a single product, the repo has this content:

image

With this approach, we could eliminate all the mirror calls, and all the IU removal ant tasks, and do either a single aggregation of all the product repos (or do it once per product, accumulating the result).

Of course the requires, with filters, are present when using 4.0.7:

image

So we can definitely eliminate all the hard coded p2.inf information...

But before I go further down this path, I need to know if you agree with this approach because there are a lot of file changes already and a bit more work to use the latest CBI p2 aggregator for doing the actual aggregation.

jonahgraham commented 5 months ago

@jonahgraham

I investigated this over the long weekend. Firstly I need to update the build to use Tycho 4.0.7. Then I noticed all the version updates for 4.32 are not there and now I have a whole big whack of changes:

The endgame for 2024-06 M1 hasn't been created yet - I normally aim to do it ahead of time (especially for M1 since the change is more substantial), normally in time for the Platform contribution. Therefore I will be doing it this week. If you have a change that does it already, I will happily review and merge that though :-)

So we can definitely eliminate all the hard coded p2.inf information...

Excellent.

But before I go further down this path, I need to know if you agree with this approach because there are a lot of file changes already and a bit more work to use the latest CBI p2 aggregator for doing the actual aggregation.

This approach looks good to me.

To confirm, that doing mvn verify -Depp.p2.all -Depp.product.all will build the whole p2 site successfully and then pointing at that p2 site will build the products so that the Jenkinsfile can continue to work. See https://github.com/eclipse-packaging/packages/blob/ea92104188dee6242ddbe7532c112808b2a9620f/Jenkinsfile#L25

Doing local builds are documented here https://github.com/eclipse-packaging/packages?tab=readme-ov-file#build-locally and allow the two steps to be combined in a single command line (i.e you don't need a temporary p2 repo)

merks commented 5 months ago

I'm testing various combinations. For more immediate gratification, I have this working locally:

image

I.e., to build just one product's p2 repository and to aggregate that repository.

image

This is working already, with no ant tasks and no mirror mojos; the aggregator creates strong checksums and produces the compressed resources, so the build is significantly simplified.

Now to make it work for all products at once...

merks commented 5 months ago

Build all works as well:

image

Now for more cleanups. Speaking of which, do you know the purpose of this:

https://github.com/eclipse-packaging/packages/blob/cad2b5baca0c29cc0e547f609f12e4d3ab5979b2/releng/org.eclipse.epp.config/parent/pom.xml#L175-L193

jonahgraham commented 5 months ago

It is to convert e.g. epp.package.rcp (product pom's artifactid) to rcp to create the desired zip names here:

https://github.com/eclipse-packaging/packages/blob/647fbf6a02ae7984701de709c339811b42f256d1/releng/org.eclipse.epp.config/parent/pom.xml#L326

That pattern needs to match what the webmaster links to, which is defined in

https://github.com/eclipse-packaging/packages/blob/647fbf6a02ae7984701de709c339811b42f256d1/packages/org.eclipse.epp.package.rcp.feature/epp.website.xml#L30

merks commented 5 months ago

I see. That's why it never matches when you don't materialize the products.

Maybe it should be moved to the <id>epp.materialize-products</id> profile. Not important right now though.

merks commented 5 months ago

@jonahgraham

I wasn't able to figure out where the new splash should come from. Currently it's this:

https://raw.githubusercontent.com/eclipse-packaging/packages/master/packages/org.eclipse.epp.package.common/splash.bmp

I found this issue:

https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/3963

So Platform has a new spash, but it's not a *.bmp so I'm not sure what to do about that....

jonahgraham commented 5 months ago

Splash screen update is part of releasing steps here: https://github.com/eclipse-packaging/packages/blob/master/RELEASING.md?plain=1#L17

which links to https://github.com/eclipse-packaging/packages/blob/master/packages/org.eclipse.epp.package.common/splash/INSTRUCTIONS.md which has the instructions on how to make bmp in the correct format. (That file now needs an update to new new gitlab issue you linked to)

merks commented 5 months ago

The product catalog generator is updated to recognized the new filters so the end-to-end story is complete.