eclipse-emfcloud / emfjson-jackson

emfjson-jackson
Other
16 stars 15 forks source link

Jars contain no OSGi instructions anymore #28

Closed maho7791 closed 2 years ago

maho7791 commented 2 years ago

The released artifacts: https://repo.maven.apache.org/maven2/org/eclipse/emfcloud/emfjson-jackson/2.0.0/ https://oss.sonatype.org/service/local/repositories/snapshots/content/org/eclipse/emfcloud/emfjson-jackson/2.1.0-SNAPSHOT/ have no OSGi Metadata in the Manifest, compared to: https://repo.maven.apache.org/maven2/org/emfjson/emfjson-jackson/1.3.0/

This can be fixed in the pom.xml

martin-fleck-at commented 2 years ago

Fixed with https://github.com/eclipse-emfcloud/emfjson-jackson/pull/29

vhemery commented 2 years ago

@martin-fleck-at @maho7791 I've noticed that since https://github.com/eclipse-emfcloud/emfjson-jackson/commit/f5d0dfa93b2c4c28c706834ca3660cdc9801556f the published plugin name has changed : image

I doubt it is intentional... ? Bnd maven plugin seem to consider only the artifactId, whereas felix was prefixing with the groupId. I don't know how to tell bnd to prefix the artifactId with the groupId.

maho7791 commented 2 years ago

That is correct bnd uses this mapping: https://github.com/bndtools/bnd/tree/master/maven/bnd-maven-plugin#default-bundle-headers for determining the Bundle. -SymbolicName. Is there a problem with this?

maho7791 commented 2 years ago

For P2 usually the bsn in the Manifest of the bundle is relevant. The file names are then used in the artifacts.xml.

maho7791 commented 2 years ago

To configure the bundle symbolic name you can also take that documentation link: There a several ways, one would be to define the Bundle-SymbolicName: ${project.groupId}.${project.artifactId} inline the bnd-maven config. Another way would be more generic to define the bnd internal variable ${bsn} also inline in the parent pom like this: -bsn: ${project.groupId}.${project.artifactId}

If you need help I can assist here.

If this is a problem because you use Require-Bundle then you should switch to Import-Package instead. Because BSN changes can happen all the time, especially when switching to different providers for a API ;-)

vhemery commented 2 years ago

If this is a problem because you use Require-Bundle then you should switch to Import-Package instead. Because BSN changes can happen all the time, especially when switching to different providers for a API ;-)

The bsn has already been changed in 2021 when moved to emf.cloud (along with the package names) with a major version bump. This might be a bit too soon to change it again... (at least the EMF.cloud Model Server source code did not take this new change in account)

@martin-fleck-at For the model server, are we assuming its safer to use only the Jackson implementation provided by EMF.cloud (hence keeping the Require-bundle dependency eg in https://github.com/eclipse-emfcloud/emfcloud-modelserver/blob/962e506fdff5e64a273f1425fcededb56ca07306/bundles/org.eclipse.emfcloud.modelserver.emf/META-INF/MANIFEST.MF#L20 ) or should we move to an Import-Package as maho7791 suggests ? Using Import-Package, the emfcloud package name should already prevent users from trying to use the https://github.com/emfjson/emfjson-jackson implementation, although I'm not sure there will be other P2 publishers of this package...

Whatever solution adopts the Model Server, I like maho7791's 2nd solution to go back to the old bsn :

Another way would be more generic to define the bnd internal variable ${bsn} also inline in the parent pom like this: -bsn: ${project.groupId}.${project.artifactId}

martin-fleck-at commented 2 years ago

Hi @vhemery, Hi @maho7791,

Thank you very much for reporting and following up on this issue! You are right, that was not intentional and I think we should restore the old naming procedure again. @maho7791 If you already know how to do it, any help is greatly appreciated.

Independent from that, it might be worth switching to Import-Package in the modelserver to prevent a similar problem in the future.

vhemery commented 2 years ago

@maho7791 Inserting Bundle-SymbolicName: ${project.groupId}.${project.artifactId} in the pom's <configuration><bnd/></configuration> worked for me, but I couldn't get the -bsn: ${project.groupId}.${project.artifactId} solution working. I also tried with an -@bsn:, separating from other instructions with an empty line, using -runproperties: to define it, but none of my attempt worked. If you could make the working patch, we would greatly appreciate it. If it doesn't work as you expected, I guess we can always fall back to Bundle-SymbolicName: ${project.groupId}.${project.artifactId} (but I fear we may also have to update the jar naming with -output in this case => probably not : the jar name looks correct for mvn publishing, the P2 publishing probably takes the plugin naming in charge ?).

Thank you for your help.

maho7791 commented 2 years ago

I take a look into it to get it your way. It would be nice, if you form your expectations rgarding naming and file name in an issue?

maho7791 commented 2 years ago

BTW if we name the artifactId: org.eclipse.emfcloud.emfjson-jackson, this would change the BSN and the final jar without doin anything else :-)