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

dev.properties provided extra classpath items are invisible to #389

Open
laeubi opened this issue Oct 30, 2023 · 9 comments
Open

dev.properties provided extra classpath items are invisible to #389

laeubi opened this issue Oct 30, 2023 · 9 comments

Comments

@laeubi
Copy link
Member

laeubi commented Oct 30, 2023

Bundle.getEntry(..) and Bundle.findEntries(...) currently do not find items from extra classpath provided by dev.properties, this is inconsistent to how a deployed bundle would operate and currently requires some ugly hacks.

@tjwatson
Copy link
Contributor

Bundle.getEntry(..) and Bundle.findEntries(...) do not search the classpath. They only search the base bundle file.

@laeubi
Copy link
Member Author

laeubi commented Oct 30, 2023

They only search the base bundle file.

Well these are actually meant as those e.g. eclipse put things in the bin/ folder, every Resource there is not found (unless you build a "real" Jar) thats not what a user would expect.

@tjwatson
Copy link
Contributor

Well these are actually meant as those e.g. eclipse put things in the bin/ folder, every Resource there is not found (unless you build a "real" Jar) thats not what a user would expect.

The docs state that the osgi.dev option is for classpath entries:

https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fmisc%2Fruntime-options.html&resultof%3D%2522%252d%2564%2565%2576%2522%2520%2522%2564%2565%2576%2522%2520&anchor=osgidev

The framework treats them as extra Bundle-Classpath entries. It was never meant to be some merging of loose content into a single representation of the base bundle file.

@laeubi
Copy link
Member Author

laeubi commented Oct 30, 2023

The framework treats them as extra Bundle-Classpath entries.

Yes but technically they are not Bundle-Classpath entries, as the later will be placed in the "base bundle file", this currently limits PDE (and Tycho) capability to not generate data into the project root (where generated data do not belongs to) and requires to add additional build.properties entries (because content can not be placed in the output folder at the first place) and makes code that uses entry scans (e.g. to find .class files or .properties in packages) fail.

So the goal would be to either have another way or simply treat them as "base bundle file" items.

@tjwatson
Copy link
Contributor

So the goal would be to either have another way or simply treat them as "base bundle file" items.

I think it would need to be a separate mechanism because osgi.dev has always been for extra entries to the class loader, not for extra content to the base bundle.

The classpath dev mode is implemented by a org.eclipse.osgi.internal.hookregistry.ClassLoaderHook (the implementation org.eclipse.osgi.internal.hooks.DevClassLoadingHook). That has only ever simply added extra classpath's to the bundle class loader. To augment content for the base bundle file you would need to implement a org.eclipse.osgi.internal.hookregistry.BundleFileWrapperFactoryHook instead. But I don't think you can get rid of the extra classpath behavior entirely because there is logic in there also for fragments to make sure the fragment classpath's in dev mode are also added to the host classloader classpath.

@HannesWell
Copy link
Member

Besides eclipse-pde/eclipse.pde#841 another use-case of that would be eclipse-platform/eclipse.platform#790.

To augment content for the base bundle file you would need to implement a org.eclipse.osgi.internal.hookregistry.BundleFileWrapperFactoryHook instead.

Thanks for that suggestion, I looked into that and this looks promising and I started to work on a DevBundleFileWrapperHook.
But in my first experiments I noticed some interference with the DevClassLoadingHook, that creates NestedDirBundleFile for each dev-classpath entry.
Furthermore EquinoxConfiguration.getDevClassPath() expects a BundleRevision from which it gets the the bsn and version to look-up in the dev.properties. But that's of course not available yet when the hook is called. Not even the Manifest can be read.
So reading the dev.properties has to be delayed until when the first search for an entry is done.
But this has the disadvantage that all BundleFiles have to the wrapped.
However, one heuristic would be to check if the Generation.getContent() returns a File to a directory.
Since bundles with shape dir are usually rare, but bundles from a primary workspace are always directories and if some plugins like BND-Tools launches workspace projects as jars, then the entire problem of this issue does not aries. So I think false negatives cannot happen with that.

@tjwatson
Copy link
Contributor

But this has the disadvantage that all BundleFiles have to the wrapped.

I don't see this as a disadvantage and if it simplifies things just do that if in devmode.

@laeubi
Copy link
Member Author

laeubi commented Dec 18, 2023

@HannesWell any updates here? This would really be useful.

@HannesWell
Copy link
Member

@HannesWell any updates here? This would really be useful.

Not yet, but I have it on my TODO List. Resolving interference with the existing DevClassLoadingHook make this more difficult than initially assumed.

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