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

4.9 and 4.10 "junit" artifacts in Maven Central have hamcrest as dependency defined while in "junit" artifact the hamcrest classes are already included #332

Closed
Vampire opened this issue Oct 3, 2011 · 53 comments · Fixed by #421

Comments

@Vampire
Copy link

Vampire commented Oct 3, 2011

No description provided.

@Stephan202
Copy link
Contributor

On this topic: I recently pondered why both junit and junit-dep are deployed to Maven Central. Shipping a JAR with its dependencies is rather un-maven-like. In other words: for Maven, can't we just have junit, without Hamcrest classes, and dispose of the junit-dep module?

@Vampire
Copy link
Author

Vampire commented Oct 3, 2011

I think the same way. I just thought the junit guys have a reason for doing so.

@Vampire Vampire closed this as completed Oct 3, 2011
@Vampire Vampire reopened this Oct 3, 2011
@redsolo
Copy link

redsolo commented Oct 4, 2011

i agree with Stephan202. I think the main junit component should only contain the JUnit classes and nothing else. The hamcrest dependency is brought in anyway due to the "compile" scope. I understand the need for a junit-dep for non-maven users, but for maven where it is easy to exclude hamcrest or add a newer version of hamcrest there is no need for a junit-dep pkg.

@marcphilipp
Copy link
Member

+1

@stefanbirkner
Copy link
Contributor

+1

@Tibor17
Copy link
Contributor

Tibor17 commented Oct 25, 2011

+1
Totally agree.

The Maven artifacts must NOT have other than own classes. In ANT we can use the junit-4.10.jar with such of mixture with Harcrest, but not for the Maven. It really looks like UnMaven style in the Maven artifacts, nothing but a mess in the JAR.

Once any JAR has embedded Hamcrest classes, Maven users are unable to remove the Hamcrest' classes via Maven from java classpath. Thus the Maven dependencies exclusions:

org.hamcrest hamcrest-core 1.1

is not the solution for the Maven users.

Solution in Maven is a separate artifact with the only JUnit classes in junit related JAR, and other un-junit jars in separate Maven artifacts with separate POM.

@dsaff
Copy link
Member

dsaff commented Oct 26, 2011

I'm completely open to getting JUnit's Maven presence more inline with Maven standards. I'm still not sure who did the initial upload of JUnit 4.4 to Maven, but I've simply been maintaining that configuration ever since, because I don't know enough about Maven to know what the optimal solution is. For more information, see gh-309. All this needs is a volunteer to gather community consensus and issue a pull request. Thanks.

@martinburger
Copy link

I think you should completely remove dependency hamcrest-core from pom-template.xml. AFAIK it does not make sense to include hamcrest's core classes and add it at dependency at the same time.

However, I only had a quick look at it and did not investigate this issue further.

@Stephan202
Copy link
Contributor

  • Martin, see my reply at junit-dep and maven: wrong dependency scope #349.
  • David, I'm open to looking into this, perhaps this weekend. It would be nice to dispose of junit:junit-dep and fix junit:junit; I found that having two artifacts with different identifiers around causes some issues, since Maven will consider them distinct and hence potentially puts both on the classpath (this can happen when a dependency depends on JUnit with scope compile or runtime).

@martinburger
Copy link

I think to dispose of junit-dep and fix junit would be the best thing to do.

@Tibor17
Copy link
Contributor

Tibor17 commented Nov 29, 2011

I agree with martin and stephan.
It makes no sense to maintain two libraries.
This would improve the clarity then.

@redsolo
Copy link

redsolo commented Nov 29, 2011

+1 for martin

@xamde
Copy link

xamde commented Dec 7, 2011

+1

1 similar comment
@Godin
Copy link
Contributor

Godin commented Jan 5, 2012

+1

@kolstae
Copy link

kolstae commented Feb 1, 2012

+1 for martin

@torarnek
Copy link

torarnek commented Feb 2, 2012

It would have been great to have junit without the Hamcrest packged within, while having dep on hamcrest-core I think is ok, as that can be configured in maven.

For us to use Hamcrest 1.2, we are currently maintaing our own junit maven package...

@Vampire
Copy link
Author

Vampire commented Feb 2, 2012

