search

eclipse-equinox / p2

Eclipse Public License 2.0
14 stars 39 forks source link

Enhance p2 director with "addJREIU" option #484

Open raghucssit opened 5 months ago

raghucssit commented 5 months ago

Support "addJREIU" option for DirectorApplication. Similar to UpdateSitePublisherApplication. Background for this enhancement is from https://github.com/eclipse-equinox/p2/issues/450

iloveeclipse commented 5 months ago

@merks: I think we should document this in N&N? Or is there a better place for p2 director options documentation?

laeubi commented 5 months ago

@merks: I think we should document this in N&N? Or is there a better place for p2 director options documentation?

There is https://help.eclipse.org/latest/topic/org.eclipse.platform.doc.isv/guide/p2_director.html

iloveeclipse commented 5 months ago

@merks: I think we should document this in N&N? Or is there a better place for p2 director options documentation?

There is https://help.eclipse.org/latest/topic/org.eclipse.platform.doc.isv/guide/p2_director.html

OK, so we should update that.

merks commented 5 months ago

@iloveeclipse

Note this part about the argument information being generated:

https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/9a3b53da0c6845c7c9a8a5bb78423503d68e12f2/eclipse.platform.common/bundles/org.eclipse.platform.doc.isv/guide/p2_director.html#L148

That code is commented out here:

image

raghucssit commented 5 months ago

@merks: I think we should document this in N&N? Or is there a better place for p2 director options documentation?

There is https://help.eclipse.org/latest/topic/org.eclipse.platform.doc.isv/guide/p2_director.html

OK, so we should update that.

Documentation is available at https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/master/eclipse.platform.common/bundles/org.eclipse.platform.doc.isv/guide/p2_director.html I will add this option there.

iloveeclipse commented 5 months ago

With https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/pull/1915 merged this can be closed. Thanks Raghu for the fix & Ed for review and very helpful hints!

iloveeclipse commented 5 months ago

Doesn't work as expected. Instead of adding Java 21 it adds Java 17. Build is running on java 21.

laeubi commented 5 months ago

Doesn't work as expected. Instead of adding Java 21 it adds Java 17. Build is running on java 21.

It doesn't really matter what the build is running but the director process, just in case... also keep in mind that this will not add anything it will only make it available during the director calls (what might or might not add things to the profile).

iloveeclipse commented 5 months ago

Everythingin the build / install process runs on Java 21

iloveeclipse commented 5 months ago

I guess what happens is that a.jre.javase for Java 17 is already present in the SDK and so the new option simply silently does nothing.

laeubi commented 5 months ago

I'm not sure what is your expectation here or where you seeing what, but the generated IU will just be transient unless you explicitly install it, so if you put a breakpoint there you should see that it is added as an extra unit but ti later will not show up anywhere.

iloveeclipse commented 5 months ago

Sure I'm also trying to install it. Is it possible, doing both actions in same director call depends on processing order of arguments? So if install is called first, and addjre is processed after that, the Java 17 is installed before Java21 is added? I'm currently on vacation, will follow up in a week.

laeubi commented 5 months ago

You usually install things on top of others, so some units are there (inital + extra units), some are requested to be installed (root units) and the units you provide (updatesites).

The all root IUs are selected + their dependencies (if not already part of initial + extra) and added to the profile.