-
Notifications
You must be signed in to change notification settings - Fork 187
Move SVG rasterizer service to source folder #2484
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
Move SVG rasterizer service to source folder #2484
Conversation
|
This pull request changes some projects for the first time in this development cycle. Warning 🚧 This PR cannot be modified by maintainers because edits are disabled or it is created from an organization repository. To obtain the required changes apply the git patch manually as an additional commit. Git patchFurther information are available in Common Build Issues - Missing version increments. |
30f4e26 to
f5adf00
Compare
HeikoKlare
left a comment
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.
Works as expected. This avoids that for standalone usage of SWT from source code (not packages as JAR) a workaround such as the one proposed in #2228 has to be applied when one wants to use SVGs.
Practically, it allows to run SWT snippets that make use of SVGs (without any modifications as per the referenced comment above) and also to run the SVGRasterizer tests as pure JUnit tests (instead of plug-in tests): Test_org_eclipse_swt_internal_SVGRasterizer.
I also ran a local build to verify that the service declaration is definitely embedded into the final JAR.
f5adf00 to
d14b11e
Compare
Yes this is currently a limitation on how JDT/PDE works when it comes to different resource handling.
Any reason we need an additional source folder instead of just placing it in the existing one? |
I don't think there is any need to do it. It is just to not mess up the actual source code folder with additional resources. Just like done in a default Maven setup with separate "src/java" and "src/resources" source folders. |
With the SVG rasterizer contributed as separate project to SWT, running pure Java Snippets did not work out of the box, when the SVG project was added to the workspace. This commit moves the service provider configuration into a separate source folder so it will be properly initialized in this use case as well.
d14b11e to
ffde040
Compare
| <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-17"/> | ||
| <classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/> | ||
| <classpathentry kind="src" path="src"/> | ||
| <classpathentry kind="src" path="resources"/> |
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.
M2e is using excluding="**" that seem to prevent java files from being processed in such resource only folders.
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.
What's the concrete proposal/question here?
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.
| <classpathentry kind="src" path="resources"/> | |
| <classpathentry kind="src" path="resources" excluding="**"/> |
that should prevent any java files from being placed here so you get a real "resource only" folder.
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.
Okay, I understand the idea, but (1) it's also a bit weird that if you place a Java file into an actual source folder (no matter whether it's called "resources") is not considered and (2) the pattern appears completely incomprehensible to me, so I am not sure if anywill will understand what this is supposed to do when taking a look at it or when dealing with issues potentially arising from it.
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 wanted a "resource folder like maven" and that is how maven does it (or more formally m2e). That it is a "source folder" is an implementation detail of JDT because no one has minded to actually implement it like in maven (where resources can also be filtered for example before copied)
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.
Just tested and adding excluding="**" completely excludes the directory, thus rendering the change completely useless as the service file will not be placed in the bin folder anymore.
With the SVG rasterizer contributed as separate project to SWT, running pure Java Snippets did not work out of the box, when the SVG project was added to the workspace. This commit moves the service provider configuration into a separate source folder so it will be properly initialized in this use case as well.