You don't need to torarnek, just use junit-dep as dependency for now. It is junit without the hamcrest files.

@torarnek
Copy link

torarnek commented Feb 2, 2012

Yeah, but then I would need to do exclude in all the dependencies that depend on junit... and that is quite a few in our project. JUnit is a popular maven artifact! ;)

@Vampire
Copy link
Author

Vampire commented Feb 8, 2012

No you wouldn't. Just don't use the dependency-level exclude (http://ant.apache.org/ivy/history/latest-milestone/ivyfile/artifact-exclude.html) but the dependencies-level exclude (http://ant.apache.org/ivy/history/latest-milestone/ivyfile/exclude.html) which serves exactly the need you have. You exclude junit once and it is excluded for the whole module.

@Tibor17
Copy link
Contributor

Tibor17 commented Feb 8, 2012

The problem is that the design of deployment to maven repo in JUnit is wrong.

Even if you use Ivy, in the Maven this must work normally but it will never work because the Maven is not able to exclude the built-in classes in junit-x.x.x.jar' by a simple exclude because junit and hamcrest files are in one jar file.

As a best solution would be to dispose junit:junit-dep, and fix junit:junit to have same jar-content and Maven dependency to Hamcrest as it is in now in junit:junit-dep.
Then it will be as it should.

@Vampire
Copy link
Author

Vampire commented Feb 9, 2012

Of course Tibor17, that is not the questions, except maybe that junit-dep should maybe not be disposed but configured to forward to junit after junit is fixed, this is possible in the POM.

Ivy also cannot exclude classes from inside a JAR.

The point between torarnek and me was about a work-around as long as the artifacts are not fixed properly.

For this you can add junit-dep and hamcrest as dependencies and exclude junit module-wide so that junit is not a transitive dependency, even if other dependencies depend on it transitively.

I guess a similar setup is also possible with Maven, but I'm no Maven guy so I cannot tell.

@Tibor17
Copy link
Contributor

Tibor17 commented Feb 9, 2012

I do not accept any workarounds anywhere.
IMHO the JUnit must have serious Maven style artifact. The other which is not Maven style should go away.

I agree with Stephan202 in the early beginning.

@Vampire
Copy link
Author

Vampire commented Feb 9, 2012

You don't have to use the workaround. But there are people out there that want to use junit with a newer Hamcrest library and need this workaround like torarnek.

Why are you so abresive? Noone forces you to do so.

@Tibor17
Copy link
Contributor

Tibor17 commented Feb 9, 2012

take a look up, what the people say. One extra non-Maven artifact is good for nothing in Maven repo.
This should not be a problem in Ant script to dispose deployment on junit-dep +fixes in junit:junit.

@torarnek
Copy link

torarnek commented Feb 9, 2012

Hey Vampire,

The links you sent me are for Ivy, while what we use is Maven. And to my best knowledge, there is no transitive exclusions of artifacts in Maven. JUnit has to be excluded in each artifact that depends on it.

My recommendation is to solve this problem with the Maven artifact, once and for all, by removing the hamcrest classes from the jar, and keep the hamcrest-core dependency. In that way things will not break.

This would have been a great improvement, let's say for the next version. :)

@Vampire
Copy link
Author

Vampire commented Feb 9, 2012

Ah, ok torarnek.
I thought Maven should have a similar possibility.
But it is an open issue: http://jira.codehaus.org/browse/MNG-3196

@Tibor17 I don't know what you are talking about. What non-Maven artifact are you talking about? I didn't say anything about adding any additional artifact. I just provided a workaround for Ivy users to exclude the junit artifact from the transitive dependencies and use junit-dep + hamcrest explicitly instead.

@Tibor17
Copy link
Contributor

Tibor17 commented Feb 9, 2012

non-Maven style artifact is the current junit:junit
See Stephan's statement
"Shipping a JAR with its dependencies is rather un-maven-like."

@marcphilipp
Copy link
Member

Pull request #421 has been merged. It changed the following:

  1. junit:junit now does not contain Hamcrest classes anymore but declares a dependency to hamcrest-core.
  2. junit:junit-dep relocates to junit:junit.

I have deployed both junit:junit and junit:junit-dep to Sonatype's snapshot repository.

Could some of you please give me a hand and try it out?

@kolstae
Copy link

kolstae commented Apr 24, 2012

Sweet!

I'll check it out asap

@kolstae
Copy link

kolstae commented Apr 26, 2012

Works like a charm :)

