Fails to run tests with latest Tycho

[akurtakov]

Swt fails to run tests locally due to eclipse-tycho/tycho#5883 .
It all boils down to svg fragment adding all fragments as extra jars

eclipse.platform.swt/bundles/org.eclipse.swt.svg/build.properties

Line 24 in cc2762f

jars.extra.classpath = platform:/plugin/org.eclipse.swt.cocoa.macosx.aarch64,\

[akurtakov]

What I can think of is Tycho to respect Eclipse-PlatformFilter when it adds extra class path jars but that is an exercise for another day.

[laeubi]

Thanks for the report I will look into this, two things I like to check

1. Checking alternatives for SWT - adding all fragments as extra classpath looks quite convoluted and wrong
2. I don't think that the jars.extra.classpath should be filtered - it is quite clear that this is something added extra outside normal rules so if user choose to we can't forbidd / filter it - but tycho-surefire must maybe be more cautious. In the end one might ask why the fragment is attached at all ... so even maybe check equinox behavior here!

[HeikoKlare]

1. Checking alternatives for SWT - adding all fragments as extra classpath looks quite convoluted and wrong

It was the best solution we achieved when introducing the SVG fragment:

• Rasterization of SVGs at Runtime for Eclipse Icons  #1638

Supported by this change to Tycho:

• Resolve jars and library extra.classpath entries optionally eclipse-tycho/tycho#4771

Do you have an alternative in mind how we can make the SWT API (provided by the native fragements) accessible for the SVG fragment?
Conceptually, it might be cleanest to have the common SWT classes provided by the SWT host and not of every native fragment. But I guess that common and OS-specific classes are so interdependent that it will not be easily possible to extract common classes into the host. Maybe it would be possible for some specific common classes (placed "at the end" of the dependency chain, including the ones the SVG fragment needs to access).

[laeubi]

Do you have an alternative in mind how we can make the SWT API (provided by the native fragements) accessible for the SVG fragment?

Not yet but though I should reconsider the current approach ... but nothing really working well without issues or requiring changes in PDE itself...

Conceptually, it might be cleanest to have the common SWT classes provided by the SWT host and not of every native fragment.

I actually tried this out but the problem is that the "common" part references the platform specific parts (e.g. Display, Platform, ...) So I linked the common part into the svg fragment and then tried to generate stubs for the missing parts - but this becomes quite ugly as well... If we would have had #1750 one could have linked in this here but also ugly on its own.

One might think about not building the SVG fragment with Tycho at all and make it a plan maven-jar... but all these not look like a quick win here...

[HeikoKlare]

I actually tried this out but the problem is that the "common" part references the platform specific parts (e.g. Display, Platform, ...)

I played around with extracting a minimal set of common functionality required for the SVG fragment (we could actually reduce it to just the service interface). I accidentally started testing that in an official PR:

• Remove dependency of SVG fragment to native fragments #3130

But extracting parts of the common SWT classes into the SWT host has some obvious drawbacks, including:

• Common classes become scattered across the host and the fragments because interdependencies between common and OS-specific classes do not allow to move every common class to the host.
• Setup becomes more complex, as the OS-specific standalone JARs cannot be just generated from the fragment anymore but need to take into account the code within the host.

Another option to reduce complexity would of course be to drop the SVG rasterizer being optional/exchangeable and just integrate it into SWT itself. The drawback would of course be the integration of a third-party library into SWT, but on the other hand it would reduce complexity and make the SVG functionality more easily usable in pure SWT setups (without OSGi).

[laeubi]

I played around with extracting a minimal set of common functionality required for the SVG fragment (we could actually reduce it to just the service interface)

I tried it without much luck, because the interface references imagedata what references RGB and so on... I just did it a bit differently, just linking the file into an own folder then added the folder as a source folder. If it helps I can push my experiemnt somwhere... so instead of changing the structure of the host but as said this leads to compile error in my case?!? It also seem to use DPIUtil what is maybe an internal API...

