Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSGi Bundle for issue #11 #32

Closed
wants to merge 14 commits into from
Closed

OSGi Bundle for issue #11 #32

wants to merge 14 commits into from

Conversation

Tibor17
Copy link

@Tibor17 Tibor17 commented Apr 7, 2013

An update of the build script in order to provide OSGi Headers in JavaHamcrest.

@Tibor17
Copy link
Author

Tibor17 commented Apr 7, 2013

Only EasyMock 2.5 or higher is OSGi bundle as a dependency in main code src/main/java.

<macrodef name="jar-osgi-bundle">
<attribute name="modulename" description="Name of the OSGi bundle to jar"/>
<sequential>
<property name="bundlename" value="@{modulename}"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given you can only set the value of a property once in ant, I'm not convinced this is correct.

@Tibor17
Copy link
Author

Tibor17 commented Apr 7, 2013

Bundle-SymbolicName is not configured by property.
So each module has BND with own symbolic name.
Every JAR updated MANIFEST.MF with generated OSGi headers.
JAR contents did not change.

@mattbishop
Copy link

I cloned and built the jar and I think it over-imports packages that are optional:

Import-Package: com.thoughtworks.qdox,com.thoughtworks.qdox.model,java
x.xml.namespace,javax.xml.parsers,javax.xml.xpath,org.easymock,org.jm
ock.core,org.w3c.dom,org.xml.sax,org.xml.sax.helpers

What about making these particular packages optional? They aren't needed for the runtime use of Hamcrest, and if someone wants to use them, they can bring them in:

com.thoughtworks.qdox.*
org.easymock.*
org.jmock.*

@Tibor17
Copy link
Author

Tibor17 commented Apr 8, 2013

@mattbishop
I made the imports optional in BND files.

@scarytom
Copy link
Member

scarytom commented Apr 9, 2013

I'm a little concerned that interleaving the extra work for generating the osgi bundles within the current ant tasks is unclear, and will add time to our normal build path. I'm just exploring ideas, but would it be possible to add an extra ant target for generating osgi, which consumed the source jars, unzipped them, then ran the bnd? Something that we could tack on the end of the existing build would be preferable in my mind.

@sf105
Copy link
Member

sf105 commented Apr 9, 2013

+1 with Tom.

@Tibor17
Copy link
Author

Tibor17 commented Apr 9, 2013