Thanks!

@marcphilipp
Copy link
Member

@kolstae Thanks for testing!

Has anyone else tried?

@Vampire
Copy link
Author

Vampire commented Apr 30, 2012

Works like a charm, I wish I had this with the 4.10 artifacts already :-)

@marcphilipp
Copy link
Member

Thanks for testing!

It would be great if we could release 4.11 with support for Hamcrest 1.3. Could you test #404 even if there's no Maven snapshot? Or, do we have to deploy a snapshot for that purpose?

@marcphilipp
Copy link
Member

In the meantime, #404 has been merged and another JUnit 4.11 snapshot has been deployed to Maven central feat. Maven-like dependencies to Hamcrest 1.3.

@Vampire I think this issue can be closed, or am I missing something?

@Vampire
Copy link
Author

Vampire commented Aug 21, 2012

Actually this issues was / is about the 4.9 and 4.10 artifacts declaring wrong dependencies to classes they already ship. This is not less true after the changes done, though I really appreciate it being done correctly now. If you refuse to fix the wrong dependency declaration for 4.9 and 4.10 in Maven Central, I guess this issue can be closed indeed.

@marcphilipp
Copy link
Member

@dsaff Am I right that we won't fix this in already released versions?
If so, please close this issue.

@Vampire
Copy link
Author

Vampire commented Aug 22, 2012

Just to clarify, it would only mean to remove the dependency from the pom file. No change to the actual artifacts.

@marcphilipp
Copy link
Member

In Maven-speak isn't the POM file also considered an artifact?

@Vampire
Copy link
Author

Vampire commented Aug 22, 2012

I'm not a native Maven speaker, but let me rephrase then, No change to the business artifacts is necessary.
With the current setup it is not really helpful as you end with having the classes twice, in junit.jar and in hamcrest.jar.

@marcphilipp
Copy link
Member

I'm not sure that all users would get the updated POM file because Maven will tend to cache released versions. Thus, I would suggest leaving the 4.9 and 4.10 artifacts/POMs as is and release 4.11 soon.

@dsaff What do you think?

@dsaff
Copy link
Member

dsaff commented Sep 4, 2012

I agree with both points.

@marcphilipp, having seen this epic struggle through so far, would you be interested in publicizing a general beta for 4.11, so we can receive community-wide feedback?

@marcphilipp
Copy link
Member

Sure, I would be glad to do so. What exactly needs to be done? Build a version called 4.11b1 and upload it to GitHub and Maven Central? Do we still upload stuff to Sourceforge? In addition I would send an announcement to the mailing list.

@dsaff
Copy link
Member

dsaff commented Sep 5, 2012

That's most of it. It's also fairly useful to assemble and announce some release notes, so people have an idea why they might be excited about the version, and also what to push on for bugs. That's often the biggest effort of a version. Do you have time for that?

We have, traditionally, still been pushing major annual releases to sourceforge, but not bothering with anything more interim.

@marcphilipp
Copy link
Member

Ok, makes perfect sense. Actually, as it looks right now, I won't have enough time for that within the next two weeks. Would afterwards be too late?

@dsaff
Copy link
Member

dsaff commented Sep 6, 2012

That's still faster than I'm likely to get to it. :-) Let me know if it starts looking like it will be significantly longer. Thanks!

@marcphilipp
Copy link
Member

I will let you know. Will you be able to review the release notes I come up with?

@dsaff
Copy link
Member

dsaff commented Sep 6, 2012

Absolutely.

@Tibor17
Copy link
Contributor

Tibor17 commented Sep 21, 2012

@marcphilipp the maven style for beta versions is 4.11-beta-01

@marcphilipp
Copy link
Member

@dsaff I started working on the release notes. Current (very early) state is availabled in this gist.

@marcphilipp
Copy link
Member

I'm closing this issue since we decided not to fix 4.9 or 4.10 but rather release 4.11 as soon as possible. Let's track everything that's release-related in #512.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.