Bug 522740 - Upgrade Batik version in Orbit to 1.9.1
Summary: Upgrade Batik version in Orbit to 1.9.1
Status: CLOSED FIXED
Alias: None
Product: Orbit
Classification: Tools
Component: bundles (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows NT
: P3 normal with 5 votes (vote)
Target Milestone: 2018-09 M2   Edit
Assignee: Pierre-Charles David CLA
QA Contact:
URL:
Whiteboard: RHT
Keywords:
Depends on:
Blocks: 535449 522742 535133
  Show dependency tree
 
Reported: 2017-09-25 10:22 EDT by Ed Willink CLA
Modified: 2019-05-22 04:11 EDT (History)
13 users (show)

See Also:


Attachments
The effect of the split package on package accessibility rules (71.63 KB, image/png)
2017-11-20 11:34 EST, Pierre-Charles David CLA
no flags Details
Apache FOP Transcoder All-In-One for Batik 1.9.1 (1.65 MB, application/x-java-archive)
2018-01-19 08:50 EST, Pierre-Charles David CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Willink CLA 2017-09-25 10:22:13 EDT
The Oxygen Orbit R20170516192513 provides org.apache.batik.css versions 1.6.0, 1.7.0, 1.8.0; very reasonable.

The latest Photon Orbit S20170912163609 provides org.apache.batik.css versions 1.6.0, 1.7.0, 1.9.0.

What happened to 1.8.0? If there is an incompatibility, why hasn't it been announced on cross-project-dev.
Comment 1 Ed Willink CLA 2017-09-27 09:21:19 EDT
https://dev.eclipse.org/mhonarc/lists/cross-project-issues-dev/msg14848.html identifies that https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-5662 mandates use of batik 1.9.0.

So perhaps the question is why does the latest Photon Orbit offer 1.6.0 or 1.7.0?
Comment 2 Roland Grunberg CLA 2017-09-27 10:50:09 EDT
I can have these removed for Photon M3. 1.6 and 1.7 seemed to have many more consumers in ipzilla than 1.8/1.9 (which were introduced by the same project).

If it weren't for the CVE, maybe it would have made sense to keep them or at least discuss the possibility for projects that still use them, but in this case removal makes sense.
Comment 3 Pierre-Charles David CLA 2017-09-27 11:40:28 EDT
[Reproducing my message from cross-projects]

From my understanding of the situation, this may break GMF Runtime. GMF Runtime currently depends on Batik 1.6, for example org.apache.batik.bridge;bundle-version="[1.6.0,1.7.0)", which itself Require-Bundle: org.apache.batik.css;bundle-version="[1.6.0,1.7.0)". If Batik 1.6 is removed from Photon M3 either GMF Runtime will not build, or (if we continue to build against an older Orbit), will fail to aggregate.

I can try to update GMF Runtime to be compatible with Batik 1.9, but currently not all parts of Batik that GMF Runtime requires are available in v1.9 in Orbit, and I'm not sure I'll have the bandwidth to make all the changes required. 

Looking at the direct requirements from GMF and indirect dependencies between the Batik bundles themselves (assuming those are the same in 1.9, I'm looking at the 1.6 bundles), it seems that we can safely assume that all Batik bundles in Orbit that have Anthony Hunter as contact were added because GMF Runtime needed them. Which gives:
- org.apache.batik.bridge
- org.apache.batik.css
- org.apache.batik.dom.svg
- org.apache.batik.ext.awt
- org.apache.batik.extension
- org.apache.batik.parser
- org.apache.batik.pdf
- org.apache.batik.svggen
- org.apache.batik.swing
- org.apache.batik.transcoder
- org.apache.batik.util
- org.apache.batik.util.gui
- org.apache.batik.xml

From what I see at http://download.eclipse.org/tools/orbit/downloads/drops/S20170912163609/, of those only org.apache.batik.css and org.apache.batik.util are present in version 1.9.0 for now.

Note that the CVE issue was already reported to me in may via https://bugs.eclipse.org/bugs/show_bug.cgi?id=517093, but from my reading of the vulnerability, I don't see how this could affect GMF's usage of Batik (I'm certainly not a security expert though, so maybe I'm missing something).
Comment 4 Ed Willink CLA 2017-09-27 11:50:37 EDT
(In reply to Pierre-Charles David from comment #3)
> From my understanding of the situation, this may break GMF Runtime. GMF
> Runtime currently depends on Batik 1.6, for example

I suspect it already does. When I run my Hudson tests with the latest Orbit, the tests fail to start with no log file anywhere. In the past this has eventually turned out to be bad dependencies that crash Eclipse before a log file is started. Since I have Papyrus (and so GMF) as part of my installation, a mixed Batik 1.9.0/1.6.0 installation might well fail.
Comment 5 Ed Willink CLA 2017-09-27 12:07:51 EDT
In principle, GMF should consume the latest version in Orbit.

But perhaps since the batik dependency is 'recent' it doesn't contribute much and so could be written out.

Or perhaps batik 1.6.0 could be re-distributed as an internal GMF library that is not visible on a classpath. (OCL did this for ASM 3 while ASM 3/ASM 5 compatibility was needed.)
Comment 6 Pierre-Charles David CLA 2017-09-28 03:24:45 EDT
(In reply to Ed Willink from comment #4)
> (In reply to Pierre-Charles David from comment #3)
> > From my understanding of the situation, this may break GMF Runtime. GMF
> > Runtime currently depends on Batik 1.6, for example
> 
> I suspect it already does. When I run my Hudson tests with the latest Orbit,
> the tests fail to start with no log file anywhere. In the past this has
> eventually turned out to be bad dependencies that crash Eclipse before a log
> file is started. Since I have Papyrus (and so GMF) as part of my
> installation, a mixed Batik 1.9.0/1.6.0 installation might well fail.

Speaking of Papyrus, it too has direct dependencies towards Batik 1.6:

./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.clazz/META-INF/MANIFEST.MF: org.apache.batik.dom.svg;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.clazz/META-INF/MANIFEST.MF: org.apache.batik.util;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.clazz/META-INF/MANIFEST.MF: org.apache.batik.svggen;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.clazz/META-INF/MANIFEST.MF: org.apache.batik.dom;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.clazz/META-INF/MANIFEST.MF: org.apache.batik.css;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.common/META-INF/MANIFEST.MF: org.apache.batik.dom.svg;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.common/META-INF/MANIFEST.MF: org.apache.batik.css;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.common/META-INF/MANIFEST.MF: org.apache.batik.util;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.common/META-INF/MANIFEST.MF: org.apache.batik.dom;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/META-INF/MANIFEST.MF: org.apache.batik.dom.svg;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/META-INF/MANIFEST.MF: org.apache.batik.util;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/META-INF/MANIFEST.MF: org.apache.batik.svggen;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/META-INF/MANIFEST.MF: org.apache.batik.dom;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/META-INF/MANIFEST.MF: org.apache.batik.css;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF: org.apache.batik.util;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF: org.apache.batik.svggen;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF: org.apache.batik.dom.svg;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF: org.apache.batik.dom;bundle-version="[1.6.0,1.7.0)",
./org.eclipse.papyrus/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF: org.apache.batik.xml;bundle-version="[1.6.0,1.7.0)",

Sirius too, but those I already knew about.

GMF Tooling seems to have the same, but it will not be part of the SimRel, so it shouldn't be a problem.
Comment 7 Pierre-Charles David CLA 2017-10-03 09:14:19 EDT
I'll start to take the steps to contribute the missing JARs from Batik 1.9 to Orbit, and then update GMF Runtime (and Sirius) to be compatible with them. I have never contributed JARs to Orbit, so I'm not sure I'll be able to finish this by M3. If I'm late, would it be an issue to wait for M4 before removing the previous versions?

Does anybody know if the steps at https://wiki.eclipse.org/Orbit/Adding_Bundles_to_Orbit are up to date regarding the steps involved?
Comment 8 Roland Grunberg CLA 2017-10-03 10:20:12 EDT
Getting the additional 1.9 bundles needed for M4 (and scheduling the 1.6/1.7 removals for then) seems fine to me.

Yes that page is up to date, but can be a bit daunting as it tries to cover many different cases that may arise, as opposed to giving a simple walkthrough of the most common way of contributing. For that, take a look at https://wiki.eclipse.org/Orbit/Adding_Bundles_To_Orbit_In_5_Minutes . I would also look at https://git.eclipse.org/r/90583/ (batik 1.8 contribution) to get an idea of what gets contributed and how it looks.
Comment 9 Pierre-Charles David CLA 2017-10-03 11:25:58 EDT
(In reply to Roland Grunberg from comment #8)
> Getting the additional 1.9 bundles needed for M4 (and scheduling the 1.6/1.7
> removals for then) seems fine to me.
> 
> Yes that page is up to date, but can be a bit daunting as it tries to cover
> many different cases that may arise, as opposed to giving a simple
> walkthrough of the most common way of contributing. For that, take a look at
> https://wiki.eclipse.org/Orbit/Adding_Bundles_To_Orbit_In_5_Minutes . I
> would also look at https://git.eclipse.org/r/90583/ (batik 1.8 contribution)
> to get an idea of what gets contributed and how it looks.

Thanks for the links.
Comment 10 Quentin Le Menez CLA 2017-10-06 03:59:31 EDT
Hi,

Yes indeed Papyrus does also need batik dependencies (a bit less but not by much). I did not look at all the ones listed above but there are the ones I got by querying.
already there:
org.apache.batik.css	(source)	1.9.0	14154	Sopot Cela
org.apache.batik.i18n	(source)	1.9.0	14152	Sopot Cela
org.apache.batik.util	(source)	1.9.0	14153	Sopot Cela

still needed:
org.apache.batik.util;bundle-version="[1.6.0,1.7.0)",
org.apache.batik.svggen;bundle-version="[1.6.0,1.7.0)",
org.apache.batik.dom.svg;bundle-version="[1.6.0,1.7.0)",
org.apache.batik.dom;bundle-version="[1.6.0,1.7.0)",
org.apache.batik.xml;bundle-version="[1.6.0,1.7.0)",
org.apache.batik.css;bundle-version="[1.6.0,1.7.0)"

Thanks for the work Pierre-Charles and let me know if you need any help :)
Comment 11 Bouchet Stéphane CLA 2017-10-26 10:54:58 EDT
Hi,

This is a very old story to get rid of the batik 1.6 off the orbit repo.
Since the release of e4, there has been always at least two version of batik, because of GMF dependencies.

here is a list of the bugs already opens related to this case : 
https://bugs.eclipse.org/bugs/show_bug.cgi?id=189139
https://bugs.eclipse.org/bugs/show_bug.cgi?id=240988
https://bugs.eclipse.org/bugs/show_bug.cgi?id=258349
https://bugs.eclipse.org/bugs/show_bug.cgi?id=291913
https://bugs.eclipse.org/bugs/show_bug.cgi?id=350792
https://bugs.eclipse.org/bugs/show_bug.cgi?id=429872
https://bugs.eclipse.org/bugs/show_bug.cgi?id=504072
https://bugs.eclipse.org/bugs/show_bug.cgi?id=509389
Comment 12 Ed Willink CLA 2017-10-26 10:57:12 EDT
(In reply to Ed Willink from comment #4)
> I suspect it already does. When I run my Hudson tests with the latest Orbit,
> the tests fail to start with no log file anywhere. In the past this has
> eventually turned out to be bad dependencies that crash Eclipse before a log
> file is started. Since I have Papyrus (and so GMF) as part of my
> installation, a mixed Batik 1.9.0/1.6.0 installation might well fail.

This was a red herring. It is more likely that the broken Hudson tests were related to the lack of JUnit5 support for a pre-JUnit 5 Eclipse such as Buckminster 4.4.
Comment 13 Pierre-Charles David CLA 2017-10-26 11:56:42 EDT
Just a quick update: I was not able yet to spend the time I wanted on this. It looks like I'll be too late for Photon M3, sorry. It should still be OK for M4 though.

@Roland, what's the hard deadline for the Orbit build for M4? It looks like for M3 the build was tagged 2 weeks before the release (https://ci.eclipse.org/orbit/job/orbit-recipes/257/), which would give 2017-12-01 for M4. Is that correct?

I actually re-started working on this today. I've got GMF Runtime compiling, with all its tests OK, with Batik 1.9, but that's with embedding the fat JAR directly.

I'm currently trying (locally) to add the ebr recipes (as described in https://wiki.eclipse.org/Orbit/Adding_Bundles_to_Orbit), but it seems at least 3 of the JARs I need for GMF (org.apache.batik.pdf, org.apache.batik.util.gui and org.apache.batik.ext.awt) have no equivalent in Maven central (see https://search.maven.org/#search%7Cga%7C2%7Cg%3A%22org.apache.xmlgraphics%22). I know the documentation mentions I should create the CQs first, but I'm not completely sure yet which JARs I need (dependencies may be different from previous versions).
Comment 14 Roland Grunberg CLA 2017-10-30 10:43:30 EDT
(In reply to Pierre-Charles David from comment #13) 
> @Roland, what's the hard deadline for the Orbit build for M4? It looks like
> for M3 the build was tagged 2 weeks before the release
> (https://ci.eclipse.org/orbit/job/orbit-recipes/257/), which would give
> 2017-12-01 for M4. Is that correct?

We try to release the Orbit milestone build a week before the deadline for +0 projects, so yes, 2017-12-01 would be when we release our M4 contribution.
Comment 15 Pierre-Charles David CLA 2017-11-14 11:26:44 EST
Here is the current status:

I have created 14 CQs for the various JARs in Batik 1.9 which are apparently required. They correspond to all the ones which were present before in Batik 1.6 (and not already added in 1.9), plus 3 new ones (batik-anim-1.9.jar, batik-codec-1.9.jar, batik-gvt-1.9.jar). Those did not exist in 1.6 but appear in the dependencies of the other ones. The CQs are: #14747, #14749, #14750, #14751, #14752, #14753, #14754, #14755, #14756, #14757, #14758, #14759, #14760, #14761. Note that the first 4 may need to be withdrawn, I have created them from the Sirius PMI UI instead of the GMF Runtime one, and they are associated to Sirius...

I'm not even sure yet that this covers everything needed, there are several JARs in the Batik 1.9.1 binary distribution which have no clear equivalent in 1.6 (e.g. serializer.jar).

One part that I know is currently missing is the PDF rendering support. The code actually originates from Apache FOP (based on Batik), although Batik provides a JAR in its distribution. In Batik 1.6, this was pdf-transcoder.jar (see CQ #2141). In Batik 1.9, it's called fop-transcoder-allinone-2.2.jar, but it is *much* larger (1.7MB against 480KB before), so it's not clear to me if it's really the equivalent or if something else is going on. Also, I couldn't find the JAR in Maven Central and have no idea how to contribute it the orbit-recipes.

In parallel I'm starting to create the actual recipes to add the JARs to Orbit. I'll probably push a patch for review today with just the first ones (e.g. xml, ext.awt). If someone can look at them and point to any mistake before I embark in adding all the rest that'd be great.

PS: Maybe I'm doing this wrong, an Batik may not be the simplest example to start with, but the amount of work needed to simply consume a updated version of a well-known library we already have in earlier versions is, quite frankly, a little demoralizing...

PPS: Apparently there was a Batik 1.9.1 released this summer, which "corrects maven metadata only" [1, 2]. While I'am at it, should I contribute directly the 1.9.1 versions? This would probably require upgrading the 3 existing ones (CSS, util, i18n) from 1.9.0 to 1.9.1.

[1] https://mail-archives.apache.org/mod_mbox/xmlgraphics-batik-users/201708.mbox/%3C009501d30ad6%24d76cb0a0%24864611e0%24%40gmail.com%3E
[2] https://svn.apache.org/viewvc?view=revision&revision=1789135
Comment 16 Pierre-Charles David CLA 2017-11-14 11:29:16 EST
(In reply to Pierre-Charles David from comment #15)
> In parallel I'm starting to create the actual recipes to add the JARs to
> Orbit. I'll probably push a patch for review today with just the first ones
> (e.g. xml, ext.awt). If someone can look at them and point to any mistake
> before I embark in adding all the rest that'd be great.

See https://git.eclipse.org/r/#/c/111564/
Comment 17 Ed Willink CLA 2017-11-14 11:32:09 EST
Suggest. Do just 1.9.1.

Observation: while adding 10 batik contributions is a pain and so you naturally do as few as possible, if the total is say 15, you may make someone else's life downstream much easier if you do all 15 at once.
Comment 18 Pierre-Charles David CLA 2017-11-20 11:33:12 EST
New status update.

On the CQ side:
* 18 CQs have been created, which (I hope) cover everything except the FOP/PDF part;
* all of them have been approved by the modeling PMC, and 14 have also been approved by legal (at the time of this writing);
* these 18 include the 1.9.1 versions of the 3 bundles which were already contributed from Batik 1.9.0 (see my previous comment and Ed's answer), so everything is consistent;
* I don't know how to create the "ATOs" corresponding to these. The examples I find in IPZilla are all created on "Product: tools / Component: tools.orbit", but I'm not commiter on Orbit so from the PMI (which I used for the others), I can't seem to create those.

On the orbit-recipes side:
* Roland has reviewed my first attempt (thank!), and it seems I'll need to tweak the osgi.bnd files to declare dependencies.
* I have a more complete patch locally which contributes all of the bundle to orbit-recipes (minus the proper Import-Package). It builds locally, and I'm trying to build and test GMF Runtime on the result.
* Beside the osgi.bnd tweaks, there's one issue I'm stuck with. Batik 1.9 has a split package (org.apache.batik.util). Most of it is found in batik-util.jar (org.apache.batik.util), except *one interface*, named XMLConstants, which is in its own JAR (batik-constants.jar, aka org.apache.batik.constants). Before I noticed this batik-constants.jar, I had GMF Runtime building fine, but crashing with NoClassDefFoundError on this XMLConstants at runtime. Now that I have added the new "constants" bundle, I can't build GMF anymore with the resulting repo: the classes from the main org.apache.batik.util JAR are considered inaccessible: Access restriction: The type 'XMLResourceDescriptor' is not API (restriction on classpath entry '/home/pcdavid/.m2/repository/p2/osgi/bundle/org.apache.batik.util/1.9.1.v20171120-1351/org.apache.batik.util-1.9.1.v20171120-1351.jar'). My interpretation is that OSGi gets the org.apache.batik.util package from the "constants" bundle and only from it, hiding the content of the same from from the main "util" bundle.

I have a few questions to move forward:
1. Is there a recommended way to find package-level dependencies (needed for proper osgi.bnd files)? Is it OK to rely on "jdeps $THE_JAR | grep 'not found'"?
2. How do I create the ATOs?
3. Any idea on how to deal with the split package issue (assuming my analysis is correct)?
4. How do I deal with the binary-only fop-transcoder-allinone-2.2.jar? I can't find it on Maven Central. My understanding from https://wiki.eclipse.org/Orbit/Adding_Bundles_to_Orbit#Library_not_available_on_Maven_Central is that it requires an Orbit commiter to upload the JAR.

See https://docs.google.com/spreadsheets/d/1tYRvaQEu_WR6mVF3w6ZjWh41E-c_TM4U4ySkUWxhenA/edit?usp=sharing for an overview of the current state (which I try to keep up to date).
Comment 19 Pierre-Charles David CLA 2017-11-20 11:34:39 EST
Created attachment 271561 [details]
The effect of the split package on package accessibility rules
Comment 20 Pierre-Charles David CLA 2017-11-21 03:55:23 EST
Regarding the split package issue, I tried:
* adding "Eclipse-ExtensibleAPI: true" on the main org.apache.batik.util side;
* setting "Fragment-Host: org.apache.batik.util" on the org.apache.batik.constants side.

with this, I can build GMF again, but the "java.lang.NoClassDefFoundError: org/apache/batik/util/XMLConstants" are back at runtime...
Comment 21 Eclipse Genie CLA 2017-11-21 04:59:21 EST
New Gerrit change created: https://git.eclipse.org/r/111952
Comment 22 Roland Grunberg CLA 2017-11-21 10:52:09 EST
(In reply to Pierre-Charles David from comment #18)
> 1. Is there a recommended way to find package-level dependencies (needed for
> proper osgi.bnd files)? Is it OK to rely on "jdeps $THE_JAR | grep 'not
> found'"?

Initially the osgi.bnd might contain just *;resolution=optional for Import-Package. When you do a full build of the module, all the required packages will be listed in the final bundle's (jar) Import-Package manifest statement, as optional dependencies. You then just need to figure out which ones are truly optional (eg. available by default in newer version of the jvm), which are simply optional requirements of the bundle (this is more rare), and which are truly mandatory requirements.

jdeps is certainly one way to go, and at times, I've even used some form of : grep "^import" with some post-processing to get an idea. However, you can actually get a good idea of imports from the .bnd file itself. I mainly use this as a sanity check just in case there is something that the automated process may have missed.

Also, when the osgi.bnd file is initially generated, you'll see some pre-defined constants at the very top, of the form : foo-version=${version;===;1.9.1} . This is usually a good indicator that the bundle uses packages from the foo bundle. Of course translating that to the actual Import-Package statement takes a bit more work.

> 2. How do I create the ATOs?

The ATO CQs can only be created by committers to the Orbit project. If no one part of this discussion happens to be an Orbit committer, I can go ahead and file the ATO CQs given that this definitely seems like a wanted change. I would have suggested you be nominated for committer on the project but historically the requirement for Orbit committers has always been to hold some commit privileges on some other project first.

> 3. Any idea on how to deal with the split package issue (assuming my
> analysis is correct)?

Here's my understanding of the issue.

Yes, I've seen the access restriction issue and it's likely due to the split package. I've followed from this guide ( https://eclipsesource.com/blogs/2008/08/22/tip-split-packages-and-visibility/ ) when dealing with split packages.

Basically, you add attributes to the Export-Package statements of the 2 bundles that export the common package so that they don't collide. A bundle that needs to use both those packages will have to Require-Bundle both from my understanding (which is kinda ugly) but I'm not sure if there's another way. I guess if the bundle only needed one part of the split package you could just do an Import-Package on with the attributes.

The other possibility to guarantee compatibility is to create a third bundle that require-bundles both bundles that contain the split, and re-export the entire package namespace, but I'd rather not create the extra bundle.

As an example, let's take java package 'org.apache.lucene.document' from Lucene 6 stack. It is exported by both lucene-core, and lucene-misc. Therefore, it became :

http://git.eclipse.org/c/gerrit/orbit/orbit-recipes.git/tree/apache/lucene/org.apache.lucene.core_6.1.0/osgi.bnd?id=b95ccb19cb21f3547c3a71e95148a1f7c696b2a2
http://git.eclipse.org/c/gerrit/orbit/orbit-recipes.git/tree/apache/lucene/org.apache.lucene.misc_6.1.0/osgi.bnd?id=b95ccb19cb21f3547c3a71e95148a1f7c696b2a2 (ignore line 19 and beyond, that is a special case)

If some package (eg. lucene-analyzers-common) needed the org.apache.lucene.document package from the core part of the split, but not misc, the import became :

http://git.eclipse.org/c/gerrit/orbit/orbit-recipes.git/tree/apache/lucene/org.apache.lucene.analyzers-common_6.1.0/osgi.bnd?id=b95ccb19cb21f3547c3a71e95148a1f7c696b2a2

But what if a package needed both, or as in our case, lucene-misc depended on core's 'org.apache.lucene.document', so we couldn't import it (namespace already existed). We would have to do a Require-Bundle, http://git.eclipse.org/c/gerrit/orbit/orbit-recipes.git/tree/apache/lucene/org.apache.lucene.misc_6.1.0/osgi.bnd?id=b95ccb19cb21f3547c3a71e95148a1f7c696b2a2#n19

> 4. How do I deal with the binary-only fop-transcoder-allinone-2.2.jar? I
> can't find it on Maven Central. My understanding from
> https://wiki.eclipse.org/Orbit/
> Adding_Bundles_to_Orbit#Library_not_available_on_Maven_Central is that it
> requires an Orbit commiter to upload the JAR.

Yes, it'll have to be uploaded to our own maven repository at Eclipse. I've done this in the past for a few bundles : https://repo.eclipse.org/content/repositories/orbit-approved-artifacts/org/eclipse/orbit/ . We have a JIPP instance that automates this process : https://ci.eclipse.org/orbit/job/upload-approved-artifacts/ . As long as the binary/source match what was approved in the CQ, then it's fine to put there.
Comment 23 Pierre-Charles David CLA 2017-11-21 11:25:41 EST
Thanks a lot for all these details! Hopefully this should be enough for me to complete this. Once this is done I'll try to contribute back these to the wiki page for future reference.

(In reply to Roland Grunberg from comment #22)
> > 2. How do I create the ATOs?
[...]
> historically the requirement for Orbit committers has always been to hold
> some commit privileges on some other project first.

I'm project lead for Sirius, EMF Services and GMF Runtime.

> > 4. How do I deal with the binary-only fop-transcoder-allinone-2.2.jar? I
> > can't find it on Maven Central. My understanding from
> > https://wiki.eclipse.org/Orbit/
> > Adding_Bundles_to_Orbit#Library_not_available_on_Maven_Central is that it
> > requires an Orbit commiter to upload the JAR.
> 
> Yes, it'll have to be uploaded to our own maven repository at Eclipse. I've
> done this in the past for a few bundles :
> https://repo.eclipse.org/content/repositories/orbit-approved-artifacts/org/
> eclipse/orbit/ . We have a JIPP instance that automates this process :
> https://ci.eclipse.org/orbit/job/upload-approved-artifacts/ . As long as the
> binary/source match what was approved in the CQ, then it's fine to put there.

I'll prepare the binary bundle once I've got the other parts done, and I may have some more questions for you then. This particular JAR corresponds to a well-contained feature (PDF support for diagrams exports), and if time becomes pressing maybe I'll disable the corresponding code (and remove the dependency) in GMF Runtime for M4, and add it back later on (for M5).
Comment 24 Roland Grunberg CLA 2017-11-21 11:34:41 EST
> (In reply to Roland Grunberg from comment #22)
> > > 2. How do I create the ATOs?
> [...]
> > historically the requirement for Orbit committers has always been to hold
> > some commit privileges on some other project first.
> 
> I'm project lead for Sirius, EMF Services and GMF Runtime.

D'oh! My mistake :) Are those credentials under another account ? For some reason https://accounts.eclipse.org/users/pdavidgp4#tab-projects doesn't show me this.
Comment 25 Pierre-Charles David CLA 2017-11-21 11:47:56 EST
(In reply to Roland Grunberg from comment #24)
> > (In reply to Roland Grunberg from comment #22)
> > > > 2. How do I create the ATOs?
> > [...]
> > > historically the requirement for Orbit committers has always been to hold
> > > some commit privileges on some other project first.
> > 
> > I'm project lead for Sirius, EMF Services and GMF Runtime.
> 
> D'oh! My mistake :) Are those credentials under another account ? For some
> reason https://accounts.eclipse.org/users/pdavidgp4#tab-projects doesn't
> show me this.

Strange that this "pdavidgp4" is associated to me (I can see my avatar on it). My actual profile is https://accounts.eclipse.org/users/pdavid#tab-projects, which should show the proper information.
Comment 26 Pierre-Charles David CLA 2017-11-23 08:35:14 EST
(In reply to Roland Grunberg from comment #14)
> We try to release the Orbit milestone build a week before the deadline for
> +0 projects, so yes, 2017-12-01 would be when we release our M4 contribution.

I'm still on it, but this is more time consuming that I expected (and I just found a 19th JAR that I'll need to add, batik-script), and I can't be full time on this only. I know I've already asked for an extension from the original plan, but how bad would it be if I can't finish this in time for M4?
Comment 27 Pierre-Charles David CLA 2017-11-23 09:15:41 EST
While looking at Batik's dependencies, I noticed that Orbit currently contains Xalan 2.7.1, which apparently is subject to https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-0107. This was fixed in Xalan 2.7.2 released in 2014 (https://xalan.apache.org/xalan-j/readme.html), so maybe we should also upgrade that.
Comment 28 Roland Grunberg CLA 2017-11-23 11:55:39 EST
(In reply to Pierre-Charles David from comment #26)
> (In reply to Roland Grunberg from comment #14)
> > We try to release the Orbit milestone build a week before the deadline for
> > +0 projects, so yes, 2017-12-01 would be when we release our M4 contribution.
> 
> I'm still on it, but this is more time consuming that I expected (and I just
> found a 19th JAR that I'll need to add, batik-script), and I can't be full
> time on this only. I know I've already asked for an extension from the
> original plan, but how bad would it be if I can't finish this in time for M4?

I don't see any issues with this (unless some other project has a strict deadline of not adopting beyond M4). The CQ submission deadline is sometime between M5 and M6, and M6 is the API freeze, but otherwise it should be fine to postpone to M5.
Comment 29 Pierre-Charles David CLA 2017-11-29 03:54:25 EST
It seems I won't be able to work on it this week, sorry. Moving to M5.
Comment 30 Pierre-Charles David CLA 2017-12-21 10:45:05 EST
The good news first: I have a new version of the patch for Orbit [1] with which I can run all the automated tests in GMF Runtime (provided some adaptation [2]) with 0 failures or errors.

The "less good" news:
* I had to add yet more JARs (batik-script-1.9.jar, batik-ext-1.9.jar, xmlgraphics-common-2.2.jar), and create the corresponding CQs, for a grand total of 21 JARs...
* Most of the CQs have been approved but there are 3 left (#15247, #15250, #15253).
* All of these will need ATOs, that I can not create myself.
* There is still the fop-transcoder-allinone-2.2.jar missing, which from what I can tell will require a different procedure. I have not yet looked at how to prepare the JAR for upload (by a commiter). For now the patch on the GMF side [2] removes the support for PDF export (and the corresponding dependency).
* Bug #528924 was reported this week, and at first glance it looks like the solution used in Lucene to deal with split packages (and which is the one I followed) may cause issues. Not sure if the problem is on the packaging or consumer side.

[1] https://git.eclipse.org/r/#/c/111952/
[2] https://git.eclipse.org/r/#/c/114571/
Comment 31 Pierre-Charles David CLA 2018-01-12 07:36:30 EST
(In reply to Pierre-Charles David from comment #30)
> * I had to add yet more JARs (batik-script-1.9.jar, batik-ext-1.9.jar,
> xmlgraphics-common-2.2.jar), and create the corresponding CQs, for a grand
> total of 21 JARs...
> * Most of the CQs have been approved but there are 3 left (#15247, #15250,
> #15253).

CQs #15247 and #15250 are now fully approved. #15253 is being processed, but Sharon mentions "Please proceed with checkin based on Mature Parallel IP.  A Diff Review will be
forthcoming." so I believe we can proceed.

> * All of these will need ATOs, that I can not create myself.

Roland proposed (in a comment on https://git.eclipse.org/r/#/c/111952/) to elect me as Orbit committer to handles these myself, which is fine with me. This would also eliminate the "problem" that my current proposed patch "is over 1000 lines of code and a CQ may be needed" by itself :-)

> * There is still the fop-transcoder-allinone-2.2.jar missing, which from
> what I can tell will require a different procedure. I have not yet looked at
> how to prepare the JAR for upload (by a commiter). For now the patch on the
> GMF side [2] removes the support for PDF export (and the corresponding
> dependency).

My understanding of what I need to do for this one:
* take the original fop-transcoder-allinone-2.2.jar as provided in the Batik 1.9.1 binary distribution;
* rename it into org.apache.batik.pdf_1.9.1.v2018MMDD;
* manually edit the content to add the proper about.html, about_files/*;
* manually edit the MANIFEST.MF and add plugin.properties with the appropriate OSGi meta-data;
* submit a new CQ for a binary-only contribution with the resulting JAR;
* once approved, have an Orbit committer upload it using the upload-approved-artifacts job, or do it myself if I'm committer at this time.

Is that correct?
Comment 32 Roland Grunberg CLA 2018-01-16 11:05:27 EST
(In reply to Pierre-Charles David from comment #31)
> (In reply to Pierre-Charles David from comment #30)
> > * There is still the fop-transcoder-allinone-2.2.jar missing, which from
> > what I can tell will require a different procedure. I have not yet looked at
> > how to prepare the JAR for upload (by a commiter). For now the patch on the
> > GMF side [2] removes the support for PDF export (and the corresponding
> > dependency).
> 
> My understanding of what I need to do for this one:
> * take the original fop-transcoder-allinone-2.2.jar as provided in the Batik
> 1.9.1 binary distribution;
> * rename it into org.apache.batik.pdf_1.9.1.v2018MMDD;
> * manually edit the content to add the proper about.html, about_files/*;
> * manually edit the MANIFEST.MF and add plugin.properties with the
> appropriate OSGi meta-data;
> * submit a new CQ for a binary-only contribution with the resulting JAR;
> * once approved, have an Orbit committer upload it using the
> upload-approved-artifacts job, or do it myself if I'm committer at this time.
> 
> Is that correct?

This sounds correct in terms of provided the bundle, if it's not available directly on maven central. I would double check that fop-transcoder-allinone-2.2.jar provides roughly all the packages/classes there were in 1.6 (allowing some differences for deletions/additions).

Once you're a committer you'll be able to upload the bundle for consumption.
Comment 33 Pierre-Charles David CLA 2018-01-16 11:41:31 EST
(In reply to Roland Grunberg from comment #32)
> (In reply to Pierre-Charles David from comment #31)
> > (In reply to Pierre-Charles David from comment #30)
> > > * There is still the fop-transcoder-allinone-2.2.jar missing, which from
> > > what I can tell will require a different procedure. I have not yet looked at
> > > how to prepare the JAR for upload (by a commiter). For now the patch on the
> > > GMF side [2] removes the support for PDF export (and the corresponding
> > > dependency).
> > 
> > My understanding of what I need to do for this one:
> > * take the original fop-transcoder-allinone-2.2.jar as provided in the Batik
> > 1.9.1 binary distribution;
> > * rename it into org.apache.batik.pdf_1.9.1.v2018MMDD;
> > * manually edit the content to add the proper about.html, about_files/*;
> > * manually edit the MANIFEST.MF and add plugin.properties with the
> > appropriate OSGi meta-data;
> > * submit a new CQ for a binary-only contribution with the resulting JAR;
> > * once approved, have an Orbit committer upload it using the
> > upload-approved-artifacts job, or do it myself if I'm committer at this time.
> > 
> > Is that correct?
> 
> This sounds correct in terms of provided the bundle,

Thanks for the confirmation. I'll try to get at least a first version by the end of the week.

> if it's not available directly on maven central.

I couldn't find it, and it seems to be some manual combination of parts of: avalon-framework-api-4.3.1.jar, commons-io-1.3.1.jar, commons-logging-1.0.4.jar and the main fop.jar from the FOP 2.2 distribution.

> I would double check that
> fop-transcoder-allinone-2.2.jar provides roughly all the packages/classes
> there were in 1.6 (allowing some differences for deletions/additions).

Compared to org.apache.batik.pdf_1.6.0.v201105071520.jar obtained from Orbit, I see two packages (org.apache.fop.image and org.apache.fop.image.analyser) missing from the new version, but also 23 new ones. I've not looked at the class level. Both are a mix of (parts of) Avalon, Commons IO, Commons Logging, and FOP itself.
Comment 34 Pierre-Charles David CLA 2018-01-19 08:50:58 EST
Created attachment 272340 [details]
Apache FOP Transcoder All-In-One for Batik 1.9.1

Here's a first version of a binary JAR derived from the fop-transcoder-allinone-2.2.jar obtained in the Batik 1.9.1 binary distribution.

The modifications I've made:
* Removed the org/apache/commons/logging part, which correspond to Apache Logging 1.0.4. By chance, this exact version is already packaged in Orbit, so I've added the corresponding Import-Package directives instead;
* Added about_files with LICENSE and NOTICE files for the included component, i.e. parts of Avalon Framework, Commons IO (a very old 1.3.1; the oldest we have currently in Orbit is 2.0.1), FOP itself, and some ICC color profiles which are present in the JAR and appear to be under the MIT-like Zlib license.
* Added about.html at the root describing these.
* Added OSGi bundle metadata in the MANIFEST.MF, with:
  * Export-Package for all org.apache.fop.* packages (version 1.9.1 to match the Batik version, even though technically this is FOP 2.2...). The rest (parts of Avalon and Commons IO) are not exported.
  * Import-Package for all the needed org.apache.batik packages (in version 1.9.1):
  * Import-Package for Commons Logging 1.0.4;
  * Import-Package for all the needed org.apache.xmlgraphics packages (in version 2.2). These are not yet in Orbit but will be added as part of my patch for Batik 1.9.1 itself.
  * An optional Import-Package for org.apache.fontbox.cff. According to jdeps, some classes in fop-transcoder-allinone-2.2.jar depend on this, but this is not included in the "all in one" JAR, so for now I'm assuming this is not really needed, at least in the context of Batik PDF rendering. Otherwise that would be yet another CQ for this component (assuming it does not have dependencies...).

I have not yet tested the resulting JAR in a real context, so this may not be the final version.
Comment 35 Pierre-Charles David CLA 2018-02-02 04:53:44 EST
I've pushed a new version of the main patch (https://git.eclipse.org/r/111952) which contains the actual ATO numbers (they have all been filed and approved) and a few changes suggested by Roland.

I've also filed the CQ (#15532) and ATO (#15533) for the PDF Transcoder JAR. It's not approved yet though: "Your IP Request has been automatically scanned; however, the results indicate IP Team involvement is required.  Please stand by for assistance." I don't expect any particular issue, but will probably need to exchange with the IP team.
Comment 36 Roland Grunberg CLA 2018-02-02 10:54:06 EST
Once org.apache.batik.pdf is approved, I can have it uploaded using the approach mentioned at https://wiki.eclipse.org/Orbit/Adding_Bundles_to_Orbit#Library_not_available_on_Maven_Central . From there it can be consumed by the build using the GAV from there.

Committers should also have permissions needed to perform the same step.
Comment 37 Pierre-Charles David CLA 2018-02-14 04:54:08 EST
I really thought the main patch was almost ready for submission, but when trying to integrate all the pieces I discovered a new issue for which I don't have a solution.

Long story short: when trying to integrate Batik 1.9.1 (using a locally built Orbit repo), GMF Runtime (patched to adapt to Batik 1.9.1) and Sirius master (patched too), starting a runtime on a fresh workspace takes more than 2 minutes, mostly spent inside Felix's ResolverImpl trying (and failing) to properly resolve all the dependencies.

The "culprit" seems to be the org.w3c.dom package. Batik depends on classes which are in this namespace, but are not part of the org.w3c.dom.* that are provided by default by the JRE:

    org.w3c.dom.Window
    org.w3c.dom.Location
    org.w3c.dom.ElementTraversal
    org.w3c.dom.events.TextEvent
    org.w3c.dom.events.EventTarget
    org.w3c.dom.events.EventListener
    org.w3c.dom.events.MutationNameEvent
    org.w3c.dom.events.KeyboardEvent
    org.w3c.dom.events.Event
    org.w3c.dom.events.MutationEvent
    org.w3c.dom.events.DocumentEvent
    org.w3c.dom.events.EventException
    org.w3c.dom.events.CustomEvent
    org.w3c.dom.events.MouseEvent
    org.w3c.dom.events.UIEvent
    
In Batik itself these are provided by a separate JAR, batik-ext.jar, and outside of an OSGi context I guess this works.

I've packaged it as org.apache.batik.ext, using the technique suggested by Roland to handle split packages. I don't know if it's because the package is a "system package" provided by the JRE, but this does not work (unless I've made a mistake). Strangely, all the GMF Runtime automated tests pass even with this problem, so I did only noticed it when integrating with Sirius, which has enough "Unresolvable" plug-ins that the startup delay becomes noticeable.

Here's a sample of the error messages that fill the runtim's error log:

  Bundle was not resolved because of a uses contraint violation.
  org.osgi.service.resolver.ResolutionException: Uses constraint violation. Unable to resolve resource org.eclipse.gmf.runtime.diagram.ui.printing.render [osgi.identity; osgi.identity="org.eclipse.gmf.runtime.diagram.ui.printing.render"; type="osgi.bundle"; version:Version="1.8.0.qualifier"] because it is exposed to package 'org.w3c.dom' from resources org.eclipse.osgi [osgi.identity; osgi.identity="org.eclipse.osgi"; type="osgi.bundle"; version:Version="3.13.0.v20180110-2102"; singleton:="true"] and org.apache.batik.ext [osgi.identity; osgi.identity="org.apache.batik.ext"; type="osgi.bundle"; version:Version="1.9.1.v20180206-0933"] via two dependency chains.

See https://gist.github.com/pcdavid/8e0a4b0ffa52211ab25804f4f1240563 for the complete message (I can provide the complete error logs if needed). Basically org.w3c.dom is exposed both via org.eclipse.core.runtime (which re-exports org.eclipse.osgi) and org.apache.batik.ext.

The message seems to indicate that the problem is related to the "uses" constraints declared in the MANIFEST.MF (which are generated by EBR/BND). I don't know if it's possible to disable the generation of these (or if it would help).

Roland suggested to replace all the "Require-Bundle: org.eclipse.runtime" in GMF by just the "Import-Package" needed (and thus *not* get org.w3c.dom from that path). I tried, but it's not practical given the dependency chains between all the GMF bundles which require org.eclipse.runtime and reexport it at multiple levels...

I've looked at how the problem was handled in previous versions of Batik packaged in Orbit. It seems some (e.g. [1]), include private (non-exported) copies of the required org.w3c.dom classes directly in the Batik JARs. Others use plain "Import-Package" which do not seem to cause problems. Maybe because they do not declare "uses" constraints?

I've already consumed much more time on this general issue that what I expected when I volunteered, and honestly at this point I'm mostly stuck. There are two approaches that *may* be worth trying, but I'm really not sure if they're worth:
* avoid generating the "uses" constraints (if that's possible) and see if it helps;
* include private copies of the required classes in every Batik JAR that needs them (crossing fingers the JARs will never pass around instances of these incompatible versions).

If anybody has an idea of how to proceed, any help would be appreciated.

[1] http://download.eclipse.org/tools/orbit/downloads/drops/R20170816192254/repository/plugins/org.apache.batik.dom_1.7.1.v201505191845.jar
Comment 38 Ed Willink CLA 2018-02-14 05:29:04 EST
(In reply to Pierre-Charles David from comment #37)
> If anybody has an idea of how to proceed, any help would be appreciated.

I see Dani on the users list but no OSGI guys.

Nightmare. It may be time for an appeal on cross-project-dev.

Naive instinctive suggestion. If the problem is that two export paths conflict, is it possible to export from a (new) ancestor so that the conflicting exporters import instead?

Observation. Uses constraints have been suggested as the solution to the non-singleton Guava version problem, but IMHO their utility has never been demonstrated in earnest. Perhaps you have hit an Equinox bug.
Comment 39 Roland Grunberg CLA 2018-02-14 08:56:06 EST
With regard to the replacing all GMF bundles' usage of o.e.core.runtime with just the necessary Import-Package, that was a small typo in my request.

I believe only org.eclipse.gmf.runtime.diagram.ui.printing.render needs the core.runtime require-bundle replaced by just the Import-Package statements needed directly from that bundle.
Comment 40 Pierre-Charles David CLA 2018-02-14 09:00:45 EST
(In reply to Pierre-Charles David from comment #37)
> There are two approaches that *may* be worth trying, but I'm really not sure
> if they're worth:
> * avoid generating the "uses" constraints (if that's possible) and see if it
> helps;

Actually, it this approach is possible using the "-nouses : true" BND instruction [1]. I'll see if it does what I think it does, and if that helps on the GMF side and report here soon.

[1] http://bnd.bndtools.org/chapters/825-instructions-ref.html
Comment 41 Eclipse Genie CLA 2018-02-14 09:37:49 EST
New Gerrit change created: https://git.eclipse.org/r/117359
Comment 42 Pierre-Charles David CLA 2018-02-14 10:03:30 EST
(In reply to Eclipse Genie from comment #41)
> New Gerrit change created: https://git.eclipse.org/r/117359

It does indeed produce bundles with no "uses" directives. However now my GMF Runtime tests fail with:

testCopyToImageOffscreenUtilTest_JPEG(org.eclipse.gmf.tests.runtime.diagram.ui.render.util.CopyToImageUtilTests)  Time elapsed: 0.985 sec  <<< ERROR!
java.lang.NoClassDefFoundError: org/w3c/dom/Document
	at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:433)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:395)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:387)
	at org.eclipse.osgi.internal.loader.ModuleClassLoader.loadClass(ModuleClassLoader.java:150)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at org.eclipse.gmf.runtime.draw2d.ui.render.awt.internal.svg.SVGImageType.isSVG(SVGImageType.java:62)
	at org.eclipse.gmf.runtime.draw2d.ui.render.awt.internal.svg.SVGImageType.autoDetect(SVGImageType.java:32)
	at org.eclipse.gmf.runtime.draw2d.ui.render.factory.RenderedImageFactory.autodetectImage(RenderedImageFactory.java:359)
	at org.eclipse.gmf.runtime.draw2d.ui.render.factory.RenderedImageFactory.getInstance(RenderedImageFactory.java:163)
	at org.eclipse.gmf.runtime.draw2d.ui.render.factory.RenderedImageFactory.getInstance(RenderedImageFactory.java:215)
	at org.eclipse.gmf.runtime.draw2d.ui.render.factory.RenderedImageFactory.getInstance(RenderedImageFactory.java:187)
	at org.eclipse.gmf.tests.runtime.diagram.ui.render.util.CopyToImageUtilTests.copyToImageOffscreenTestForFormat(CopyToImageUtilTests.java:197)
	at org.eclipse.gmf.tests.runtime.diagram.ui.render.util.CopyToImageUtilTests.testCopyToImageOffscreenUtilTest_JPEG(CopyToImageUtilTests.java:139)

Debugging, we hit this part of org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(String name, boolean checkParent), with name = "org.w3c.dom.Document"

		// 3) search the imported packages
		PackageSource source = findImportedSource(pkgName, null);
		if (source != null) {
			if (debug.DEBUG_LOADER) {
				Debug.println("BundleLoader[" + this + "] loading from import package: " + source); //$NON-NLS-1$ //$NON-NLS-2$
			}
			// 3) found import source terminate search at the source
			result = source.loadClass(name);
			if (result != null)
				return result;
			throw new ClassNotFoundException(name + " cannot be found by " + this); //$NON-NLS-1$
		}

It resolves "source" to "org.w3c.dom -> [org.apache.batik.ext_1.9.1.v20180214-1354]", which of course does not contain org.w3c.dom.Document.

I can't investigate further right now, but will try to get back to it later.
Comment 43 Roland Grunberg CLA 2018-02-14 11:24:37 EST
(In reply to Pierre-Charles David from comment #42)
> (In reply to Eclipse Genie from comment #41)
> > New Gerrit change created: https://git.eclipse.org/r/117359
> 
> It does indeed produce bundles with no "uses" directives. However now my GMF

Disabling uses constraints is probably ok just to allow everything to load and carry forward with possible issues (as we're doing now) but it should really be a last resort and well documented.

uses-constraints, from my understanding, are basically pre-runtime checks to ensure we don't fail at runtime with cryptic errors like :

ClassCastException : Cannot cast org.foo.Bar to org.foo.Bar.

Chasing down such issues can often be a giant pain. From when I last looked at the code it seemed like the bundle in question (org.eclipse.gmf.runtime.diagram.ui.printing.render) doesn't actually use batik or org.w3c*, so even though it requires core.runtime, and another bundle that, down the line happens to require batik, the issue can't happen. This is why I suggested changing the core.runtime requirement, in the hopes that this would make it more clear to the uses-constraint checker that there is no double exposure.


> Runtime tests fail with:
> 
> testCopyToImageOffscreenUtilTest_JPEG(org.eclipse.gmf.tests.runtime.diagram.
> ui.render.util.CopyToImageUtilTests)  Time elapsed: 0.985 sec  <<< ERROR!
> java.lang.NoClassDefFoundError: org/w3c/dom/Document
> 	at
> org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.
> java:433)
> 	at
> org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:
> 395)
> 	at
> org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:
> 387)
> 	at
> org.eclipse.osgi.internal.loader.ModuleClassLoader.
> loadClass(ModuleClassLoader.java:150)
> 	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
> 	at
> org.eclipse.gmf.runtime.draw2d.ui.render.awt.internal.svg.SVGImageType.
> isSVG(SVGImageType.java:62)
> 	at
> org.eclipse.gmf.runtime.draw2d.ui.render.awt.internal.svg.SVGImageType.
> autoDetect(SVGImageType.java:32)
> 	at
> org.eclipse.gmf.runtime.draw2d.ui.render.factory.RenderedImageFactory.
> autodetectImage(RenderedImageFactory.java:359)
> 	at
> org.eclipse.gmf.runtime.draw2d.ui.render.factory.RenderedImageFactory.
> getInstance(RenderedImageFactory.java:163)
> 	at
> org.eclipse.gmf.runtime.draw2d.ui.render.factory.RenderedImageFactory.
> getInstance(RenderedImageFactory.java:215)
> 	at
> org.eclipse.gmf.runtime.draw2d.ui.render.factory.RenderedImageFactory.
> getInstance(RenderedImageFactory.java:187)
> 	at
> org.eclipse.gmf.tests.runtime.diagram.ui.render.util.CopyToImageUtilTests.
> copyToImageOffscreenTestForFormat(CopyToImageUtilTests.java:197)
> 	at
> org.eclipse.gmf.tests.runtime.diagram.ui.render.util.CopyToImageUtilTests.
> testCopyToImageOffscreenUtilTest_JPEG(CopyToImageUtilTests.java:139)
> 
> Debugging, we hit this part of
> org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(String name,
> boolean checkParent), with name = "org.w3c.dom.Document"
> 
> 		// 3) search the imported packages
> 		PackageSource source = findImportedSource(pkgName, null);
> 		if (source != null) {
> 			if (debug.DEBUG_LOADER) {
> 				Debug.println("BundleLoader[" + this + "] loading from import package: "
> + source); //$NON-NLS-1$ //$NON-NLS-2$
> 			}
> 			// 3) found import source terminate search at the source
> 			result = source.loadClass(name);
> 			if (result != null)
> 				return result;
> 			throw new ClassNotFoundException(name + " cannot be found by " + this);
> //$NON-NLS-1$
> 		}
> 
> It resolves "source" to "org.w3c.dom ->
> [org.apache.batik.ext_1.9.1.v20180214-1354]", which of course does not
> contain org.w3c.dom.Document.

I don't have the source changes you made, so I'm working off of a 1.9.0 GMF SDK, but it looks like :

In SVGImageType:62 , there is an attempted instantiation of org.apache.batik.dom.svg.SAXSVGDocumentFactory . In batik 1.9.1, this class is in org.apache.batik.anim, as org.apache.batik.anim.dom.SAXSVGDocumentFactory. I'm assuming you made this change to the sources.

This class definitely uses org.w3c.dom.Document which is not in batik-ext. However in the batik.anim.dom osgi.bnd file, there is :

Import-Package :
...
org.w3c.dom;ext=split;mandatory:=ext;version="${batik-ext-version}", \
...

So batik-ext gets resolved, and it's unable to satisfy org.w3.dom.Document.

In fact, looking more closely at org.apache.batik.anim, it only imports the following org.w3c.dom packages :

org.w3c.dom.Attr
org.w3c.dom.CDATASection
org.w3c.dom.Comment
org.w3c.dom.Document
org.w3c.dom.DocumentFragment
org.w3c.dom.DocumentType
org.w3c.dom.DOMException
org.w3c.dom.DOMImplementation
org.w3c.dom.Element
org.w3c.dom.EntityReference
org.w3c.dom.NamedNodeMap
org.w3c.dom.Node
org.w3c.dom.NodeList
org.w3c.dom.ProcessingInstruction
org.w3c.dom.Text

None of these could be satisfied by batik-ext so it seems to me like the Import-Package should actually be satisfied by the JVM, or xml-apis, as defined at the top of the osgi.bnd.
Comment 44 Pierre-Charles David CLA 2018-02-19 11:53:08 EST
So, trying one last time to get a complete grasp of the situation. Maybe the effort of writing this will reveal something I missed. Otherwise I think I'll take Ed's advice and ask for help on cross-project.

The batik-ext JAR from Batik, which is bundled as org.apache.batik.ext in my patch, provides the following types:
- org.w3c.dom.Window
- org.w3c.dom.Location
- org.w3c.dom.ElementTraversal
- org.w3c.dom.events.TextEvent
- org.w3c.dom.events.EventTarget
- org.w3c.dom.events.EventListener
- org.w3c.dom.events.MutationNameEvent
- org.w3c.dom.events.KeyboardEvent
- org.w3c.dom.events.Event
- org.w3c.dom.events.MutationEvent
- org.w3c.dom.events.DocumentEvent
- org.w3c.dom.events.EventException
- org.w3c.dom.events.CustomEvent
- org.w3c.dom.events.MouseEvent
- org.w3c.dom.events.UIEvent

Some of these are already present in the JRE. The ones which are specific to batik-ext are:
- org.w3c.dom.ElementTraversal (batik-dom)
- org.w3c.dom.Location (batik-bridge)
- org.w3c.dom.Window (batik-bridge)
- org.w3c.dom.events.CustomEvent (batik-dom)
- org.w3c.dom.events.KeyboardEvent (batik-anim, batik-dom)
- org.w3c.dom.events.MutationNameEvent (batik-dom)
- org.w3c.dom.events.TextEvent (batik-dom)

In parentheses I put the Batik components which actually make use of these, based on a simple grep in the Batik 1.9 source code [1]. The problematic bundles are thus: batik-anim, batik-bridge and batik-dom.

batik-anim uses:
- types from org.w3c.dom, but only the ones that are available from the JRE.
- types from org.w3c.dom.events, both from the JDK and ones that only exists in batik-ext.

Here we'd want to obtain org.w3c.dom from the JRE, and org.w3c.dom.events from batik-ext (it also includes the org.w3c.dom.events.* provided by the JRE). It *seems* this should be possible by importing the tagged/annotated package and disabling the generation of uses directives (which would re-created an implicit and un-tagged dependency towards the package).

batik-dom uses:
- types from org.w3c.dom which come from both the JRE (e.g. Document, Element), *and* ElementTraversal which is specific to batik-ext.
- types from org.w3c.dom.events, but only ones which are available from the JRE.

batik-bridge uses:
- types from org.w3c.dom which come from both the JRE, *and* ones which are specific to batik-ext (Window and Location).
- types from org.w3c.dom.events, but only ones which are available from the JRE.

For the last two we'd want to merge org.w3c.dom as provided by the JRE *and* the two types which only exist in batik-ext. From what I read at http://web.ist.utl.pt/ist162500/?p=65#Requiring_Bundles, it seems that for the split package approach ("ext=split;mandatory:=ext") to work all the sources of the split package must be annotated alike, which seems incompatible with the fact that org.w3c.dom is a "system package".

Note that org.w3c.dom.ElementTraversal first appeared in Batik 1.7 [2], so the dependency already exists in the Orbit version we have [3], except that it was apparently manually added to the JAR to work around the issue.

For the Window and Location types which cause problems in batik-bridge, they were apparently only introduced later in 2008 [4], and only appear in Batik 1.8 and later, which explains why this particular issue had not occured before.

I guess we could try to post-process the JARs built by ebr to embed the problematic types directly in the bundles, following the approach used for ElementTraversal, but my Maven-fu is not strong enough to try that. What to you think?

[1] https://github.com/apache/batik/tree/batik-1_9
[2] http://batik.2283329.n4.nabble.com/ANN-Batik-1-7-released-td2976677.html
[3] http://download.eclipse.org/tools/orbit/downloads/drops/R20170816192254/repository/plugins/org.apache.batik.dom_1.7.1.v201505191845.jar
[4] https://svn.apache.org/viewvc?view=revision&revision=712954
Comment 45 Roland Grunberg CLA 2018-02-20 12:29:57 EST
(In reply to Pierre-Charles David from comment #44)
> ...
> batik-anim uses:
> - types from org.w3c.dom, but only the ones that are available from the JRE.
> - types from org.w3c.dom.events, both from the JDK and ones that only exists
> in batik-ext.
> 
> Here we'd want to obtain org.w3c.dom from the JRE, and org.w3c.dom.events
> from batik-ext (it also includes the org.w3c.dom.events.* provided by the
> JRE). It *seems* this should be possible by importing the tagged/annotated
> package and disabling the generation of uses directives (which would
> re-created an implicit and un-tagged dependency towards the package).

Thanks for looking into this further. My initial analysis had some gaps but I mainly just wanted to point out that there's still packaging issues to sort out.

Yes this sounds fine.

> batik-dom uses:
> - types from org.w3c.dom which come from both the JRE (e.g. Document,
> Element), *and* ElementTraversal which is specific to batik-ext.
> - types from org.w3c.dom.events, but only ones which are available from the
> JRE.
> 
> batik-bridge uses:
> - types from org.w3c.dom which come from both the JRE, *and* ones which are
> specific to batik-ext (Window and Location).
> - types from org.w3c.dom.events, but only ones which are available from the
> JRE.
> 
> For the last two we'd want to merge org.w3c.dom as provided by the JRE *and*
> the two types which only exist in batik-ext. From what I read at
> http://web.ist.utl.pt/ist162500/?p=65#Requiring_Bundles, it seems that for
> the split package approach ("ext=split;mandatory:=ext") to work all the
> sources of the split package must be annotated alike, which seems
> incompatible with the fact that org.w3c.dom is a "system package".
> 
> Note that org.w3c.dom.ElementTraversal first appeared in Batik 1.7 [2], so
> the dependency already exists in the Orbit version we have [3], except that
> it was apparently manually added to the JAR to work around the issue.
> 
> For the Window and Location types which cause problems in batik-bridge, they
> were apparently only introduced later in 2008 [4], and only appear in Batik
> 1.8 and later, which explains why this particular issue had not occured
> before.
> 
> I guess we could try to post-process the JARs built by ebr to embed the
> problematic types directly in the bundles, following the approach used for
> ElementTraversal, but my Maven-fu is not strong enough to try that. What to
> you think?

One approach to this is just by copying the necessary source files directly onto the src/main/java/ path in the bundle recipe. Have a look at the path :

./github/mongodb/org.eclipse.orbit.mongodb_3.2.2/src/main/java/com/mongodb/connection/GSSAPIAuthenticator.java .

The build process will compile such files and ensure they end up in the final jars (binary and source).

This is the same approach used when needing to modify the sources slightly due to patches. It would require us to request the 'modified' flag on the corresponding CQs, but given that this is what previous bundles did, it seems to be the way forward.
Comment 46 Pierre-Charles David CLA 2018-02-22 11:08:43 EST
So the good news is: using the approaches mentioned in the two previous comments, I've got a version with which all the GMF Runtime automated tests pass *and* with no OSGi resolution errors in the logs.

Just for reference (because it took me a while to find out): when adding the sources of the problematic types (Window, Location and ElementTraversal) as hinted by Roland, one has also to adjust the default BND rules of the affected bundles, otherwise the "catch-all" rule for Export-Package

 *;version="${package-version}"

exports these new types which re-introduces unwanted dependencies down the line. The solution was to change the default rule to:

 org.apache.*;version="${package-version}"

I'm now working on the integration with Sirius to further validate the packaging, and hitting new issues: that's the bad news.

The first issue is that in 2015, Batik added a "Services" mechanism to discover/register implementations of some of its interfaces [1], [2]. This does not work in an OSGi context, at least not out of the box. For now I've hard-coded the initialization of the registries I needed in the activator of the Sirius plug-in's that uses Batik:

  ImageWriterRegistry.getInstance().register(new org.apache.batik.ext.awt.image.codec.imageio.ImageIOPNGImageWriter());
  ImageWriterRegistry.getInstance().register(new org.apache.batik.ext.awt.image.codec.imageio.ImageIOTIFFImageWriter());
  ImageWriterRegistry.getInstance().register(new org.apache.batik.ext.awt.image.codec.imageio.ImageIOJPEGImageWriter());
            
  ImageTagRegistry.getRegistry().register(new org.apache.batik.ext.awt.image.codec.imageio.ImageIOJPEGRegistryEntry());
  ImageTagRegistry.getRegistry().register(new org.apache.batik.ext.awt.image.codec.imageio.ImageIOPNGRegistryEntry());
  ImageTagRegistry.getRegistry().register(new org.apache.batik.ext.awt.image.codec.imageio.ImageIOTIFFRegistryEntry());

Ugly, but gets the job done for now to continue integration testing. I'm now hit by various ClassLoader issues (e.g. when Batik tries to instanciate a SAX parser), but it's not clear yet if the problems are on the Sirius side or if they'll require some adaptation in the packaging.

I'll probably post a new version of the patch tomorrow, but depending on what I find during integration testing it may not be its final form.

[1] https://github.com/apache/batik/blob/batik-1_9/batik-util/src/main/java/org/apache/batik/util/Service.java
[2] https://svn.apache.org/viewvc?view=revision&revision=1721858
Comment 47 Pierre-Charles David CLA 2018-02-27 09:35:00 EST
(In reply to Roland Grunberg from comment #45)
> It would require us to request the 'modified' flag on the
> corresponding CQs, but given that this is what previous bundles did, it
> seems to be the way forward.

Done: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=14747#c5 and https://dev.eclipse.org/ipzilla/show_bug.cgi?id=14749#c4.
Comment 49 Roland Grunberg CLA 2018-03-05 14:45:12 EST
It looks like they're attempting to track down the source code in https://dev.eclipse.org/ipzilla/show_bug.cgi?id=15532.
Comment 50 Eclipse Genie CLA 2018-03-13 12:01:53 EDT
New Gerrit change created: https://git.eclipse.org/r/119346
Comment 52 Roland Grunberg CLA 2018-03-16 10:44:08 EDT
Looks like CQ 15532 is approved. @Pierre-Charles, can you confirm that the binary jar attached to this bug (same one attached to the CQ) represents the content needed ? If so, I can go ahead and upload it to our maven repository for approved artifacts, once the ATO CQ is approved.
Comment 53 Pierre-Charles David CLA 2018-03-16 10:48:38 EDT
(In reply to Roland Grunberg from comment #52)
> Looks like CQ 15532 is approved. @Pierre-Charles, can you confirm that the
> binary jar attached to this bug (same one attached to the CQ) represents the
> content needed ? If so, I can go ahead and upload it to our maven repository
> for approved artifacts, once the ATO CQ is approved.

Noted. Thanks for taking this one, I don't have much time to work on this at the moment. I'll have a look at this next week (perhaps monday, but most probably on tuesday).
Comment 54 Pierre-Charles David CLA 2018-03-20 05:04:35 EDT
(In reply to Roland Grunberg from comment #52)
> Looks like CQ 15532 is approved. @Pierre-Charles, can you confirm that the
> binary jar attached to this bug (same one attached to the CQ) represents the
> content needed ? If so, I can go ahead and upload it to our maven repository
> for approved artifacts, once the ATO CQ is approved.

Yes, I confirm that (to the best of my knowledge), the JAR is correct. I matches exactly the original fop-transcoder-allinone-2.2.jar with only the packaging changes mentioned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=522740#c34.
Comment 55 Roland Grunberg CLA 2018-03-23 14:44:29 EDT
The attached jar, " Apache FOP Transcoder All-In-One for Batik 1.9.1 ", should now be available as :

<dependency>
  <groupId>org.eclipse.orbit.batik</groupId>
  <artifactId>batik-pdf</artifactId>
  <version>1.9.1</version>
</dependency>

You would just need to give those coordinates when creating the recipe with 'mvn ebr:create-recipe ...' or by copying an existing one and changing the dependency listed in the pom.
Comment 56 Eclipse Genie CLA 2018-04-13 04:26:19 EDT
New Gerrit change created: https://git.eclipse.org/r/121123
Comment 57 Eclipse Genie CLA 2018-04-17 10:08:20 EDT
New Gerrit change created: https://git.eclipse.org/r/121261
Comment 60 Roland Grunberg CLA 2018-04-17 14:59:07 EDT
With the Batik stack in, I'd just like to confirm one more matter.

The following projects in the Photon aggregation repo seem to currently depend on older versions of Batik :

BIRT, GMF, Graphiti, Papyrus, Sirius, and TM4E.

GMF, and Sirius will be taken care of as Pierre-Charles can react to necessary changes there.

What about BIRT, Graphiti, Papyrus, Sirius and TM4E ? I can post to cross-project and see how likely the adoption of 1.9.1 will be to figure out if we can remove batik 1.6 & 1.7 from active builds.
Comment 61 Roland Grunberg CLA 2018-04-17 15:01:36 EDT
(In reply to Roland Grunberg from comment #60)
> What about BIRT, Graphiti, Papyrus, Sirius and TM4E ?

BIRT, Graphiti, Papyrus, and TM4E.
Comment 62 Pierre-Charles David CLA 2018-04-18 02:34:20 EDT
(In reply to Roland Grunberg from comment #60)
> With the Batik stack in, I'd just like to confirm one more matter.
> 
> The following projects in the Photon aggregation repo seem to currently
> depend on older versions of Batik :
> 
> BIRT, GMF, Graphiti, Papyrus, Sirius, and TM4E.
> 
> GMF, and Sirius will be taken care of as Pierre-Charles can react to
> necessary changes there.

Yes. It's mostly done for GMF, with one patch not merged yet to re-enable PDF support now that the last JAR is in. The patch for Sirius should also be merged soon.

> What about BIRT, Graphiti, Papyrus, and TM4E ?

For Papyrus, Quentin Lemenez from CEA is in CC of this ticket and had looked at the actual dependencies (see comment 10), so I had assumed he would take care of that.

For the others I don't know.

The initial plan was to remove older versions, but that was when we assumed it could be done by M3... I'm sorry it took so long, but given that Batik 1.9 will only become generally available in M7, it seems risky to remove the older versions, with very little time to react if anything goes wrong. Roland, what's you current plan concerning the removal of older versions?
Comment 63 Roland Grunberg CLA 2018-04-18 12:55:37 EDT
(In reply to Pierre-Charles David from comment #62)
> The initial plan was to remove older versions, but that was when we assumed
> it could be done by M3... I'm sorry it took so long, but given that Batik
> 1.9 will only become generally available in M7, it seems risky to remove the
> older versions, with very little time to react if anything goes wrong.
> Roland, what's you current plan concerning the removal of older versions?

I'll post to cross-project-issues and see if BIRT, Graphiti, and TM4E have any plans to adopt 1.9.1 given it'll be available (and the vulnerability of pre-1.9 versions)
Comment 64 Michael Wenz CLA 2018-04-19 04:30:11 EDT
For Graphiti I can confirm that I we switch to Batik 1.9.1 for Photon M7. I had that already on my plan, but could so far not change the build because some pieces were missing. With the new Batik version (I20180417184143) and platform version (4.8-I-builds/I20180418-2000) the build works now.

Current master branch of Graphiti now uses Batik 1.9.1 and this will go into M7.

Michael
Comment 65 Roland Grunberg CLA 2018-04-19 10:53:29 EDT
(In reply to Michael Wenz from comment #64)
> For Graphiti I can confirm that I we switch to Batik 1.9.1 for Photon M7. I
> had that already on my plan, but could so far not change the build because
> some pieces were missing. With the new Batik version (I20180417184143) and
> platform version (4.8-I-builds/I20180418-2000) the build works now.
> 
> Current master branch of Graphiti now uses Batik 1.9.1 and this will go into
> M7.

Thanks for doing this so quickly. Yes, batik.pdf was the final piece needed. I've also taken a quick look at TM4E and the project mainly seems to use batik as an optional dependency, and only 'org.apache.batik.css.parser.Parser'. This class hasn't had any API change between 1.7 and 1.9.1 so it really should be just a simple releng bump.

BIRT uses a few more classes, so that may be trickier but I'll wait to hear back from the project with their intent.
Comment 66 Quentin Le Menez CLA 2018-04-22 11:49:20 EDT
Hi, Papyrus should now run with the latest 1.9.1 on its master branch and will contribute it to M7.

Cheers,
Quentin
Comment 67 Roland Grunberg CLA 2018-04-30 11:48:41 EDT
Just as an update, after looking into TM4E, I see that they don't actually use Batik 1.7 from Orbit in their build process, but they simply have an optional requirement to 1.7.0 or higher.  Given that there's no API breaking changes for org.apache.batik.css.Parser between 1.7 and 1.9.1 (and the main change is closing a stream in a finally clause iirc), things should work the same.

Also, given that BIRT will likely continue using the Orbit R20170307180635 (Neon.3) build, it should be safe to remove Batik 1.6 & 1.7 from active builds.
Comment 68 Roland Grunberg CLA 2018-05-07 13:09:13 EDT
Marking as CLOSED (FIXED), as this is now in our Photon M7 contribution. If there's any issues with the metadata, they should be opened up as separate bugs. Thank-you Pierre-Charles for all the work you've done on this.
Comment 69 Pierre-Charles David CLA 2018-05-08 04:28:41 EDT
(In reply to Roland Grunberg from comment #68)
> Marking as CLOSED (FIXED), as this is now in our Photon M7 contribution. If
> there's any issues with the metadata, they should be opened up as separate
> bugs. Thank-you Pierre-Charles for all the work you've done on this.

Great news! Thank you for your help and guidance.
Comment 70 Eclipse Genie CLA 2018-05-23 11:05:55 EDT
New Gerrit change created: https://git.eclipse.org/r/123182
Comment 71 Yulin Wang CLA 2018-05-23 21:30:00 EDT
BIRT just switched to batik 1.9.1. But it looks like it cannot work correctly due to following error:
                Unresolved requirement: Require-Bundle: org.apache.batik.transcoder; bundle-version="[1.7.0,2.0.0)"
                  -> Bundle-SymbolicName: org.apache.batik.transcoder; bundle-version="1.9.1.v20180227-1645"
                     org.apache.batik.transcoder [20]
                       Unresolved requirement: Import-Package: org.apache.batik.anim.dom; version="1.9.1"
                         -> Export-Package: org.apache.batik.anim.dom; bundle-symbolic-name="org.apache.batik.anim"; bundle-version="1.9.1.v20180227-1645"; version="1.9.1"
                            org.apache.batik.anim [8]
                              Unresolved requirement: Import-Package: org.w3c.dom.events; ext="split"; version="1.9.1"; mandatory:="ext"

It looks like it fails to find package org.w3c.dom.events v1.9.1. However this package exists in bundle org.w3c.dom.events v3.0.0. The feature and product file also include this bundle.
Do you guys know why?

If this issue cannot be resolved, BIRT still has to use batik 1.7.1.
Comment 72 Pierre-Charles David CLA 2018-05-24 03:54:02 EDT
(In reply to Yulin Wang from comment #71)
> BIRT just switched to batik 1.9.1. But it looks like it cannot work
> correctly due to following error:
>                 Unresolved requirement: Require-Bundle:
> org.apache.batik.transcoder; bundle-version="[1.7.0,2.0.0)"
>                   -> Bundle-SymbolicName: org.apache.batik.transcoder;
> bundle-version="1.9.1.v20180227-1645"
>                      org.apache.batik.transcoder [20]
>                        Unresolved requirement: Import-Package:
> org.apache.batik.anim.dom; version="1.9.1"
>                          -> Export-Package: org.apache.batik.anim.dom;
> bundle-symbolic-name="org.apache.batik.anim";
> bundle-version="1.9.1.v20180227-1645"; version="1.9.1"
>                             org.apache.batik.anim [8]
>                               Unresolved requirement: Import-Package:
> org.w3c.dom.events; ext="split"; version="1.9.1"; mandatory:="ext"
> 
> It looks like it fails to find package org.w3c.dom.events v1.9.1. However
> this package exists in bundle org.w3c.dom.events v3.0.0. The feature and
> product file also include this bundle.
> Do you guys know why?
> 
> If this issue cannot be resolved, BIRT still has to use batik 1.7.1.

Which version of Orbit are you getting Batik from? It looks like issues that were present at a time but have been fixed now (or it's another similar one that we did not catch). http://download.eclipse.org/tools/orbit/downloads/drops/S20180504181223/repository is the most up to date version.
Comment 73 Pierre-Charles David CLA 2018-05-24 05:06:26 EDT
(In reply to Yulin Wang from comment #71)
> If this issue cannot be resolved, BIRT still has to use batik 1.7.1.

Batik 1.7.1 is no longer available in the Orbit repo for Photon.

Roland mentioned earlier that "Also, given that BIRT will likely continue using the Orbit R20170307180635 (Neon.3) build, it should be safe to remove Batik 1.6 & 1.7 from active builds."

It means the Photon repo will still contain a mix of Batik versions (at least 1.7.1 and 1.9.1). Not sure if this is a problem (it's not worse than what we had before).
Comment 74 Ed Willink CLA 2018-05-24 06:01:19 EDT
(In reply to Yulin Wang from comment #71)
>                          -> Export-Package: org.apache.batik.anim.dom;
> bundle-symbolic-name="org.apache.batik.anim";
> bundle-version="1.9.1.v20180227-1645"; version="1.9.1"
>                             org.apache.batik.anim [8]
>                               Unresolved requirement: Import-Package:
> org.w3c.dom.events; ext="split"; version="1.9.1"; mandatory:="ext"

"1.9.1.v20180227-1645" is seriously old compared to the current Orbit.

org.w3c.dom.events ... version="1.9.1" looks like a 'typo' in the seriously old version.

The 'typo' does not appear to be present in S20180504181223

v20180227 does not seem to correspond to any build on http://download.eclipse.org/tools/orbit/downloads/drops.

S20180302171354 is closest and has the 'typo'.
Comment 75 Yulin Wang CLA 2018-05-24 13:20:06 EDT
Thanks for your help. Using batik from latest orbit can resolve the dependency issue.

However I hit another problem: java.lang.IncompatibleClassChangeError: Class org.apache.batik.css.parser.Parser does not implement the requested interface org.w3c.css.sac.Parser

Is that caused by batik API change? Our code used to work in batik 1.7.
Comment 76 Roland Grunberg CLA 2018-05-24 14:04:18 EDT
(In reply to Ed Willink from comment #74)
> (In reply to Yulin Wang from comment #71)
> >                          -> Export-Package: org.apache.batik.anim.dom;
> > bundle-symbolic-name="org.apache.batik.anim";
> > bundle-version="1.9.1.v20180227-1645"; version="1.9.1"
> >                             org.apache.batik.anim [8]
> >                               Unresolved requirement: Import-Package:
> > org.w3c.dom.events; ext="split"; version="1.9.1"; mandatory:="ext"
> 
> "1.9.1.v20180227-1645" is seriously old compared to the current Orbit.

The bundle qualifiers are generated based on the timestamp of the last change to that module (that resulted in change to actual change to the content/metadata). There are a few Batik bundles with this older qualifier simply because they haven't seen any changes since that date.

> org.w3c.dom.events ... version="1.9.1" looks like a 'typo' in the seriously
> old version.
> 
> The 'typo' does not appear to be present in S20180504181223

I believe org.w3c.dom.events package is provided by a Batik bundle as well.

IU: org.apache.batik.ext 1.9.1.v20180227-1645
* java.package/org.w3c.dom.events/1.9.1
IU: javax.xml 1.3.4.v201005080400
* java.package/org.w3c.dom.events/2.0.0
IU: org.w3c.dom.events 3.0.0.draft20060413_v201105210656
* java.package/org.w3c.dom.events/3.0.0

(In reply to Yulin Wang from comment #75)
> However I hit another problem: java.lang.IncompatibleClassChangeError: Class
> org.apache.batik.css.parser.Parser does not implement the requested
> interface org.w3c.css.sac.Parser
> 
> Is that caused by batik API change? Our code used to work in batik 1.7.

This might cause issues if you have both batik 1.6/17 mixed in with 1.9.1.

@Pierre, thoughts on that error ?
Comment 77 Yulin Wang CLA 2018-05-24 14:07:45 EDT
No, I don't use batik 1.7 any more. Only batik 1.9 is used.
Comment 78 Yulin Wang CLA 2018-05-24 16:49:42 EDT
The issue "java.lang.IncompatibleClassChangeError: Class org.apache.batik.css.parser.Parser does not implement the requested interface org.w3c.css.sac.Parser" has been fixed in BIRT side. Basically it's caused by sequence difference in MutliSourcePackage from BundleLoader, if there are multiple org.w3c.css.sac.jar are found. Sequence adjust to required bundles can fix this issue. I think this issue is not related to batik change.


Remaining issue seems related to batik change. There are multiple copies of org.w3c.dom.events (one from batik.ext, and the other from org.w3c.dom.events bundle), and both match the version check >=1.9.1. Batik seems not able to find the correct one.
Here is the error:
java.lang.LinkageError: loader constraint violation: when resolving method "org.apache.batik.anim.dom.SVGOMStyleElement.addEventListenerNS(Ljava/lang/String;Ljava/lang/String;Lorg/w3c/dom/events/EventListener;ZLjava/lang/Object;)V" the class loader (instance of org/eclipse/osgi/internal/loader/EquinoxClassLoader) of the current class, org/apache/batik/anim/dom/SVGOMStyleElement, and the class loader (instance of org/eclipse/osgi/internal/loader/EquinoxClassLoader) for the method's defining class, org/apache/batik/dom/AbstractNode, have different Class objects for the type org/w3c/dom/events/EventListener used in the signature
Comment 79 Roland Grunberg CLA 2018-05-24 17:07:06 EDT
(In reply to Yulin Wang from comment #78) 
> Remaining issue seems related to batik change. There are multiple copies of
> org.w3c.dom.events (one from batik.ext, and the other from
> org.w3c.dom.events bundle), and both match the version check >=1.9.1. Batik
> seems not able to find the correct one.

Would it be possible to use a Require-Bundle on the batik one, or restrict the range to only include 1.9.1 ?
Comment 80 Yulin Wang CLA 2018-05-24 17:14:31 EDT
No. The reason is org.w3c.dom.smil_1.0.1.v200903091627.jar depends on package org.w3c.dom.events [3.0.0,4.0.0), so no way to remove org.w3c.dom.events.3.0.0.jar and use batik's.

Why does batik.ext keep own org.w3c.dom.events without using dependency bundle?
Comment 81 Pierre-Charles David CLA 2018-05-25 02:52:52 EDT
(In reply to Yulin Wang from comment #80)
> No. The reason is org.w3c.dom.smil_1.0.1.v200903091627.jar depends on
> package org.w3c.dom.events [3.0.0,4.0.0), so no way to remove
> org.w3c.dom.events.3.0.0.jar and use batik's.
> 
> Why does batik.ext keep own org.w3c.dom.events without using dependency
> bundle?

Probably a mistake on my part. Anthony did the packaging of previous versions, and I've had to rediscover all the traps and weird issues (plus the additional ones specific to 1.9).

My analysis at the time can be seen in comment 44. After the facts, it seems that I overlooked the fact that despite the different name, org.w3c.dom.events.3.0.0.jar which is already in Orbit is identical the org.w3c.dom.events part of Batik-Ext.

At the time, I wrote:

> batik-anim uses:
> - types from org.w3c.dom, but only the ones that are available from the JRE.
> - types from org.w3c.dom.events, both from the JDK and ones that only exists in batik-ext.
> 
> Here we'd want to obtain org.w3c.dom from the JRE, and org.w3c.dom.events from batik-ext (it also includes the org.w3c.dom.events.* provided by the JRE). It *seems* this should be possible by importing the tagged/annotated package and disabling the generation of uses directives (which would re-created an implicit and un-tagged dependency towards the package).

Actually, we could get all the org.w3c.dom.events.* needed from org.w3c.dom.events_3.0.0.draft20060413_v201105210656.jar and avoid batik-ext completely here.

> batik-dom uses:
> - types from org.w3c.dom which come from both the JRE (e.g. Document, Element), *and* ElementTraversal which is specific to batik-ext.
> - types from org.w3c.dom.events, but only ones which are available from the JRE.

We've handled the ElementTraversal case by putting a private copy of it directly in the batik-dom JAR, and nothing particular is needed for org.w3c.dom.events here (whether it resolves towards the standard version in the JRE or the augmented one in org.w3c.dom.events_3.0.0.draft20060413_v201105210656.jar at runtime should not matter.

> batik-bridge uses:
> - types from org.w3c.dom which come from both the JRE, *and* ones which are specific to batik-ext (Window and Location).
> - types from org.w3c.dom.events, but only ones which are available from the JRE.

Same approach as for batik-dom (private copies of org.w3c.dom.{Window,Location}, nothing special for org.w3c.dom.events.

It *looks* like maybe we could get rid of batik-ext and use org.w3c.dom.events_3.0.0.draft20060413_v201105210656.jar instead everywhere. It seems like a cleaner approach, but frankly after having been hit by so many unforeseen issues, I'm not very comfortable making such a change this late.

A less impacting change which maybe could fix the issue would be to export the org.w3c.dom.events package in batik-ext with version 3.0.0 (instead of 1.9.1 currently, the version of the bundle). org.w3c.dom.smil_1.0.1.v200903091627.jar uses "Import-Package: org.w3c.dom,org.w3c.dom.events;version="[3.0.0,4.0.0)"", so with such a change it could resolve to the (identical) version in batik-ext.

I'll prepare a patch for the second solution and try to think more about the first while waiting for your opinions, Roland and Yulin.
Comment 82 Pierre-Charles David CLA 2018-05-25 03:17:03 EDT
@Roland: we're at Photon RC2+0 today, what's the hard deadline for the final Orbit build for Photon?
Comment 83 Eclipse Genie CLA 2018-05-25 03:18:34 EDT
New Gerrit change created: https://git.eclipse.org/r/123302
Comment 84 Ed Willink CLA 2018-05-25 03:22:41 EDT
The duplicate org.w3c.dom.events seems like an unfortunate oversight. But trying to fudge ways of tolerating the duplicate risks far more problems, certainly in the long term.

For instance, one related problem, Bug 534933, has recently occurred with a duplicate org.slf4j when one of the versions suddenly changed to being signed. 

Presumably redistributing a non-identical version of something in Orbit is contrary to at least the spirit of the Orbit guidelines.
Comment 85 Pierre-Charles David CLA 2018-05-25 03:52:44 EDT
Note that the just released (yesterday) Batik 1.10 [1] (released to fix another CVE [2]) includes changes regarding these: from what I see, to ensure Java 10 compat, Batik themselves had to deal with these "split package" issues (at least the ones overlapping JRE-provided packages). They renamed the non-standard org.w3c.dom[.events] types they use into their own org.apache.batik.w3c.dom[.events] namespace [3] [4]. If/when Batik 1.10 is packaged in Orbit, maybe this particular issue will go away (maybe; at this point I'm not sure of anything anymore :) ).

[1] https://mail-archives.apache.org/mod_mbox/xmlgraphics-batik-users/201805.mbox/%3C000401d3f28e%243a4e6390%24aeeb2ab0%24%40gmail.com%3E
[2] https://mail-archives.apache.org/mod_mbox/xmlgraphics-batik-users/201805.mbox/%3C000701d3f28f%24d01860a0%24704921e0%24%40gmail.com%3E
[3] https://svn.apache.org/viewvc?view=revision&revision=1830543
[4] https://svn.apache.org/viewvc?view=revision&revision=1830681
Comment 86 Alexander Kurtakov CLA 2018-05-25 03:55:22 EDT
Pierre, do you plan working on 1.10? It would be nice to get smth saner.
Comment 87 Pierre-Charles David CLA 2018-05-25 04:12:36 EDT
(In reply to Alexander Kurtakov from comment #86)
> Pierre, do you plan working on 1.10? It would be nice to get smth saner.

Not sure. I've already spent a lot more time than what I had planned on 1.9.1, to the detriment of my work on Sirius. Let's finish 1.9.1 for good first, and we'll see.
Comment 88 Eclipse Genie CLA 2018-05-25 09:25:18 EDT
New Gerrit change created: https://git.eclipse.org/r/123335
Comment 89 Pierre-Charles David CLA 2018-05-25 09:30:21 EDT
(In reply to Eclipse Genie from comment #88)
> New Gerrit change created: https://git.eclipse.org/r/123335

This one corresponds to the cleaner approach mentioned above: it gets rid of the whole org.apache.batik.ext bundle, the reasoning being that the types it provided either:
1. have been copied directly into the bundles that needed them, or
2. will be provided by the existing org.w3c.dom.events_3.0.0.draft20060413_v201105210656.jar, with a more correct version of 3.0.0 (which should be compatible with SMIL).

I've made some initial tests with versions of GMF Runtime and Sirius built on top of this, have not seen any regression (but these are just some quick tests).

I'm not sure how this can be tested by BIRT without actually merging this in Orbit and getting an I-build with the result.
Comment 90 Roland Grunberg CLA 2018-05-25 10:34:41 EDT
(In reply to Pierre-Charles David from comment #89)
> (In reply to Eclipse Genie from comment #88)
> > New Gerrit change created: https://git.eclipse.org/r/123335
> 
> This one corresponds to the cleaner approach mentioned above: it gets rid of
> the whole org.apache.batik.ext bundle, the reasoning being that the types it
> provided either:
> 1. have been copied directly into the bundles that needed them, or
> 2. will be provided by the existing
> org.w3c.dom.events_3.0.0.draft20060413_v201105210656.jar, with a more
> correct version of 3.0.0 (which should be compatible with SMIL).
> 
> I've made some initial tests with versions of GMF Runtime and Sirius built
> on top of this, have not seen any regression (but these are just some quick
> tests).
> 
> I'm not sure how this can be tested by BIRT without actually merging this in
> Orbit and getting an I-build with the result.

I prefer this approach over the one to export the package from batik at 3.0.0.

It would be best to do our final release anytime before June 1st as we should at least give a week for projects to adopt the latest repo. For most, there would be no perceived changes other than updating the repository reference.
Comment 91 Yulin Wang CLA 2018-05-25 11:49:20 EDT
(In reply to Pierre-Charles David from comment #89) 
> I'm not sure how this can be tested by BIRT without actually merging this in
> Orbit and getting an I-build with the result.

How can BIRT get new batik 1.10 or new patch 1.9.x to test? Can you tell me the repo URL?
I think this is exactly what we want, i.e. batik reuses w3c.dom.events from required bundles, instead of own copies.
Comment 92 Pierre-Charles David CLA 2018-05-25 12:15:16 EDT
(In reply to Yulin Wang from comment #91)
> (In reply to Pierre-Charles David from comment #89) 
> > I'm not sure how this can be tested by BIRT without actually merging this in
> > Orbit and getting an I-build with the result.
> 
> How can BIRT get new batik 1.10

Batik 1.10 will have to wait.

> or new patch 1.9.x to test? Can you tell me the repo URL?

There is none yet. Possible solutions:
1. The patch above is merged, a new Orbit build is launched and produces an I-build that you can use instead of the S-build.
2. I (or someone else) publishes a locally-built version of Orbit somewhere accessible on the web, and creates an artificial pseudo-Orbit repo that points to this (and the legacy CVS part of Orbit).
3. If you feel like it, you can get the patch and build the repo yourself, but it's more involved.

I'll see if I can find a quick way to publish the version I built (and tested) locally (option 2).

> I think this is exactly what we want, i.e. batik reuses w3c.dom.events from
> required bundles, instead of own copies.
Comment 93 Pierre-Charles David CLA 2018-05-25 12:27:58 EDT
(In reply to Pierre-Charles David from comment #92)
> (In reply to Yulin Wang from comment #91)
> > (In reply to Pierre-Charles David from comment #89) 
> > > I'm not sure how this can be tested by BIRT without actually merging this in
> > > Orbit and getting an I-build with the result.
> > 
> > How can BIRT get new batik 1.10
> 
> Batik 1.10 will have to wait.
> 
> > or new patch 1.9.x to test? Can you tell me the repo URL?
[...]
> I'll see if I can find a quick way to publish the version I built (and
> tested) locally (option 2).

Can you try with http://pcdavid.net/orbit-recipes/ (as a replacement for http://download.eclipse.org/tools/orbit/downloads/drops/S20180504181223/)?
Comment 94 Yulin Wang CLA 2018-05-25 12:53:03 EDT
(In reply to Pierre-Charles David from comment #93)
> Can you try with http://pcdavid.net/orbit-recipes/ (as a replacement for
> http://download.eclipse.org/tools/orbit/downloads/drops/S20180504181223/)?

[ERROR]   Missing requirement: org.eclipse.birt.feature.group 4.8.0.qualifier requires 'com.lowagie.text [2.1.7,2.1.8)' but it could not be found
Comment 95 Pierre-Charles David CLA 2018-05-25 13:02:32 EDT
(In reply to Yulin Wang from comment #94)
> (In reply to Pierre-Charles David from comment #93)
> > Can you try with http://pcdavid.net/orbit-recipes/ (as a replacement for
> > http://download.eclipse.org/tools/orbit/downloads/drops/S20180504181223/)?
> 
> [ERROR]   Missing requirement: org.eclipse.birt.feature.group
> 4.8.0.qualifier requires 'com.lowagie.text [2.1.7,2.1.8)' but it could not
> be found

I think it should be fixed by https://github.com/pcdavid/orbit-recipes/commit/653be5ab820e0eb6cdff0d73fdc745e926aabf18.
Comment 96 Pierre-Charles David CLA 2018-05-28 07:21:22 EDT
@Roland, it seems Yulin has trouble consuming the unofficial repo I setup for testing https://git.eclipse.org/r/c/123335/.

I'm not familiar with the rules in Orbit (especially this late in the RCs). Do you require a +2 vote from another commiter for a patch to be merged? Otherwise I propose to merge it and get an I-build with it asap.
Comment 98 Roland Grunberg CLA 2018-05-28 13:51:06 EDT
@Yulin, @Pierre :

Can you try http://download.eclipse.org/tools/orbit/downloads/drops/I20180528174014/repository to test ?
Comment 99 Pierre-Charles David CLA 2018-05-29 03:18:15 EDT
(In reply to Roland Grunberg from comment #98)
> @Yulin, @Pierre :
> 
> Can you try
> http://download.eclipse.org/tools/orbit/downloads/drops/I20180528174014/
> repository to test ?

It seems to be OK for GMF: I've pushed a change which consumes this version of Orbit and removes the batik-ext dependencies from GMF Runtime and all tests are green: https://ci.eclipse.org/gmf-runtime/job/gmf-runtime-master/1862/

This built is not yet promoted, but I'll use it to check on the Sirius side.
Comment 100 Pierre-Charles David CLA 2018-05-29 11:48:00 EDT
For information/reference, it seems the same issue that hit BIRT (or something very similar) has also hit Papyrus: https://dev.eclipse.org/mhonarc/lists/mdt-papyrus.dev/msg04348.html

There's an RC2 of GMF Runtime built against the latest Orbit I-build available at http://download.eclipse.org/modeling/gmp/gmf-runtime/updates/milestones/S201805290950, and a corresponding version of Sirius should be coming soon.

@Yulin: have you been able to test if the new version fixes the issue for you?
Comment 101 Quentin Le Menez CLA 2018-05-29 12:00:20 EDT
We do have a very strange comportment with the latest update. 
Moreover we get hit by print messages from batik in our logs (inherited through GMF_Runtime) - from DefaultGraphics2D. Did you know if we had to instantiate new methods to override these ?
Comment 102 Yulin Wang CLA 2018-05-29 14:40:07 EDT
(In reply to Pierre-Charles David from comment #100)
> For information/reference, it seems the same issue that hit BIRT (or
> something very similar) has also hit Papyrus:
> https://dev.eclipse.org/mhonarc/lists/mdt-papyrus.dev/msg04348.html
> 
> There's an RC2 of GMF Runtime built against the latest Orbit I-build
> available at
> http://download.eclipse.org/modeling/gmp/gmf-runtime/updates/milestones/
> S201805290950, and a corresponding version of Sirius should be coming soon.
> 
> @Yulin: have you been able to test if the new version fixes the issue for
> you?

I can pass the build now but I still get the error in runtime: java.lang.LinkageError: loader constraint violation: loader (instance of <bootloader>) previously initiated loading for a different type with name "org/w3c/dom/events/EventListener"

I debugged our code as well as batik code, and find a problem in org.apache.batik.bridge/manifest.mf
Imported Package "org.w3c.dom.events" doesn't specify version "3.0.0", so it may refer to "org.w3c.dom.events" from JDK, and leads to linkage error.
You may forget to do so because I see org.apache.batik.anim is correct.
If possible, please also check other batik bundles have same problem.
Comment 103 Roland Grunberg CLA 2018-05-29 15:09:01 EDT
(In reply to Yulin Wang from comment #102)
> I can pass the build now but I still get the error in runtime:
> java.lang.LinkageError: loader constraint violation: loader (instance of
> <bootloader>) previously initiated loading for a different type with name
> "org/w3c/dom/events/EventListener"
> 
> I debugged our code as well as batik code, and find a problem in
> org.apache.batik.bridge/manifest.mf
> Imported Package "org.w3c.dom.events" doesn't specify version "3.0.0", so it
> may refer to "org.w3c.dom.events" from JDK, and leads to linkage error.
> You may forget to do so because I see org.apache.batik.anim is correct.
> If possible, please also check other batik bundles have same problem.

The following packages have requirement on org.w3c.dom.events at 0.0.0.

IU: org.apache.batik.css 1.9.1.v20180313-1559
-> package org.w3c.dom.events 0.0.0

IU: org.apache.batik.dom 1.9.1.v20180528-1434
-> package org.w3c.dom.events 0.0.0

IU: org.apache.batik.bridge 1.9.1.v20180313-1559
-> package org.w3c.dom.events 0.0.0

IU: org.apache.batik.dom.svg 1.9.1.v20180313-1559
-> package org.w3c.dom.events 0.0.0

IU: org.apache.batik.extension 1.9.1.v20180313-1559
-> package org.w3c.dom.events 0.0.0

IU: org.apache.batik.swing 1.9.1.v20180313-1559
-> package org.w3c.dom.events 0.0.0

In most cases, it seems they're either in osgi.bnd Import-Package without a version, or picked up by the optional requirement catch-all.
Comment 104 Eclipse Genie CLA 2018-05-30 02:40:33 EDT
New Gerrit change created: https://git.eclipse.org/r/123594
Comment 105 Pierre-Charles David CLA 2018-05-30 02:51:40 EDT
(In reply to Eclipse Genie from comment #104)
> New Gerrit change created: https://git.eclipse.org/r/123594

This one adds the explicit version requirement for org.w3c.dom.events to all the bundles mentioned by Roland. I think I'll push it directly after a few tests to try and reduce the delays.
Comment 106 Pierre-Charles David CLA 2018-05-30 03:14:38 EDT
(In reply to Quentin Le Menez from comment #101)
> We do have a very strange comportment with the latest update. 
> Moreover we get hit by print messages from batik in our logs (inherited
> through GMF_Runtime) - from DefaultGraphics2D. Did you know if we had to
> instantiate new methods to override these ?

Can you give some details?

From what I see, Batik's own DefaultGraphics2D "implements" many of its methods by simply printing a message, but GMF Runtime's Graphics2DToGraphicsAdaptor sub-class overrides most of them with actual behavior. Only two are not overriden on the GMF side (setXORMode(Color) and copyArea(int, int, int, int, int, int)) and indeed will produce traces if called. However I don't find any actual calls to these (neither in Batik, GMF or Sirius) and none of these two classes seem to have changed "ever" (I just checked, and they were the same in Batik 1.6).
Comment 107 Quentin Le Menez CLA 2018-05-30 03:24:09 EDT
Sure,
It happens when dropping an elment in the diagram (seems to be tied to the svg portion of the problem). We get this kind of traces in  the console/build log:
draw(Shape)
draw(Shape)
dispose
draw(Shape)
draw(Shape)
dispose
fill

which seem to come from org.apache.batik.ext.awt.g2d.DefaultGraphics2D's methods draw(Shape s), dispose() and fil(Shape s).
Comment 108 Pierre-Charles David CLA 2018-05-30 04:24:31 EDT
(In reply to Quentin Le Menez from comment #107)
> Sure,
> It happens when dropping an elment in the diagram (seems to be tied to the
> svg portion of the problem). We get this kind of traces in  the
> console/build log:
> draw(Shape)
> draw(Shape)
> dispose
> draw(Shape)
> draw(Shape)
> dispose
> fill
> 
> which seem to come from org.apache.batik.ext.awt.g2d.DefaultGraphics2D's
> methods draw(Shape s), dispose() and fil(Shape s).

Are you sure these traces are new? The code they come from does not look like it has changed either on the Batik or GMF side. Can you put a conditional breakpoint with "new RuntimeException().printStackTrace(); return false;" on the offending methods in Batik to see what call chain triggers these?
Comment 109 Quentin Le Menez CLA 2018-05-30 06:07:44 EDT
Yes these appeared when we updated to GMF Runtime M7+. I mentioned them here as it seemed linked to batik (because of the mentioned traces) but I may completely mistaken.
Comment 110 Quentin Le Menez CLA 2018-05-30 06:09:26 EDT
(In reply to Quentin Le Menez from comment #109)
> Yes these appeared when we updated to GMF Runtime M7+. I mentioned them here
> as it seemed linked to batik (because of the mentioned traces) but I may
> completely mistaken.
I could not find any code change that would be the cause of this behavior in the Runtime changes though...
Comment 111 Pierre-Charles David CLA 2018-05-30 06:21:06 EDT
(In reply to Pierre-Charles David from comment #105)
> (In reply to Eclipse Genie from comment #104)
> > New Gerrit change created: https://git.eclipse.org/r/123594
> 
> This one adds the explicit version requirement for org.w3c.dom.events to all
> the bundles mentioned by Roland. I think I'll push it directly after a few
> tests to try and reduce the delays.

It doesn't seem to fix the issue. I may have done something wrong in my tests, but I get the LinkageError when running the Sirius tests:

java.lang.LinkageError: loader constraint violation in interface itable initialization: when resolving method "org.apache.batik.dom.AbstractDocument.createEvent(Ljava/lang/String;)Lorg/w3c/dom/events/Event;" the class loader (instance of org/eclipse/osgi/internal/loader/EquinoxClassLoader) of the current class, org/apache/batik/anim/dom/SVGOMDocument, and the class loader (instance of <bootloader>) for interface org/w3c/dom/events/DocumentEvent have different Class objects for the type org/w3c/dom/events/Event used in the signature

I'm still on it, but without much clue for the moment.
Comment 112 Pierre-Charles David CLA 2018-05-30 11:03:27 EDT
(In reply to Pierre-Charles David from comment #111)
> (In reply to Pierre-Charles David from comment #105)
> > (In reply to Eclipse Genie from comment #104)
> > > New Gerrit change created: https://git.eclipse.org/r/123594
> > 
> > This one adds the explicit version requirement for org.w3c.dom.events to all
> > the bundles mentioned by Roland. I think I'll push it directly after a few
> > tests to try and reduce the delays.
> 
> It doesn't seem to fix the issue. I may have done something wrong in my
> tests, but I get the LinkageError when running the Sirius tests:
> 
> java.lang.LinkageError: loader constraint violation in interface itable
> initialization: when resolving method
> "org.apache.batik.dom.AbstractDocument.createEvent(Ljava/lang/String;)Lorg/
> w3c/dom/events/Event;" the class loader (instance of
> org/eclipse/osgi/internal/loader/EquinoxClassLoader) of the current class,
> org/apache/batik/anim/dom/SVGOMDocument, and the class loader (instance of
> <bootloader>) for interface org/w3c/dom/events/DocumentEvent have different
> Class objects for the type org/w3c/dom/events/Event used in the signature
> 
> I'm still on it, but without much clue for the moment.

I'm commenting/iterating on different approaches on https://git.eclipse.org/r/c/123594 to avoid flooding the ticket more than it is already. If someone (Roland, Yulin, Quentin?) has some idea, any help would be appreciated.
Comment 113 Yulin Wang CLA 2018-05-30 11:44:06 EDT
(In reply to Pierre-Charles David from comment #112)
> I'm commenting/iterating on different approaches on
> https://git.eclipse.org/r/c/123594 to avoid flooding the ticket more than it
> is already. If someone (Roland, Yulin, Quentin?) has some idea, any help
> would be appreciated.

I added some comments to your change to org.apache.batik.anim_1.9.1, and it's also applicable to other jars.
Comment 114 Yulin Wang CLA 2018-05-31 12:40:39 EDT
Do you have new build after recent change?
Comment 115 Pierre-Charles David CLA 2018-06-01 04:40:52 EDT
(In reply to Yulin Wang from comment #114)
> Do you have new build after recent change?

Even with all the variants of https://git.eclipse.org/r/c/123594 I could think of, the LinkageError still happens. I'm sorry I couldn't find a solution, but the version of Batik 1.9.1 in the final build of Orbit made for Photon yesterday should be considered broken, except for some very specific cases (the platform for example uses only the batik-css component which is pretty isolated, and should be safe with the 1.9.1 version they've already adopted).

As mentioned on cross-project yesterday [1] the safest thing to do is to revert back to the previous versions of Batik. This means consuming the Orbit repo from Oxygen.3 (they are not available in the Photon version of Orbit anymore). That's what I'm doing for GMF Runtime and Sirius, but Papyrus, BIRT and Graphiti(?) should probably do this too.

[1] https://dev.eclipse.org/mhonarc/lists/cross-project-issues-dev/msg15623.html
Comment 116 Eclipse Genie CLA 2018-06-04 14:22:08 EDT
New Gerrit change created: https://git.eclipse.org/r/123946
Comment 118 Thorsten Meinl CLA 2018-06-11 16:18:56 EDT
I ran into one of those infamous LinkageErrors today (with Batik 1.6 and 1.7, though). What I'm wondering is why is this old org.w3c.dom.events bundle needed anyway? The org.w3c.dom.* packages have been part of the JRE since ages by now so there should be no need to ship them in a bundle. This would immediately solve all related problems.
Comment 119 Pierre-Charles David CLA 2018-06-12 08:19:13 EDT
(In reply to Thorsten Meinl from comment #118)
> I ran into one of those infamous LinkageErrors today (with Batik 1.6 and
> 1.7, though).

Can you give some details on the situation which triggered the error? It may not be strictly related to this particular ticket, but any information that could shade more light on the problem could (maybe) help finding a solution.

> What I'm wondering is why is this old org.w3c.dom.events
> bundle needed anyway? The org.w3c.dom.* packages have been part of the JRE
> since ages by now so there should be no need to ship them in a bundle. This
> would immediately solve all related problems.

From my understanding, it's still needed for at least two reasons:
* it exports its packages with version 3.0.0, and some bundles (for example, the SMIL bundle, used by BIRT, see comment 80) require this particular version. I think such a versioned Import-Package would not with the unversioned, JRE-provided package.
* more importantly, it may look like the contents of the org.w3c.dom.events bundle is redundant with what is provided by the JRE, but the content of the org.w3c.dom.events Java package provided by the Orbit bundle is a super-set of what is available in the JRE: org.w3c.dom.events.KeyboardEvent, org.w3c.dom.events.MutationNameEvent and org.w3c.dom.events.TextEvent, all of which are used by parts of Batik, are not present in the JRE. The fact that they are in a Java package which clashes with one of the JRE-provided ones is actually a big part of the issue.

The good news is that the second point should not be a problem anymore (for Batik) with the recent Batik 1.10, which now use a different, Batik-specific org.apache.batik.w3c.dom.events namespace for these "extended types": https://svn.apache.org/viewvc?view=revision&revision=1830681
Comment 120 Thorsten Meinl CLA 2018-06-13 02:47:03 EDT
We are both using GMF and Birt in our application. GMF relies on Batik 1.6 whereas Birt relies on 1.7. If GMF was invoked before Birt, it would load the classes from the JRE (as Batik 1.6 does not have a version range on the package import). Then everyting using Batik 1.7 would fail with a LinkageError. We solved it by adding fragments for Batik 1.6 that import the package with a version range constraint. This works until someone else loads the classes from the JRE again.
Comment 121 Thorsten Meinl CLA 2018-06-13 11:18:51 EDT
Actually, org.apache.xerces is another bundle that loads org.w3c.dom.events.Event from the JRE. I.e. if you parse any XML file with Xerces before using Batik 1.7 (e.g. a log4j XML configuration) it breaks again. Using the same fragment hack as with Batik seems to work here, too. But this whole topic is a huge mess in my opinion.
Comment 122 Roland Grunberg CLA 2018-07-16 17:01:35 EDT
I was able to reproduce the errors mentioned with the BIRT CBI build (in the org.eclipse.birt.chart.tests) .

java.lang.LinkageError: loader constraint violation in interface itable initialization: when resolving method "org.apache.batik.dom.AbstractDocument.createEvent(Ljava/lang/String;)Lorg/w3c/dom/events/Event;" the class loader (instance of org/eclipse/osgi/internal/loader/EquinoxClassLoader) of the current class, org/apache/batik/dom/svg/SVGOMDocument, and the class loader (instance of <bootloader>) for interface org/w3c/dom/events/DocumentEvent have different Class objects for the type org/w3c/dom/events/Event used in the signature

With some modifications to the Batik bundles under orbit-recipes (mainly just eliminating org.w3c.dom.events from Import-Package to ensure they resolve from the JRE itself) the tests seem to pass now.

I still fail get failures further down in org.eclipse.birt.report.designer.tests but they seem unrelated.
Comment 123 Eclipse Genie CLA 2018-07-17 12:21:37 EDT
New Gerrit change created: https://git.eclipse.org/r/126192
Comment 124 Eclipse Genie CLA 2018-07-17 12:21:41 EDT
New Gerrit change created: https://git.eclipse.org/r/126191
Comment 125 Eclipse Genie CLA 2018-07-24 10:32:53 EDT
New Gerrit change created: https://git.eclipse.org/r/126561
Comment 126 Eclipse Genie CLA 2018-07-24 10:41:31 EDT
New Gerrit change created: https://git.eclipse.org/r/126562
Comment 127 Dani Megert CLA 2018-07-24 10:42:10 EDT
What's the state of this bug? It is targeted for Photon which shipped last month.
Comment 128 Roland Grunberg CLA 2018-07-24 10:47:34 EDT
(In reply to Dani Megert from comment #127)
> What's the state of this bug? It is targeted for Photon which shipped last
> month.

Sorry for the noise. It seems like git-revert didn't properly create a Change-Id so had to abandon and resubmit.

Most of the Batik 1.9.1 stack (excluding portions used by platform) were disabled in the Photon release to avoid issues discovered there. I believe I have a fix for them but would need one of the projects where the original issue was uncovered to verify it solves their issue.

My test case has mainly been through running BIRT tests with Tycho as that's where I was able to reproduce the problem.
Comment 129 Yulin Wang CLA 2018-09-19 14:53:03 EDT
(In reply to Roland Grunberg from comment #128)
> (In reply to Dani Megert from comment #127)
> > What's the state of this bug? It is targeted for Photon which shipped last
> > month.
> 
> Sorry for the noise. It seems like git-revert didn't properly create a
> Change-Id so had to abandon and resubmit.
> 
> Most of the Batik 1.9.1 stack (excluding portions used by platform) were
> disabled in the Photon release to avoid issues discovered there. I believe I
> have a fix for them but would need one of the projects where the original
> issue was uncovered to verify it solves their issue.
> 
> My test case has mainly been through running BIRT tests with Tycho as that's
> where I was able to reproduce the problem.

Could you provide the jars somewhere? I can manually verify it from BIRT side.
Comment 130 Roland Grunberg CLA 2018-09-19 16:10:21 EDT
(In reply to Yulin Wang from comment #129)
> (In reply to Roland Grunberg from comment #128)
> > (In reply to Dani Megert from comment #127)
> > > What's the state of this bug? It is targeted for Photon which shipped last
> > > month.
> > 
> > Sorry for the noise. It seems like git-revert didn't properly create a
> > Change-Id so had to abandon and resubmit.
> > 
> > Most of the Batik 1.9.1 stack (excluding portions used by platform) were
> > disabled in the Photon release to avoid issues discovered there. I believe I
> > have a fix for them but would need one of the projects where the original
> > issue was uncovered to verify it solves their issue.
> > 
> > My test case has mainly been through running BIRT tests with Tycho as that's
> > where I was able to reproduce the problem.
> 
> Could you provide the jars somewhere? I can manually verify it from BIRT
> side.

Sure, I've put an update site at https://rgrunber.fedorapeople.org/orbit-batik/ containing the updated jars.
Comment 133 Alexander Kurtakov CLA 2018-11-15 08:00:58 EST
Roland, anything pending or this one can be closed?
Comment 134 Roland Grunberg CLA 2018-11-15 09:47:16 EST
Thanks, I'll mark this as CLOSED (FIXED) for now.

I was able to confirm with some maintainers of BIRT that the issue seems to be resolved. There are other things to address, such as upgrading a bundle with a known vulnerability, and moving to Batik 1.10, but those can be done separately.