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

inconsistent source.info with p2 director based installations #350

Open
gireeshpunathil opened this issue Oct 10, 2023 · 11 comments
Open

inconsistent source.info with p2 director based installations #350

gireeshpunathil opened this issue Oct 10, 2023 · 11 comments

Comments

@gireeshpunathil
Copy link

A user reported that they get source.info file modified in an unexpected manner.

steps to reproduce:

  • eclipse -noSplash -application org.eclipse.equinox.p2.director -repository https://download.eclipse.org/eclipse/updates/4.29/R-4.29-202309031000/ -installIU org.eclipse.sdk.ide -profile SDKProfile -destination ./target -configuration ./target/configuration -roaming
  • observe that target/configuration/org.eclipse.equinox.source/source.info lists the source bundles from both instances: target and the builder
  • install yet another feature: eclipse -noSplash -application org.eclipse.equinox.p2.director -repository https://download.eclipse.org/eclipse/updates/4.29/R-4.29-202309031000/ -installIU org.eclipse.swt.tools.feature.feature.group,org.eclipse.swt.tools.feature.source.feature.group -profile SDKProfile -destination ./target -configuration ./target/configuration
  • source.info now lists only the plugins from the builder instance.
@gireeshpunathil
Copy link
Author

gireeshpunathil commented Dec 8, 2023

what would be the reason to do this here? this results in the entire bundles in the target to be copied into the source info file (of the installation config)

Either there is a different use case that I am not following, or P2 gets confused when invoked in this manner?

@gireeshpunathil
Copy link
Author

looks like this is the infliction point.

  • in the passing case, there is no existing configuration, so this returns null. that leads to bundle names of only IUs being written into the source.info file
  • in the failing case, there is an existing configuration (who created that?) subsequently, the entire list of bundles from the target is loaded, which is appended with the IU list, causing the entry duplication

open questions:

  • is loading target bundles in source.info a needed thing in some use cases?
  • not doing that is a good solution here?
  • or assuming there are scenarios, dee-duplicating the bundle info at the end (towards save config) is a more pragmatic approach?

@gireeshpunathil
Copy link
Author

being unfamiliar with the p2 logic, some guidance would be appreciated here in terms of what is the expected behaviour. This is what I would think is a reasonable behaviour:

  • if builder path is different from the destination, the source.info should list only the incoming IUs.
  • if builder path is same as the destination, the source.info should list both the incoming IUs as well as the builder's inherent bundles.

@merks
Copy link
Contributor

merks commented Dec 11, 2023

Before you raised this issue, I had no idea that a sources.info file exists, it's purpose, nor how it's managed.

I imagine that the behavior of installing via p2 directory should be the same as installing into the running IDE, i.e., the same as for self updates/installs.

While the Eclipse SDK ships that file, when I look in the EPP packages, I see they don't ship that file:

image

Perhaps that's because Tycho invokes the p2 director multiple times for the install="root" features making the incomplete source.info file kind of unless.

Unfortunately I don't think the people who designed this thing are around to give guidance. Looking at how eclipse/configuration/org.eclipse.equinox.simpleconfigurator/bundles.info is correctly maintained is probably the best guidance in this case.

@laeubi
Copy link
Member

laeubi commented Dec 11, 2023

@gireeshpunathil I have no clue either, maybe you can ask the user that report this about

  1. Why is it "unexpected", e.g. what is the expectation at all, its not clear to me
  2. What is the problem the user sees with the file changes in an inappropriate way e.g. is there a stacktrace or missing / failing functionality

@merks
Copy link
Contributor

merks commented Dec 11, 2023

FYI, I did this search:

image

So from this I gather that PDE uses that information too, when available.

I see this in a SDK zip:

image

And that looks verify similar to this:

image

So I fully expect the file should list, for each bundle int eh bundles.info for which a corresponding source bundle exists, the corresponding information for that bundle in the source.info.

@gireeshpunathil
Copy link
Author

@gireeshpunathil I have no clue either, maybe you can ask the user that report this about

  1. Why is it "unexpected", e.g. what is the expectation at all, its not clear to me
  2. What is the problem the user sees with the file changes in an inappropriate way e.g. is there a stacktrace or missing / failing functionality

while I await user response, here is what it shows in my reproduce:

#encoding=UTF-8
#version=1
com.ibm.icu.source,72.1.0,../../../e-prod/4.30/Eclipse.app/Contents/Eclipse/plugins/com.ibm.icu.source_72.1.0.jar,-1,false
com.ibm.icu.source,67.1.0.v20200706-1749,plugins/com.ibm.icu.source_67.1.0.v20200706-1749.jar,-1,false

the first entry is from the builder, and the second entry is from the IU repo. As I am not running any application with this, I am not sure what damage it can cause. However, it is my assertion that the builder's bundle info entering into the target installation does not make sense! because, it will then imply that the installation will depend on the p2.director instance that is used for installation!

@laeubi
Copy link
Member

laeubi commented Dec 11, 2023

It is jsut a guess but it might be PDE needs this for old pre OSGi bundles to find out what are the sources "plugins" while today it uses the Eclipse-Source-Bundle header and probably other sources so it might be that the file is irrelevant anyways.

Second guess it to find sources from the eclipse install even if another target platform is currently choose... al of this sounds quite fragile and more legacy than useful today.

@gireeshpunathil
Copy link
Author

from the user:

So after step 3 the "source.info" file of the installed Eclipse loses 
all the source bundles from this instance.

The reproducing steps I gave you mimic exactly what our product 
"IBM Installation Manager" does. The IBM Installation Manager is 
Eclipse based and it uses the Eclipse p2 engine to install, modify,etc. 
other Eclipse based products (RAD, RSE, etc.).  The problem for us is 
that the products that we install via p2 will have messed up entries in 
their configuration file "source.info"  and hence I expect that the 
customers of these products could run into issues related to "missing source files".

@gireeshpunathil
Copy link
Author

So I fully expect the file should list, for each bundle int eh bundles.info for which a corresponding source bundle exists, the corresponding information for that bundle in the source.info.

thanks @merks . I am planning to go line by line in the list and see if there is a consumer for this file and then what the consumer does with it. that may give us some clues as to what is the implied damage.

@laeubi
Copy link
Member

laeubi commented Dec 13, 2023

I expect that the
customers of these products could run into issues related to "missing source files"

That's quite vague to confess, so checking what this file does (and probably remove/replace) seems a good way forward, I would assume that additional entries can't harm and probably because source bundles are not installed along with the request to modify eclipse, but they can be seen from the repository that installs the binary so they are added as "best effort".

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

No branches or pull requests

3 participants