-
Notifications
You must be signed in to change notification settings - Fork 192
Make an independent bundle out of SWT SVG fragment #3131
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
Draft
HeikoKlare
wants to merge
1
commit into
eclipse-platform:master
Choose a base branch
from
HeikoKlare:svg-as-bundle
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <classpath> | ||
| <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-21"/> | ||
| <classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/> | ||
| <classpathentry kind="src" path="src"> | ||
| <attributes> | ||
| <attribute name="test" value="true"/> | ||
| </attributes> | ||
| </classpathentry> | ||
| <classpathentry kind="output" path="bin"/> | ||
| </classpath> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <projectDescription> | ||
| <name>org.eclipse.swt.svg.tests</name> | ||
| <comment></comment> | ||
| <projects> | ||
| </projects> | ||
| <buildSpec> | ||
| <buildCommand> | ||
| <name>org.eclipse.jdt.core.javabuilder</name> | ||
| <arguments> | ||
| </arguments> | ||
| </buildCommand> | ||
| <buildCommand> | ||
| <name>org.eclipse.pde.ManifestBuilder</name> | ||
| <arguments> | ||
| </arguments> | ||
| </buildCommand> | ||
| <buildCommand> | ||
| <name>org.eclipse.pde.SchemaBuilder</name> | ||
| <arguments> | ||
| </arguments> | ||
| </buildCommand> | ||
| </buildSpec> | ||
| <natures> | ||
| <nature>org.eclipse.pde.PluginNature</nature> | ||
| <nature>org.eclipse.jdt.core.javanature</nature> | ||
| </natures> | ||
| <linkedResources> | ||
| <link> | ||
| <name>.settings/org.eclipse.jdt.core.prefs</name> | ||
| <type>1</type> | ||
| <locationURI>SHARED_SETTINGS/org.eclipse.jdt.core.prefs</locationURI> | ||
| </link> | ||
| </linkedResources> | ||
| <variableList> | ||
| <variable> | ||
| <name>SHARED_SETTINGS</name> | ||
| <value>$%7BPARENT-2-PROJECT_LOC%7D/examples/.settings_shared</value> | ||
| </variable> | ||
| </variableList> | ||
| </projectDescription> |
2 changes: 2 additions & 0 deletions
2
tests/org.eclipse.swt.svg.tests/.settings/org.eclipse.core.resources.prefs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| eclipse.preferences.version=1 | ||
| encoding/<project>=UTF-8 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| Manifest-Version: 1.0 | ||
| Bundle-ManifestVersion: 2 | ||
| Bundle-Name: Eclipse SWT Tests SVG | ||
| Fragment-Host: org.eclipse.swt.svg | ||
| Bundle-SymbolicName: org.eclipse.swt.svg.tests | ||
| Bundle-Version: 3.0.0.qualifier | ||
| Automatic-Module-Name: org.eclipse.swt.svg.tests | ||
| Bundle-RequiredExecutionEnvironment: JavaSE-21 | ||
| Import-Package: org.junit.jupiter.api;version="[5.14.0,6.0.0)", | ||
| org.junit.jupiter.api.function;version="[5.14.0,6.0.0)", | ||
| org.junit.platform.suite.api;version="[1.14.0,2.0.0)" | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| source.. = src/ | ||
| output.. = bin/ | ||
| bin.includes = META-INF/,\ | ||
| . | ||
|
|
||
| pom.model.packaging = eclipse-test-plugin | ||
| pom.model.property.testClass = org.eclipse.swt.svg.AllSVGTests |
21 changes: 21 additions & 0 deletions
21
tests/org.eclipse.swt.svg.tests/src/org/eclipse/swt/svg/AllSVGTests.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /******************************************************************************* | ||
| * Copyright (c) 2026 Vector Informatik GmbH and others. | ||
| * | ||
| * This program and the accompanying materials | ||
| * are made available under the terms of the Eclipse Public License 2.0 | ||
| * which accompanies this distribution, and is available at | ||
| * https://www.eclipse.org/legal/epl-2.0/ | ||
| * | ||
| * SPDX-License-Identifier: EPL-2.0 | ||
| *******************************************************************************/ | ||
| package org.eclipse.swt.svg; | ||
|
|
||
| import org.junit.platform.suite.api.SelectClasses; | ||
| import org.junit.platform.suite.api.Suite; | ||
|
|
||
| @Suite | ||
| @SelectClasses({ // | ||
| JSVGRasterizerTest.class, // | ||
| }) | ||
| public class AllSVGTests { | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You would need something here to wire SWT+SVG bundles usually like
(maybe make it optional if we think SVG rasterizer is really an optional feature)
Then swt.svg would need this in the manifest:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, thank you. That's something that is required now that the SVG part is not a fragment but a separate bundle such that both use independent class loaders.
What I don't fully get is why there is now a cycle detected:
The SWT bundle has an (optional) requirement for the service provider capability that is provided by the SVG bundle and the SVG bundle of course requires the SWT bundle. Since the former is optional, I would have expected that this is okay. Is that against the specification?
When starting a runtime from the IDE, this all works fine. The SWT bundle starts first, as the SVG bundle depends on it. Then the SVG bundle starts and provides the service. Does that only work by accident?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it's the issue that you have reported already. I missed to link that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One can disable optional see https://tycho.eclipseprojects.io/doc/latest/target-platform-configuration/target-platform-configuration-mojo.html#dependency-resolution + https://github.com/eclipse-tycho/tycho/blob/main/target-platform-configuration/src/main/java/org/eclipse/tycho/target/DependencyResolutionConfiguration.java + https://github.com/eclipse-tycho/tycho/blob/main/tycho-api/src/main/java/org/eclipse/tycho/OptionalResolutionAction.java but I'm not 100% sure it is working as this feature is very seldom used
In general the OSGi specification only define runtime behavior never compile / package behavior! Also "optional" has no meaning at compile time because you can't "optionally compile" a class so most often optional is actually mandatory to build the bundle except rare cases (e.g. pur reflection usage or case like this).
Yes runtime is easy, because there is only that all wiring must be fulfilled and there is no need for one thing being defined before the other and one can easily have cycles. But for compilation we need a strict cycle free graph of projects. Technically one would be able to compile such setups as one big project, but this conflicts of course with the maven reactor project concept. Of course it won't be a problem at all if
swt+swt.svgwould be compiled independently ... so it is not always easy.No it does work exactly as one would suppose it to work, we have similar problems with
p2.infand some tricks to avoid it but not have such thing (yet) for the generic requirement/capability model of OSGi, what would generally would be superior to thep2.infapproach becausep2.infonly works at installation time (thats also why I proposed this in the first place instead of rely onp2.infor features).So to summarize: Yes it should work and we have good reasons to make it work and I hope to find some free time to implement something soon (so don't hesitate to ping me if I forget :-)