One possible option would be to make the service interface more independent. e.g. instead of a true ImageData it can return a separate class (or even record) that just returns the bare data needed and one has to copy it over to ImageData on the consumer side.

Another option to reduce complexity would of course be to drop the SVG rasterizer being optional/exchangeable and just integrate it into SWT itself.

What actually makes it tricky is the part that it is a fragment of SWT at all! I wonder if this is required... making it a regular bundle would make it compile like any other thing that references SWT.. we then just would need a litte trick to load it in OSGi but this would not be that hard...

[HeikoKlare]

One possible option would be to make the service interface more independent. e.g. instead of a true ImageData it can return a separate class (or even record) that just returns the bare data needed and one has to copy it over to ImageData on the consumer side.

That's exactly what I did in my experiment: I made SVGRasterizer independent from anything else in SWT. the JSVGRasterizer transforms an AWT BufferedImage to an SWT ImageData anyway, so that you can also make the SVGRasterizer return the BufferedImage. The idea of having it return ImageData was that you finally need SWT ImageData anyway, so it would be best if the rasterizer would directly generate it (without additional transformations). Using an AWT BufferedImage would result in always having a transformation applied within SWT (to ImageData) and maybe even having another transformation in a concrete rasterizer that does not yield a BufferedImage but anything else.
So your proposal of having some bare/independent data representation (that is easy to transform into SWT ImageData) could help to have the right level of abstraction.

What actually makes it tricky is the part that it is a fragment of SWT at all! I wonder if this is required... making it a regular bundle would make it compile like any other thing that references SWT.. we then just would need a litte trick to load it in OSGi but this would not be that hard...

That's a valid point. I think the idea back then was to have the fragment loaded with the host (in case it is present) so that consumers don't need to specifiy any dependency to / inclusion of an additional bundle. But we then decided to use an OSGi capability for the rasterizer as well, which, if I am not mistaken, makes the rationale for using a fragment obsolete: the fragment/bundle will be pulled in by an according required capability, so that it could just be an independent bundle.

[laeubi]

So your proposal of having some bare/independent data representation (that is easy to transform into SWT ImageData) could help to have the right level of abstraction.

The idea here was that likely the ImageData is just some arrays so it can be come a rather trivial copy operation.
That would allow theoretical other Rasterizer as well.

Then one can extract SVGRasterizer + an (imaginary) SVGRasterizerResult into an own source folder, and then link this source folder as usual into the jsvg bundle, configure it to use an own output folder and simply not include it in the bin.includes. This will then be compiled twice but not included twice and I think that's fine given we compile all common SWT code already 6 times.

That's a valid point. I think the idea back then was to have the fragment loaded with the host (in case it is present) so that consumers don't need to specifiy any dependency to / inclusion of an additional bundle.

As we using the require-capabilty here it will be included automatically anyways... given we even already need SPIFly due to slf4j we even would not need any special "tricks" if I think further here it should work out-of-the box already if we include the right SPI headers...

[laeubi]

I now checked "Alternative 2" - making it a regular bundle but this is currently blocked by eclipse-tycho/tycho#5916 - I hope to fix it in this release but as my current time is limited I don't want to promise too much.

So I think the most promising (intermediate) solution would be "Alternative 1" - making the API independent and I'll try to propose a PR for that.

Another "Alternative 3" would be to make this configurable as proposed in the original PR here we just found it to not implement it the time back.

[HeikoKlare]

I now checked "Alternative 2" - making it a regular bundle but this is currently blocked by eclipse-tycho/tycho#5916 - I hope to fix it in this release but as my current time is limited I don't want to promise too much.

If I understand the Tycho issue correctly, it's about the capability provider (in our case the SVG bundle) having a dependency to the API bundle (in our case the SVG host/fragments) and the API bundle requiring the capability, leading to a a dependency cycle. Is that correct? I ask because the that's currently not the case: SWT host/fragments do and should not require the capability, but the actual consumer bundles (those using SVGs) do. That's how it is implemented now with the goal to make SWT usable without an SVG rasterizer implementation in case SVGs are not used. So maybe the approach is feasible even without any improvements to Tycho?