Re: [orbit-dev] Add versions without CVEs?

[Roland Grunberg <rgrunber@xxxxxxxxxx>]

On Fri, 2019-02-22 at 13:16 +0000, Carsten Reckord wrote:
> > xml/org.dom4j_2.1.1/src/main/resources/about_files/about_files/license
> > The license file is named "BSD_3-CLAUSE_NEW_LICENSE.html" and contains
> > the contents from https://github.com/dom4j/dom4j/blob/master/LICENSE.
> > That is, it includes all of the contents of the web page including all of the
> > github framing.
> > This is because https://github.com/dom4j/dom4j/blob/master/LICENSE is
> > listed in the dom4j 2.1.1 pom.
> 
> That's a bit of a strange reference on their end. But I think, given the license conditions themselves and the intent here, it should be safe and much cleaner to replace this with the text content from https://raw.githubusercontent.com/dom4j/dom4j/master/LICENSE.

Yes, please replace their 'HTML LICENSE' with the proper one. Ideally,
all licenses of the same type should evaluate to the same checksum for
the sake of sanity, but of course maybe certain ones use different
whitespace or something like that.

The longer answer is that at :
https://github.com/dom4j/dom4j/blob/master/build.gradle#L76
they point to the exact license that ends up in the pom.xml metadata
and ebr-maven-plugin simply reads this location. I think they need to
point to a raw license location. I don't think this is the first time
this has happened, and it's a common issue with GitHub. If it's
becoming standard practice to use something that isn't the raw license
in the pom license metadata, then maybe ebr might need to work around
the issue.


> > xml/org.dom4j_2.1.1/pom.xml
> > Of course there are expected differences due to version.
> > The generated name is dom4j instead of DOM4J - is that ok or does it need to
> > match the 1.6.1 name?
> 
> I don't think it really matters. That name will only show up in the Orbit build logs. The Bundle-Name of the generated bundle is defined in src/main/resources/OSGI-INF/l10n/bundle.properties, and other manifest headers are taken directly from the maven artifact's manifest.
> 
> I'd probably still change it personally, to match the previous version, just for consistency.

Agreed, keeping things the same in this case is probably the best thing
to do. The bundle name here is mostly for display purposes and in most
cases, people just trust what ebr generates but you're free to preserve
what was used from before.


> > The recipe.excludes property is missing - I assume this was added manually
> > and I should add it as well?
> 
> If the Maven artifact still packages those files in the new version, I would add it.
> 
> What I like to do usually is to compare the finished bundle with the previous version if there is one, or just look at its contents in general. I use a couple of shell oneliners for that. Here's an example comparing the bundle contents of the old HttpClient 4.5.5 and the new 4.5.6 bundle:
> 
> diff -u \
>    <(unzip -Z -1 org.apache.httpcomponents.httpclient_4.5.5/target/org.apache.httpcomponents.httpclient-4.5.5-SNAPSHOT.jar) \
>    <(unzip -Z -1 org.apache.httpcomponents.httpclient_4.5.6/target/org.apache.httpcomponents.httpclient-4.5.6-SNAPSHOT.jar|sort)
> 
> Of course you need to build both bundles first with mvn -DdirtyWorkingTree=warning clean package

By default the ebr-maven-plugin runs with :

<!-- but filter out maven instructions, we'll generate new ones below -->
<recipe.excludes>META-INF/maven/**/*.*</recipe.excludes>

If there are certain files you wish to exclude that this doesn't cover,
you are free to override the property in the bundle's pom.xml. Just
search for poms containing 'recipe.excludes' to get an idea.

Comparing to the previous version is definitely a great idea, but keep
in mind that some difference may just be new stuff that's introduced in
a later version.


> > xml/org.dom4j_2.1.1/osgi.bnd
> > Export-Package has some extra lines:
> > *.implementation.*;x-internal:=true;version="${package-version}", \
> >  *.impl.*;x-internal:=true;version="${package-version}", \
> > With regard to dependencies, dom4j 2.1.1 lists several optional
> > dependencies with <scope>runtime</scope>.
> > Should I add these to osgi.bnd Import-Package?
> > How do I map the maven coordinates from
> > https://search.maven.org/artifact/org.dom4j/dom4j/2.1.1/jar to package
> > ids?
> 
> For Im- and Export-Package stuff, I usually check the generated MANIFEST.MF to see what bnd puts in there, and then adjust the osgi.bnd accordingly.
> 
> I have another oneliner for that to make the MANIFEST.MF more readable:
> 
> cat target/MANIFEST.MF | \
>   sed -r -e ':a;N;$!ba;s/\r?\n //g;:b;/(="[^",]+),/!bc;s/(="[^",]+),/\1~/g;bb;:c;s/,([a-z])/,\n \1/g;s/~/,\n      /g;s/;uses/;\n   uses/g'
> 
> This will reassemble the split lines in the manifest and then rebreak them in a more readable fashion.
> 
> It should be easy to check the Export-Package lines for additional packages that look like they should be declared internal (internal, impl, and so on are your usual suspects).
> 
> And for Import-Package declarations, the default " *;resolution:=optional" rule in osgi.bnd will cause all dependencies that are not treated specifically to show up as optional and version-less in the generated MANIFEST.MF. You can have a look at the output and check which ones you might want to add to the osgi.bnd explicitly. Matching the packages back to the Maven dependencies should be fairly obvious. For version ranges, I'd usually go for the version listed in the pom up to the next major exclusive, i.e. a ${range;[==,+);${ version}}. 

The export-package restrictions there are just for packages likely not
meant to be exported. I just took a quick look at :
http://central.maven.org/maven2/org/dom4j/dom4j/2.1.1/dom4j-2.1.1.pom

It seems all dependencies are only with runtime scope and yet I can
clearly see them being required in various places of the sources, so
maybe scope=runtime isn't right. Then again, the pom is only generated
by the gradle build. I would enforce the dependencies as I'm not sure
how it would run without them.

You can always decide if you want to make a package required or
optional by using the 'jdeps' tool, or even simply searching the
sources for all import statements.


> > For future reference, would you prefer that I just open a change request and
> > discuss this there?
> > Or is it ok to ask questions like this on orbit-dev?
> 
> Personally, I find it easier to talk about a concrete Gerrit change. But it's definitively okay to ask here. 

You can always ask questions on orbit-dev. If they're related to a
specific change it's much easier to track what has been done, and what
hasn't on gerrit. Even if your change is a work in progress, you can
certainly post it, and even benefit from others having a look through
and maybe having some advice.

You can also have a look at 
https://wiki.eclipse.org/Orbit/Adding_Bundles_to_Orbit as it has some
information about certain special cases.

Cheers,
-- 
Roland Grunberg