@scarytom
I will do so!
Q: The generator.jar contains wrong maven descriptors:
META-INF/maven/**.pom.properties
META-INF/maven/**.pom.xml
These descriptors have different groupId and artifactId from hamcrest-generator.pom. They should be same.
I was trying not to break the jar contents, but ignoring these two descriptors would simplify the build again.
Can I remove it?

@Tibor17
Copy link
Author

Tibor17 commented Apr 9, 2013

Done.

@mattbishop
Copy link

It or, would you mind measuring the time difference so we know if we have a problem with build times?

My concern with a separate task for the manifest fiddling is human error. I have seen several projects keep it as a separate step, forget, push to sonatype / maven central, then get bugs, spin up a new release, etc. it's a big enough problem that most people include the OSGi step. Interestingly, Apache Commons requires the OSGi headers be built inline. Their parent Pom provides the necessary plugin to do so.

It is important that the manifests are correct on the artifacts in maven central. We don't want to have to require someone to build hamcrest themselves to get the OSGi manifests.

OSGi deployment mechanisms like OBR rely on maven servers like Nexus to provide bundles. These are not systems that usually build their dependencies, nor include them in a /libs style source repository.

If the timings reveal no appreciable difference, would anyone have an objection to keeping the manifest generation in the main build flow?

@Tibor17
Copy link
Author

Tibor17 commented Apr 9, 2013

@mattbishop
The ant build (command ant, ant all, or ant bundle) inlined the headers to the manifest of Maven artifacts, the build takes ca 37 sec. The build without BND takes 35 sec on my system. The difference 2 sec == 6% is acceptable.
You do not need to type "ant osgi".

@mattbishop
Copy link

cool, what is it if you take out the osgi step?

@Tibor17
Copy link
Author

Tibor17 commented Apr 9, 2013

what you mean by step? If you mean commented out the inside of macro def, then it is like the official build.
I am testing two builds. One is the original from Hamcrest official branch and second is my branch. I am comparing the content of JAR files all the time by every build change and I compared the build time exactly this way.

@mattbishop
Copy link

I think we want to know this:

  1. time of current official build (without your osgi patch)
  2. time of your osgi-patched build

We know that #2 takes 37 seconds on your machine. How long does #1 take on
your machine?

On Tue, Apr 9, 2013 at 9:18 AM, Tibor Digana notifications@github.comwrote:

what you mean by step? If you mean commented out the inside of macro def,
then it is like the official build.
I am testing two builds. One is the original from Hamcrest offitial branch
and second is my branch. I am comparing the content of Ar files all the
time by every build change and I compared the build time exactly this way.


Reply to this email directly or view it on GitHubhttps://github.com//pull/32#issuecomment-16123167
.

@Tibor17
Copy link
Author

Tibor17 commented Apr 9, 2013

@mattbishop
Did you see the bnd files and manifest?
It seems to be okay.

@Tibor17
Copy link
Author

Tibor17 commented Apr 9, 2013

For #1 it takes 35 seconds on my laptop.
Can you measure as well?

@mattbishop
Copy link

I can measure, but for this discussion we can say that the osgi manifest
header step adds 2 seconds to the build.

On Tue, Apr 9, 2013 at 9:39 AM, Tibor Digana notifications@github.comwrote:

For #1 #1 it takes 35
seconds on my laptop.
Can you measure as well?


Reply to this email directly or view it on GitHubhttps://github.com//pull/32#issuecomment-16124543
.

@Tibor17
Copy link
Author

Tibor17 commented Apr 10, 2013

I used Karaf OSGi container to see what exported packages will be seen by a real OSGi consumer.
ant bundle -Dversion=1.4
Created mvn structure in Maven local repo and copied the artifacts in ~/.m2/repository/org/hamcrest.
Started the script apache-karaf-2.3.1/bin/karaf.
Installed maven artifacts into OSGi container:

karaf@root> osgi:install mvn:org.hamcrest/hamcrest-core/1.4 Bundle ID: 55 karaf@root> osgi:install mvn:org.hamcrest/hamcrest-integration/1.4 Bundle ID: 56 karaf@root> osgi:install mvn:org.hamcrest/hamcrest-library/1.4 Bundle ID: 57

Print exported osgi packages from the container:

karaf@root> exports 55 ID Packages 55 org.hamcrest.core; version=1.4.0 55 org.hamcrest; version=1.4.0 karaf@root> exports 56 ID Packages 56 org.hamcrest; version=1.4.0 56 org.hamcrest.integration; version=1.4.0 karaf@root> exports 57 ID Packages 57 org.hamcrest.number; version=1.4.0 57 org.hamcrest.beans; version=1.4.0 57 org.hamcrest.object; version=1.4.0 57 org.hamcrest.io; version=1.4.0 57 org.hamcrest.xml; version=1.4.0 57 org.hamcrest; version=1.4.0 57 org.hamcrest.collection; version=1.4.0 57 org.hamcrest.text; version=1.4.0

Later I printed generator and integration packages:
karaf@root> osgi:install mvn:org.hamcrest/hamcrest-generator/1.4 Bundle ID: 60 karaf@root> osgi:install mvn:org.hamcrest/hamcrest-all/1.4 Bundle ID: 61 karaf@root> exports 60 ID Packages 60 org.hamcrest.generator.qdox.directorywalker; version=1.4.0 60 org.hamcrest.generator.qdox.model; version=1.4.0 60 org.hamcrest.generator.qdox.parser.impl; version=1.4.0 60 org.hamcrest.generator.qdox.parser.structs; version=1.4.0 60 org.hamcrest.generator.qdox.tools; version=1.4.0 60 org.hamcrest.generator.config; version=1.4.0 60 org.hamcrest.generator.qdox.model.util; version=1.4.0 60 org.hamcrest.generator.qdox; version=1.4.0 60 org.hamcrest.generator.qdox.parser; version=1.4.0 60 org.hamcrest.generator; version=1.4.0 60 org.hamcrest.generator.qdox.ant; version=1.4.0 60 org.hamcrest.generator.qdox.junit; version=1.4.0 60 org.hamcrest.generator.qdox.model.annotation; version=1.4.0 karaf@root> exports 61 ID Packages 61 org.hamcrest.number; version=1.4.0 61 org.hamcrest.object; version=1.4.0 61 org.hamcrest.generator.config; version=1.4.0 61 org.hamcrest.io; version=1.4.0 61 org.hamcrest.collection; version=1.4.0 61 org.hamcrest.core; version=1.4.0 61 org.hamcrest.beans; version=1.4.0 61 org.hamcrest.xml; version=1.4.0 61 org.hamcrest.generator; version=1.4.0 61 org.hamcrest.text; version=1.4.0 61 org.hamcrest; version=1.4.0 61 org.hamcrest.integration; version=1.4.0

@Tibor17
Copy link
Author

Tibor17 commented Apr 10, 2013

@mattbishop
Can you see and check the exported packages in the test above?
Did you have time to perform build time measurements?

@Tibor17
Copy link
Author

Tibor17 commented May 14, 2013

@mattbishop
Here is a test org.hamcrest.examples.osgi.ExampleWithOSGi. I used the required-bundle section which works fine. If you update the project, the activator would not fail and the test will pass.
I have got wiring exceptions if using fragment host. There is a space for bundle symbolic name in import section per package, but we cannot define two bundles per one package.

@Tibor17
Copy link
Author

Tibor17 commented May 25, 2013

@mattbishop
@scarytom
Do you have some little spare time to see my last commit?
Thx.

@mattbishop
Copy link

I can have a look next week.

On Sat, May 25, 2013 at 2:18 PM, Tibor Digana notifications@github.comwrote:

@mattbishop https://github.com/mattbishop
@scarytom https://github.com/scarytom
Do you have some little spare time to see my last commit?
Thx.


Reply to this email directly or view it on GitHubhttps://github.com//pull/32#issuecomment-18454049
.

@Tibor17
Copy link
Author

Tibor17 commented Jun 10, 2013

@mattbishop
Hi Matt,
Pls let me know when you are ready.
Thx, Tibor

@mattbishop
Copy link

I picked up your changes but I don't know what I need to do to run the test. Has someone filed a bug to move Hamcrest to maven or gradle?

@Tibor17
Copy link
Author

Tibor17 commented Jun 12, 2013

@mattbishop
I am using IDEA. I run the JUnit test org.hamcrest.examples.osgi.ExampleWithOSGi from the module hamcrest-examples. The class path should have lib/osgi/org.apache.felix.framework-4.2.1.jar as well.
I think the default project files of IDEA has other libs in path, namely lib/integration/*.*, build/*.* and project module hamcrest-integration. The user.dir is the project root.

I totally agree with you that this project could be designed in Maven with the benefit. Normally I am developing only Maven project because project-files based system is problematic like now it has happened, but that would need opening a new pull.

@mattbishop
Copy link

So I've spent about 20 minutes trying to get intellij to pick up the
dependencies, including the other modules, for examples. Everything is
hooked up, except for the Matchers class. WTH? Where is that source file?

On Wed, Jun 12, 2013 at 12:53 PM, Tibor Digana notifications@github.comwrote:

@mattbishop https://github.com/mattbishop
I am using IDEA. I run the JUnit test
org.hamcrest.examples.osgi.ExampleWithOSGi from the module
hamcrest-examples. The class path should have
lib/osgi/org.apache.felix.framework-4.2.1.jar as well.
I think the default project files of IDEA has other libs in path, namely
lib/integration/., build/. and project module hamcrest-integration.
The user.dir is the project root.

I totally agree with you that this project could be designed in Maven with
the benefit. Normally I am developing only Maven project because
project-files based system is problematic like now it has happened, but
that would need opening a new pull.


Reply to this email directly or view it on GitHubhttps://github.com//pull/32#issuecomment-19351039
.

@Tibor17
Copy link
Author

Tibor17 commented Jun 14, 2013

@mattbishop
I want to help you out if you reach me on my email on Skype "tibor.digana".
@scarytom
Is there any activity to move support the Hamcrest with Maven in the project? This would really help here especially the integration tests under the Maven.

@Tibor17
Copy link
Author

Tibor17 commented Jun 19, 2013

@scarytom
I added an integration test which is running in IDEA. I understand that some people may have a problem running it in other IDE, but Hamcrest was designed with *.iml files for IDEA.
In order to make such integration tests easy to use in whatever IDE I would prefer Hamcrest built by Maven.
If we really want to move this feature, we should launch the test as it exists now. Can you guys @mattbishop , @scarytom try to launch the test ExampleWithOSGi on your side?

@mattbishop
Copy link

I'm sorry Tibor, you're wasting my time with this IDE-based testing. Can
you please wire your OSGi examples into the ant 'examples' target and get
that working please? Right now it fails because it can't find the osgi
packages in the classpath.

On Wed, Jun 19, 2013 at 3:33 PM, Tibor Digana notifications@github.comwrote:

@scarytom https://github.com/scarytom
I added an integration test which is running in IDEA. I understand that
some people may have a problem running it in other IDE, but Hamcrest was
designed with *.iml files for IDEA.
In order to make such integration tests easy to use in whatever IDE I
would prefer Hamcrest built by Maven.
If we really want to move this feature, we should launch the test as it
exists now. Can you guys @mattbishop https://github.com/mattbishop ,
@scarytom https://github.com/scarytom try to launch the test
ExampleWithOSGi on your side?


Reply to this email directly or view it on GitHubhttps://github.com//pull/32#issuecomment-19719801
.

@Tibor17
Copy link
Author

Tibor17 commented Jun 29, 2013

@mattbishop
ant examples-tests -Dversion=123
The tests on examples-module are available.
I am very sorry for that.

@Tibor17
Copy link
Author

Tibor17 commented Jul 6, 2013

@scarytom
@mattbishop
Would you guys have time to start these tests ant examples-tests -Dversion=123? Thx.

@mattbishop
Copy link

I rebased on your latest changes Tibor and ran your ant command (not sure what version is for). The tests appear to pass.

@Tibor17
Copy link
Author

Tibor17 commented Jul 9, 2013

@mattbishop
The version -Dversion is not mine addon. It was here all the time and it is used when making a release.
@scarytom
The regex on version ([^-])+ excludes artifacts like hamcrest*-1.4-sources.jar from the OSGi test.
Do we have a pattern on version like 1.3-RC2, etc?
If yes, then I have to modify this regex.
I see in Maven Central that we are using a pattern like 1.3.RC2. So in my opinion the regex is fine.

@Tibor17
Copy link
Author

Tibor17 commented Jul 16, 2013

@mattbishop
I am glad that the tests are working now on your side as well.
What should be the next step?

@Tibor17
Copy link
Author

Tibor17 commented Aug 22, 2013

@scarytom
@mattbishop
The project is idle for few months.
Are we waiting for any unanswered questions or for the committer?
@mattbishop Is there anything you would like me to change?

@Tibor17
Copy link
Author

Tibor17 commented Feb 23, 2014

@scarytom
What's up?
You haven't reply for quite a long time. We need you in Hamcrest project. Can you join us please?
Thx, Tibor

@cvgaviao
Copy link

hey, where are the committers of this project ?? is this project dead ?

@sf105
Copy link
Member

sf105 commented May 29, 2014

Not sure what's happened to @scarytom
A couple of points. I've just raised a proposal to combine all the jars. I don't see that they're carrying their weight. Also, I'm not keen to include extra jars just for OSGI, which is still a minority need.
Would it make sense to have this as a separate linked project? Then you can commit and ship on your own timetable. We can provide references from our site.

@cvgaviao
Copy link

@sf105, I could find at least 3 different existent projects packaging Hamcrest in an OSGi compatible way, but each has chosen different paths to resolve the split package issue. The problem is that they are not portable.
And to me, the main problem is that JUnit team won't turn its project OSGi compatible until a Hamcrest official OSGi compatible jar is released. :(

@oberlies
Copy link

I'm also from the "minority" who wants to have Hamcrest as an OSGi bundle! (Is there a better way to "vote" for an issue than adding a comment?)

@sf105
Copy link
Member

sf105 commented May 30, 2014

Damn, this is confusing.

If we collapsed the jars down to one, would that help?

Could you create a project that includes both JUnit and Hamcrest in a single jar? Would that be easier to work with? Or does it need signing by the "official" project?

S

On 29 May 2014, at 20:54, Cristiano Gavião notifications@github.com wrote:

@sf105, I could find at least 3 different existent projects packaging Hamcrest in an OSGi compatible way, but each has chosen different paths to resolve the split package issue. The problem is that they are not portable.
And to me, the main problem is that JUnit team won't turn its project OSGi compatible until a Hamcrest official OSGi compatible jar is released. :(


Reply to this email directly or view it on GitHub.

@Tibor17
Copy link
Author

Tibor17 commented Jun 1, 2014

Yes the Hamcrest is dead, which i mentioned in JUnit team.
If the commiter does not come back, we will have a serious problem to provide new fixes.
Are we able to deploy to Maven Central with same gpg key as before?

@sf105 has already said the arguments.
Modularity is concept in a real development which solves the cost issues.

josephw added a commit to josephw/JavaHamcrest that referenced this pull request Dec 2, 2014
Include maven-bundle-plugin to generate the default OSGi
manifests for the core and library artifacts.
@sf105
Copy link
Member

sf105 commented Apr 7, 2016

Hoping that this is fixed with #98

@sf105 sf105 closed this Apr 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants