eclipse-pde / eclipse.pde

Eclipse Public License 2.0
25 stars 61 forks source link

copy bin-included resources to the output folder #327

Open laeubi opened 1 year ago

laeubi commented 1 year ago

This is the counter-part of the Tycho issue:

Currently one can define certain resources to be included in "the binary build" but those are not copied to the output folder but when running a launch the project itself is made available as bundle content.

This has some drawbacks:

  1. Whether or not something is included is not really taken into account, and in the IDE everything is accessible which can easily lead to the situation that a missing entry is only discovered when the plugin is deployed (e.g. a icon is not showing up), because in the IDE one won't notice that and at build time this can not be detected.
  2. Tools that work on the JDT classpath (e.g. plain JUnit Test, a Main Class, APT Generators, ...) can't see these resources and fail

Thus I'd like to propose that the PDE Builder (ManifestBuilder or SchemaBuilder) is enhanced to copy the resources from the bin-includes to the output folder and then (maybe in a later step) no longer add the root of the project when launching products/osgi frameworks.

merks commented 1 year ago

I'm not sure the impact this is supposed to have on built-in URLs like this and how FileLocator resolves them:

platform:/plugin/org.eclipse.oomph.ui -> file:/D:/Users/merks/oomph-1.26/git/org.eclipse.oomph/plugins/org.eclipse.oomph.ui/
bundleentry://151.fwk1840194473/ -> file:/D:/Users/merks/oomph-1.26/git/org.eclipse.oomph/plugins/org.eclipse.oomph.ui/

It seems to me that without the "maybe later step" such a change would simply result in duplicates on the classpath with no other benefit. And if it's not "maybe later," I wonder how many assumptions are broken elsewhere and whether that offset the benefit...

HannesWell commented 1 year ago
1. Whether or not something is included is not really taken into account, and in the IDE **everything** is accessible which can easily lead to the situation that a missing entry is only discovered when the plugin is deployed (e.g. a icon is not showing up), because in the IDE one won't notice that and at build time this can not be detected.

That is a point that hit me and my colleges regularly (the frequency declines with the experience but it still happens). A while ago I even wrote a wrapper that checks if the requested resource is specified in the bin.includes if a product is launched from the IDE. But of course this does not scale well. Such 'filtering'-approach would only be useful if built-in into the EquinoxClassloader and EquinoxBundle when calling getResource() respectively getEntry(). But of course Equinox does not know PDE's specific so that would probably only be done reasonably if a resource filter can be injected in development mode of the Equinox-Framework.

Copying the resources to the bin folder as well and using that folder as bundle root would be an elegant change that can be done in PDE only. The drawback is that resources are duplicated in the workspace. That is OK for a few small icons, but can increase the workspace size significantly if a Plug-in has big resources like embedded executables of other programs that can have several hundredths of Megabytes (or maybe even more).

Whichever way this is addressed, if we find a solution for that it would make PDE especially more friendly for beginners.

mickaelistria commented 1 year ago

I believe the bundle/URL mapping is provided by the PDE Launch. So the PDE Launch would need to be tweaked too. Once PDE Launch is changed to reference the bin/ folder and this bin/ folder contains a complete pre-package of the bundle, I don't think FileLocator would need further work.

But the issue is the one pointed by @HannesWell : duplication of files may cause other confusions (eg someone working on the derived plugin.xml or the derived MANIFEST.MF by mistake) and also extra resource (disk/CPU/RAM) consumption. I'm afraid that if we do not do it perfectly (who knows what that means yet?), then we'll basically result in a more resource-hungry builder that would replace some existing source of confusion by a new source of confusion, so the outcome may even be overall negative...

laeubi commented 1 year ago

I'm not sure the impact this is supposed to have on built-in URLs like this and how FileLocator resolves them

As these URLs are resolved at runtime, they are not really affected in functionality but might point to the then changed location (e.g. if they point to a real file).

It seems to me that without the "maybe later step" such a change would simply result in duplicates on the classpath

Duplicates are not really bad, as long as both entries are "in sync", what of course has to be the case.

And if it's not "maybe later," I wonder how many assumptions are broken elsewhere and whether that offset the benefit...

Without trying to move forward here, we will never know, that's why I said "maybe later", because there are two possible outcomes:

  1. We change this, and found no one is affected and thus can change this immediately
  2. There are some effects outside the platform code and we either want to make this opt-in or simply give others some time to adapt until we remove the duplicates.

The drawback is that resources are duplicated in the workspace. That is OK for a few small icons, but can increase the workspace size significantly if a Plug-in has big resources like embedded executables of other programs that can have several hundredths of Megabytes (or maybe even more).

I think embedding many hundreds of megabytes inside a bundle is a very rare case and should generally be avoided. Beside that, of course one could also link resources (if file system supports this, e.g. ntfs, ext3/4, ...)

laeubi commented 1 year ago

eg someone working on the derived plugin.xml or the derived MANIFEST.MF by mistake

As output folders are usually hidden, its very hard to do so, still an IDE can't prevent all mistakes, and when it comes to JDT it seems there is no issue at all with this where resources are always inside a source folder...

It is more that PDE is very special with its "bin-includes" stuff and people are always confused about that.

HannesWell commented 1 year ago

The drawback is that resources are duplicated in the workspace. That is OK for a few small icons, but can increase the workspace size significantly if a Plug-in has big resources like embedded executables of other programs that can have several hundredths of Megabytes (or maybe even more).

I think embedding many hundreds of megabytes inside a bundle is a very rare case and should generally be avoided. Beside that, of course one could also link resources (if file system supports this, e.g. ntfs, ext3/4, ...)

I would like to avoid that but how should we then deliver such resources as part of our products? However, is there a reason that speaks against always creating file-system links to the real included resources in the bin-folder? Then the overhead would be really small. And since since this is only used in the IDE and not in builds that should be fine, shouldn't it? Only PDE-build maybe would have to be adjusted.

laeubi commented 1 year ago

However, is there a reason that speaks against always creating file-system links to the real included resources in the bin-folder?

Don't think so, just wanted to point out that "copy" not always requires a full copy that takes additional space, e.g. linux supports even hard links and with the Java-NIO Path API all that stuff has become much more accessible to java.

And since since this is only used in the IDE and not in builds that should be fine, shouldn't it? Only PDE-build maybe would have to be adjusted

I don't think any build tool needs to be adjusted here, it is just that the "in the IDE build", build class files and copy them, but not the "bin-included things" ...