search

hamcrest / JavaHamcrest

Java (and original) version of Hamcrest
http://hamcrest.org/
BSD 3-Clause "New" or "Revised" License
2.11k stars 379 forks source link

Release Version 2.1 On Maven #224

Closed ankurpathak closed 5 years ago

ankurpathak commented 5 years ago

I can see the last artificat of Hamcrest released on maven central was back in 2015. But the latest master has many updates which are not availabe on that maven build. For example empty matcher for Map. So please release the binaries for latest master on maven central. So we can consume it in our projects. Also the latest master and binaries on maven central should be in sync.

tumbarumba commented 5 years ago

I think this is a great idea. It's been far too long since releases. I plan to try and do this myself over the next couple of weeks.

@hamcrest/hamcrest-java-devs: looking at the shiny new Hamcrest Distributables page, there are 5 jars mentioned. I can see the code for hamcrest-core.jar and hamcrest-library.jar, but I don't see any code for hamcrest-integration.jar or hamcrest-generator.jar (I understand hamcrest-all.jar is just the combination of all the other jars). Does anyone have any idea where this missing code might be? Is it still relevant?

sf105 commented 5 years ago

I tried to collapse everything into one jar. We don't need the generator any more as we've stopped doing the code generation for the Matchers class. There's still a decision about whether to have separate jars for the test framework integrations as they're outside the JDK. Might make life easier for dealing with junit 4/5 and other test frameworks.

tumbarumba commented 5 years ago

Thanks @sf105. The test frame integration classes do not appear to be checked into this github repository. Do you have them somewhere else?

tumbarumba commented 5 years ago

I found the java files on the old Google Code repo: https://code.google.com/archive/p/hamcrest/source/default/source

I'll see if I can import them into GitHub this weekend.

tumbarumba commented 5 years ago

@hamcrest/hamcrest-java-devs: I've just landed #228, which is preparing the Gradle build for reproducing the various jars as per the 1.3 release. Included in this change: I've set the version to 1.4-SNAPSHOT.

Are there any objections to calling the next version 1.4? If so, what should the next version be?

ankurpathak commented 5 years ago

@tumbarumb Using 1.4 for new version will create same confusion which to use 1.3.x.x or 2.x.x.x. So lets skip these 1 and 2. And start a new life for hamcrest with 3.x.x.x or 4.x.x.x and regular releases on maven. So their wont be any confusion remains. Since human by their greedy nature have the tendncy to use latest and greatest one. And using 4.x.x.x skipping 3.x.x.x will mark new life for hamcrest project and will also match jdk style of rebrading from 1.x to x. In our case we can justify it as 1.4 as 4.x in our case and will remove each confusion.

tumbarumba commented 5 years ago

it's worth talking about exactly what artefacts we want to publish in the next release. I've just created a new pull request (#229) to bring back hamcrest-integration. As a result of this pull request, there will be 5 artifacts:

One question I have is whether we should keep the existing 4 artefacts (core, library, integration and all), or remove core and library, replaced by java-hamcrest. My personal preference: keep the existing jars, give people warning that some of them will disappear in a future version. What do other people think?

When (or if) we do remove hamcrest-core and hamcrest-library, we can change the dependency on hamcrest-integration to java-hamcrest. hamcrest-all essentially remains the same.

sf105 commented 5 years ago

I'd like to collapse all the artefacts. It turns out there's really no point to the division, it's just confusing and complicated. There should be exactly one jar for all the matchers with no dependencies outside the JVM (except junit for testing it). The confusion with java-hamcrest is that was a failed attempt at major new version.

I recommend that we do minor version changes into hamcrest-all and deprecate the other jars. That we deprecate java-hamcrest back to hamcrest-all. That, at the next major change, we deprecate hamcrest-all in favour of a hamcrest artefact.

tumbarumba commented 5 years ago

I really like the idea of having an artefact called hamcrest (where we merge hamcrest-core and hamcrest-library). java-hamcrest didn't seem quite right to me.

I'm not sure about hamcrest-integration, though. I don't use it myself, so I'm not sure how popular it is. However, if we include it, it will add optional dependencies to jmock and easymock. Is it feasible to not release any new versions of it? Given that we already have a v2 artefact (a breaking change under semver), the break might be "no more integration". In that case, we can exclude the integration code from hamcrest-all, and it won't need to declare any other dependencies.

Thoughts?

tumbarumba commented 5 years ago

I've tweaked PR #229 with a new proposal: 2 artifacts: hamcrest and hamcrest-integration. We only published hamcrest, though. More details on the PR.

tumbarumba commented 5 years ago

I've been making a bit of progress on the release, but there are still some things to do:

I have created a request on OSSRH to allow me to publish the artifact (https://issues.sonatype.org/browse/OSSRH-44320). @sf105, can you please comment on that JIRA to authorise that change?

What have I missed. Anything else important?

tumbarumba commented 5 years ago

I've just closed issue #183, as I think the new release should resolve all the concerns raised there.

As a summary, here's what I'm thinking about at the moment for the next release:

geekybaiyi commented 5 years ago

sounds good, any ETA for the release ?

tumbarumba commented 5 years ago

Very Soon Now™

I'm actually wondering if we should publish a release candidate (RC1) build first. Give people a chance to kick the tyres.

sf105 commented 5 years ago

Yes. That's a good idea.

tumbarumba commented 5 years ago

@sf105, @npryce or @scarytom : could one of you please take a leisurely stroll along to https://issues.sonatype.org/browse/OSSRH-44320 and put a comment along the lines of "tumbarumba has permission to publish to org.hamcrest" (similar to what happened at https://issues.sonatype.org/browse/MVNCENTRAL-554).

If I can get permissions in time, I'll try and put a new RC build out this weekend.

npryce commented 5 years ago

Done!

On Fri, 23 Nov 2018 at 18:16, Joe Schmetzer notifications@github.com wrote:

@sf105 https://github.com/sf105 or @npryce https://github.com/npryce : could one of you please take a leisurely stroll along to https://issues.sonatype.org/browse/OSSRH-44320 and put a comment along the lines of "tumbarumba has permission to publish to org.hamcrest" (similar to what happened at https://issues.sonatype.org/browse/MVNCENTRAL-554).

If I can get permissions in time, I'll try and put a new RC build out this weekend.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/hamcrest/JavaHamcrest/issues/224#issuecomment-441298533, or mute the thread https://github.com/notifications/unsubscribe-auth/AADbm2JVxWHvo_2sIaDXPccjk5f0sHQLks5uyDuBgaJpZM4YNxB2 .

tumbarumba commented 5 years ago

Thanks Nat!

I've just pushed out hamcrest-2.1-rc1.jar. I doesn't look like Maven Central has synced it yet. I'll send out a note to the mailing lists when it comes through.

tumbarumba commented 5 years ago

So... first experience report of using 2.1-rc1...

I tried it on my project at work. It's an Apache Flink pipeline, with the tests using (among other things) JUnit 4.12, Mockito, Spock, and Flink Spector. What I found was that in this particular example, the hamcrest-core-1.3.jar was still pulled in as a dependency through a lot of different transitive paths. It turns out that a lot of stuff depends transitively on JUnit 4.12 (which transitively depends upon hamcrest-core), Mockito depends directly on hamcrest-core. I ended up having to do a lot of tweaks to my Gradle dependencies to get it to work. For example, my old test dependencies looked like this:

dependencies {
    testCompile 'io.flinkspector:flinkspector-datastream_2.11:0.8.4',
            'junit:junit:4.12',
            'org.hamcrest:hamcrest-library:1.3',
            'org.mockito:mockito-core:1.10.19',
            'org.spockframework:spock-core:1.1-groovy-2.4'
}

After upgrading Hamcrest, and adding in the excludes to avoid adding hamcrest-core-1.3.jar to the classpath, I had to do this:

dependencies {
    testcompile 'org.hamcrest:hamcrest:2.1-rc1'
    testCompile('io.flinkspector:flinkspector-datastream_2.11:0.8.4') {
        exclude group: 'org.hamcrest'
    }
    testCompile('junit:junit:4.12') {
        exclude group: 'org.hamcrest'
    }
    testCompile('org.mockito:mockito-core:1.10.19')  {
        exclude group: 'org.hamcrest'
    }
    testCompile('org.spockframework:spock-core:1.1-groovy-2.4')  {
        exclude group: 'org.hamcrest'
    }
}

Needless to say, I think is is probably very error prone, and don't think this looks very nice. I hate to think what it would look like in Maven.

I wonder if there's a more seamless way to allow someone to upgrade Hamcrest, and allow other third-party libraries to automatically get the new version, too. The problem at the moment is that I've given the 2.1-rc1 library a different name, so it doesn't actually get involved in version conflict resolution with the 1.3 jars.

One possible solution that comes to mind: we publish new poms for hamcrest-core and hamcrest-library for 2.1. If we make these poms just declare a dependency upon hamcrest (with no jar otherwise), I think everything should upgrade without problems (given that there should be no API incompatibility). That way, I can just declare a dependency upon 'org.hamcrest:hamcrest-library:2.1' in my project, and it will just pick up the new jar. I think we still want to get away from even publishing anything hamcrest-core and hamcrest-library, but this seems like a reasonable stepwise migration.

What do people think?

garretwilson commented 5 years ago

So I take it the Hamcrest people aren't coordinating with the JUnit people? If JUnit depends on a Hamcrest library, and Hamcrest tries to refactor the packaging completely independently from what the JUnit people do, it will just be a mess.

Here is an idea, though: Maven will take the latest version of any transitive dependency declared in the POM. Currently I just want a newer version of hamcrest-library (which is the one I use) to get the latest functionality. JUnit depends on hamcrest-core. The Hamcrest people want to combine all their JARs into a single dependency going forward. So why not just name this new combined Hamcrest library hamcrest-core?

That way I can switch from using hamcrest-library to (the new) hamcrest core, and it will automatically replace (and "upgrade") the one referenced by JUnit. I won't have to do any other exclusions or other surgery. (This assumes you're careful and make sure that the new combined hamcrest-core is compatible with Junit.)

tumbarumba commented 5 years ago

Thanks @garretwilson.

The JUnit 4 team chose to pick up hamcrest-core as a dependency, and talked about using assertThat(thing, matcher) as the recommended way of writing assertions. The JUnit 5 team changed directions, and has completely dropped the dependency on Hamcrest, or any other matcher library. These are now orthogonal concerns (which is a good thing, I think - you can easily mix and match the matcher library used, depending upon needs).

For the Java Hamcrest 2.1 release, I'm proposing we completely remove hamcrest-core and hamcrest-library as published artefacts. These have both been merged into a single artefact called hamcrest (e.g. hamcrest-2.1.jar). Naming the combined jar hamcrest-core might simplify the upgrade. I'm trying to think of any technical problems it might have, but I can't think of any big things up front, so this might be a good option.

However, the intent was to make a clear statement that the packaging had changed, and giving it a different name was a way to make this explicit.

Looking back at the 2.1-rc1 release again, where there is no hamcrest-core or hamcrest-library, I found a better way to exclude the old 1.3 artefacts in my Gradle script. Just do this:

configurations.all {
    exclude group: 'org.hamcrest', module: 'hamcrest-core'
    exclude group: 'org.hamcrest', module: 'hamcrest-library'
}

That way, I don't have to butcher all the other dependency declarations.

I don't have any handy Maven pom based projects to try, though I'm curious to hear about other peoples' experiences. What about other build tools? Is it very hard to make the change?

sf105 commented 5 years ago

interesting ideas. Could we upgrade hamcrest-core and hamcrest-library just to have a dependency on the new combined artefact? I think that's better than using hamcrest-core because that doesn't explain what happened to library.

tumbarumba commented 5 years ago

It would be easy enough to create a pom-only artefact for hamcrest-core and hamcrest-library, each with a dependency on hamcrest. I think it will be a fairly trivial change to the Gradle build.

Unless someone is aware of any major problems with this approach, I can try this in an rc2 release in the next couple of days.

garretwilson commented 5 years ago

However, the intent was to make a clear statement that the packaging had changed, and giving it a different name was a way to make this explicit.

Yes, I agree this is a great approach in theory. The existing JUnit dependency on hamcrest-core in a pragmatic concern, though; it throws a wrench in the theory. 😄

The ideal solution in that case is to contact the JUnit people and have them release one more version of JUnit 4 with a dependency on your new Hamcrest-with-a-different-name. Any chance of that happening?

With @sf105 's idea, it seems you would doing both: issuing a new Hamcrest naming convention, and releasing drop-in "aliases" that would override JUnit's transitive dependencies. I suppose that would work, but if a "clear statement" is what you want… wouldn't this just cause confusion for those who haven't read this thread—more confusion than just making hamcrest-core the new combined JAR? I mean, I understand what's going on. But I predict you'll get a bunch of bug requests and Stack Overflow questions saying, "Which one of these three JARS do I choose now?" The answer would be "any of them", at which point the user would say, "then why do we have three if they are the same"?

If the Hamcrest response is that "we can document all this on some page", history indicates that this isn't likely to happen (see #183). And after all, wouldn't be easier to document the new contents of hamcrest-core than trying to explain three libraries?

Lastly, aren't most people likely to use Hamcrest with JUnit, in which case they'll need to use the "alias" hamcrest-core? How would they know that without reading this thread?

My purpose here is not to argue; rather I'm just trying to lay out all the options and their consequences so you can make an informed decision.

tumbarumba commented 5 years ago

Thanks @garretwilson, I appreciate you helping us clarify the various points of view. Let me try to summarise the various positions, as I understand them right now.

What have I missed? Are there other options?

Option 1

Merge hamcrest-core and hamcrest-library into hamcrest

We create a new jar called hamcrest-2.1.jar, which has all the code previously in core and library. We do not publish jars for the old core and library, but we do publish a pom for each of those, which just has a single dependency on the new merged jar (i.e. hamcrest-core-2.1.pom and hamcrest-library-2.1.pom).

Option 1 Implications

Option 2

Merge hamcrest-library into hamcrest-core

We merge all the code from library into core. We publish a single jar called hamcrest-core-2.1.jar. We also publish a pom-only hamcrest-library-2.1.pom, which contains a dependency upon hamcrest-core.

Option 2 Implications

garretwilson commented 5 years ago

Option 2 Implications … Users will only have to change the version on whatever jar they are depending upon (e.g. 1.3 -> 2.1)

Actually it's not that simple. To avoid manually excluding JUnit dependencies, a user would have to know to switch to hamcrest-core.

Take my situation, for instance. Our parent POM uses junit:junit:4.12 and org.hamcrest:hamcrest-library:1.3. (It's been ages since I set this up, so I don't remember the details, but I remember researching which libraries to use—I didn't just blindly guess, for what it's worth.) So I couldn't just upgrade the version on hamcrest-library, I'd still have to manually exclude hamcrest-core from junit, or my build would wind up with two versions of hamcrest-core, wouldn't it?! In any case, I'm pretty sure a transitive hamcrest-core dependency of the new org.hamcrest:hamcrest-library:2.1 wouldn't override the transitive hamcrest-core dependency of junit:junit:4.12. So basically I'd still need to do the manual exclusion, or switch to explicitly including org:hamcrest:hamcrest-core:2.1.

npryce commented 5 years ago

I don’t know if either of you have seen this before, but here’s our previous attempt to decouple JUnit and Hamcrest: https://github.com/hamcrest/JavaHamcrest/issues/92

This was before JUnit 5, which was a clean slate and doesn’t depend on Hamcrest at all.

On Mon, 26 Nov 2018 at 19:16, Garret Wilson notifications@github.com wrote:

Option 2 Implications … Users will only have to change the version on whatever jar they are depending upon (e.g. 1.3 -> 2.1)

Actually it's not that simple. To avoid manually excluding JUnit dependencies, a user would have to know to switch to hamcrest-core.

Take my situation, for instance. Our parent pom uses junit:junit:4.12 and org.hamcrest:hamcrest-library:1.3. (It's been ages since I set this up, so I don't remember the details, but I remember researching which libraries to use—I didn't just blindly guess, for what it's worth.) So I couldn't just upgrade the version on hamcrest-library, I'd still have to manually exclude hamcrest-core from junit, or my build would wind up with two versions of hamcrest-core, wouldn't it?! In any case, I'm pretty sure a transitive hamcrest-core dependency of the new org.hamcrest:hamcrest-library:2.1 wouldn't override the transitive hamcrest-core dependency of junit:junit:4.12. So basically I'd still need to do the manual exclusion, or switch to explicitly including org:hamcrest:hamcrest-core:2.1.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hamcrest/JavaHamcrest/issues/224#issuecomment-441761980, or mute the thread https://github.com/notifications/unsubscribe-auth/AADbm7LSTIkxQQ8Rhxpq7p52DErcPKHsks5uzD4ngaJpZM4YNxB2 .

garretwilson commented 5 years ago

Maybe we shouldn't try to do too many things at one time.

  1. I think the biggest thing is getting out a new version with the latest improvements. I know that's all I care about for now. I just want the regex matchers, for example. Put that out and keep the same package divisions. If I have to manually indicate hamcrest-core and hamcrest-library in my POM for now, fine.
  2. Coordinate with the JUnit people (are they the same people as the Hamcrest people?) and have them release a new JUnit 4 version that uses the new Hamcrest version. Then we're back to where we are right now, except that we have an updated Hamcrest that isn't years old.
  3. Now that we've bought some time, coordinate with the JUnit people more to release a version of JUnit 4. that doesn't depend on Hamcrest, at the same time you release a new Hamcrest bundling. That way you skip the whole manual exclusion and multiple JAR confusion altogether.
tumbarumba commented 5 years ago

@npryce ooh, I hadn't see that! Interesting bit of history, though I think it's pretty much moot now that JUnit 5 is out.

@garretwilson I don't think we need to ask the JUnit folks to release a new version of JUnit 4. As long as Hamcrest maintains API compatibility (which I believe Hamcrest 2.1 does), then you can upgrade the version of Hamcrest used by JUnit 4 just by explicitly declaring a more recent version of hamcrest-core.

Dependency resolution works like this, AFAIK:

This means: as long as you declare a directly dependency on hamcrest-core-2.1.jar and hamcrest-library-2.1.jar, then JUnit 4 code will end up working with the latest version.

Of course, this breaks if someone is using the junit-dep artifact, where the Hamcrest classes are actually embedded inside that jar. However, the JUnit folks stopped releasing that particular monstrosity at junit-dep-4.11.jar.

tumbarumba commented 5 years ago

Just to clarify a bit on the behaviour of Option 1: In cases where a project imports both 'org.hamcrest:hamcrest:2.1' and 'junit:junit:4.12', then both hamcrest-2.1.jar and hamcrest-core-1.3.jar will end up in the classpath. because the artifactIds do not match (hamcrest vs hamcrest-core), and so version conflict resolution will not kick in. This can be avoided by specifying an explicit dependency on 'org.hamcrest:hamcrest-core:2.1' (this is the pom-only artifact - see PR #235)

Option 2 does not have this quirk in behaviour.

sf105 commented 5 years ago

I don't think we should mess with -core and -integration, rather we should just let them expire. If we make them pom-only, both depending on the new hamcrest, does that do all consistency stuff we need?

tumbarumba commented 5 years ago

I believe so. Any existing user of Hamcrest can just upgrade their existing dependency to the latest version, and the build tool will transitively pull in the new artifact.

I have a pull request out to create and publish the pom-only artifacts (#235). I'll merge that tonight, and publish a new release candidate to test out (2.1-rc2)

tumbarumba commented 5 years ago

Hamcrest version 2.1-rc2 has been published, hopefully it will be in Maven Central soon.

tumbarumba commented 5 years ago

The upgrade seemed to work well for me. I tried one of my projects that was using both JUnit 4 and Hamcrest. I changed the version of hamcrest-library as follows:

dependencies {
    testImplementation 'junit:junit:4.12'
    testImplementation 'org.hamcrest:hamcrest-library:2.1-rc2'
}

Here's what the dependency resolution looked like:

> ./gradlew dependencies --configuration testRuntimeClasspath
...
testRuntimeClasspath - Runtime classpath of source set 'test'.
+--- junit:junit:4.12
|    \--- org.hamcrest:hamcrest-core:1.3 -> 2.1-rc2
|         \--- org.hamcrest:hamcrest:2.1-rc2
\--- org.hamcrest:hamcrest-library:2.1-rc2
     \--- org.hamcrest:hamcrest-core:2.1-rc2 (*)

The tests still passed, too :heavy_check_mark:

tumbarumba commented 5 years ago

One thing I'm very interested in confirming: does the Java 9 module tweak in the manifest work? I'm not yet a heavy user of Java 9, so I don't have any handy way of quickly checking.

garretwilson commented 5 years ago

I tried this out with Maven. First, here is my dependency tree beforehand:

org.example:foobar:pom:1.2.3
+- junit:junit:jar:4.12:test
|  \- org.hamcrest:hamcrest-core:jar:1.3:test
+- org.hamcrest:hamcrest-library:jar:1.3:test

Here is my POM:

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.12</version>
  <scope>test</scope>
</dependency>

<dependency>
  <groupId>org.hamcrest</groupId>
  <artifactId>hamcrest-library</artifactId>
  <version>1.3</version>
  <scope>test</scope>
</dependency>

Based upon what was written above I changed that to the following as the first test:

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.12</version>
  <scope>test</scope>
</dependency>

<dependency>
  <groupId>org.hamcrest</groupId>
  <artifactId>hamcrest-library</artifactId>
  <version>2.1-rc2</version>
  <scope>test</scope>
</dependency>

My Maven build (just a simple mvn dependency:tree) broke:

[ERROR] Failed to execute goal …: Could not resolve dependencies …: Could not find artifact
org.hamcrest:hamcrest-library:jar:2.1-rc2 in central (https://repo.maven.apache.org/maven2)

Apparently now I have to include hamcrest-library as a <type>pom</type>, because you didn't publish JARs. So I made the following change:

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.12</version>
  <type>pom</type>
  <scope>test</scope>
</dependency>

<dependency>
  <groupId>org.hamcrest</groupId>
  <artifactId>hamcrest-library</artifactId>
  <version>1.3</version>
  <type>pom</type>
  <scope>test</scope>
</dependency>

That gives me the following dependency tree:

org.example:foobar:pom:1.2.3
+- junit:junit:jar:4.12:test
|  \- org.hamcrest:hamcrest-core:jar:1.3:test
+- org.hamcrest:hamcrest-library:pom:2.1-rc2:test

Obviously that's not what I want. I would have to do manual exclusions, or use hamcrest-core, like this:

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.12</version>
  <type>pom</type>
  <scope>test</scope>
</dependency>

<dependency>
  <groupId>org.hamcrest</groupId>
  <artifactId>hamcrest-library</artifactId>
  <version>1.3</version>
  <type>pom</type>
  <scope>test</scope>
</dependency>

That gives me this dependency tree:

org.example:foobar:pom:1.2.3
+- junit:junit:jar:4.12:test
|  \- org.hamcrest:hamcrest-core:jar:1.3:test
+- org.hamcrest:hamcrest-core:pom:2.1-rc2:test
   \- org.hamcrest:hamcrest:jar:2.1-rc2:test

That isn't what I want, either. I think the moral of this (chapter of the) story is that you can't override a "jar" type with a "pom" type artifact, even with the same group ID and artifact ID—Maven considers them different things.

So to even get close to what we're looking for here, I think you'll have to release "jar" versions of the "alias" libraries.

tumbarumba commented 5 years ago

Thanks for the detailed investigation, @garretwilson, that's really helpful.

So, in summary, 2.1-rc2:

One option to resolve this: continue to publish empty jars for hamcrest-core-2.1.jar and hamcrest-library-2.1.jar, each with a dependency on hamcrest-2.1.jar.

I can test this out, but I wonder if there are other options available? Better ways of configuring the pom, perhaps?

monperrus commented 5 years ago

For the record: we always test against the latest versions of all our dependencies using:

 mvn versions:use-latest-versions -DallowSnapshots=true

Our build is broken, because use-latest-versions recognizes a new version hamcrest-core:2.1-rc2 on Central (while there is actually no jar).

What solution would you recommend?

tumbarumba commented 5 years ago

I've just created PR #236 to build empty jars for hamcrest-core and hamcrest-library. I believe that this should resolve this particular issue. If no one objects, I'll look to creating 2.1-rc3 later today or tomorrow.

tumbarumba commented 5 years ago

I've just release version 2.1-rc3 to resolve the problem with maven transitive dependency resolution.

I'd be interested in hearing feedback from folks who have tried this in Gradle, Maven, or any other dependency management tool out there.

monperrus commented 5 years ago

Fixed the problem for me, thanks a lot.

garretwilson commented 5 years ago

I tried the new JAR version 2.1-rc3. Remember that this is my dependency tree beforehand:

org.example:foobar:pom:1.2.3
+- junit:junit:jar:4.12:test
|  \- org.hamcrest:hamcrest-core:jar:1.3:test
+- org.hamcrest:hamcrest-library:jar:1.3:test

Here is my POM:

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.12</version>
  <scope>test</scope>
</dependency>

<dependency>
  <groupId>org.hamcrest</groupId>
  <artifactId>hamcrest-library</artifactId>
  <version>1.3</version>
  <scope>test</scope>
</dependency>

First I tried just the new hamcrest-library:

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.12</version>
  <scope>test</scope>
</dependency>

<dependency>
  <groupId>org.hamcrest</groupId>
  <artifactId>hamcrest-library</artifactId>
  <version>2.1-rc3</version>
  <scope>test</scope>
</dependency>

As expected this updated hamcrest-library but not the transitive hamcrest-core:

org.example:foobar:pom:1.2.3
+- junit:junit:jar:4.12:test
|  \- org.hamcrest:hamcrest-core:jar:1.3:test
+- org.hamcrest:hamcrest-library:jar:2.1-rc3:test

So I changed to hamcrest-core:

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.12</version>
  <scope>test</scope>
</dependency>

<dependency>
  <groupId>org.hamcrest</groupId>
  <artifactId>hamcrest-core</artifactId>
  <version>2.1-rc3</version>
  <scope>test</scope>
</dependency>

As expected (and desired), this replaced the hamcrest-core that JUnit transitively brings in:

org.example:foobar:pom:1.2.3
+- junit:junit:jar:4.12:test
+- org.hamcrest:hamcrest-core:jar:2.1-rc3:test
   \- org.hamcrest:hamcrest:jar:2.1-rc3:test

I personally think some users will be confused by this whole process, and it's a bit of a kludge to have this "empty" intermediate hamcrest-core. If I were doing it myself, I would either put everything in hamcrest-core, or keep things split out into hamcrest-core and hamcrest-library (understanding that I'd have to declare two things in my POM in the latter scenario for this to work with an old JUnit).

Nevertheless there are tradeoffs with all these approaches, and the current approach works reasonably well. But you'll need to make this very clear to users in a prominent documentation page or there will be confusion:

tumbarumba commented 5 years ago

I've done a bit of reading about Maven dependency resolution and conflict handling. This was useful: https://cwiki.apache.org/confluence/display/MAVENOLD/Dependency+Mediation+and+Conflict+Resolution

The interesting thing (to me, at least) was the different strategies that Gradle and Maven use for version conflicts by default.

Here's an example of that in action. Given this pom.xml:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>

  <groupId>org.hamcrest.example</groupId>
  <artifactId>junit-hamcrest</artifactId>
  <version>1.0</version>
  <packaging>jar</packaging>
  <name>JUnit 4 with Hamcrest</name>
  <properties>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
  </properties>

  <dependencies>
    <dependency>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
      <version>4.12</version>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>org.hamcrest</groupId>
      <artifactId>hamcrest-library</artifactId>
      <version>2.1-rc3</version>
      <scope>test</scope>
    </dependency>
  </dependencies>
</project>

Here's the dependency tree:

% mvn dependency:tree -Dverbose
[INFO] Scanning for projects...
[INFO] 
[INFO] ----------------< org.hamcrest.example:junit-hamcrest >-----------------
[INFO] Building JUnit 4 with Hamcrest 1.0
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ junit-hamcrest ---
[INFO] org.hamcrest.example:junit-hamcrest:jar:1.0
[INFO] +- junit:junit:jar:4.12:test
[INFO] |  \- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] \- org.hamcrest:hamcrest-library:jar:2.1-rc3:test
[INFO]    \- (org.hamcrest:hamcrest-core:jar:2.1-rc3:test - omitted for conflict with 1.3)
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 0.691 s
[INFO] Finished at: 2018-11-30T18:01:11Z
[INFO] ------------------------------------------------------------------------

If I change the order of dependency declaration:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>

  <groupId>org.hamcrest.example</groupId>
  <artifactId>junit-hamcrest</artifactId>
  <version>1.0</version>
  <packaging>jar</packaging>
  <name>JUnit 4 with Hamcrest</name>
  <properties>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
  </properties>

  <dependencies>
    <dependency>
      <groupId>org.hamcrest</groupId>
      <artifactId>hamcrest-library</artifactId>
      <version>2.1-rc3</version>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
      <version>4.12</version>
      <scope>test</scope>
    </dependency>
  </dependencies>
</project>

Then the hamcrest-library version "wins" the precedence:

% mvn dependency:tree -Dverbose
[INFO] Scanning for projects...
[INFO] 
[INFO] ----------------< org.hamcrest.example:junit-hamcrest >-----------------
[INFO] Building JUnit 4 with Hamcrest 1.0
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ junit-hamcrest ---
[INFO] org.hamcrest.example:junit-hamcrest:jar:1.0
[INFO] +- org.hamcrest:hamcrest-library:jar:2.1-rc3:test
[INFO] |  \- org.hamcrest:hamcrest-core:jar:2.1-rc3:test
[INFO] |     \- org.hamcrest:hamcrest:jar:2.1-rc3:test
[INFO] \- junit:junit:jar:4.12:test
[INFO]    \- (org.hamcrest:hamcrest-core:jar:1.3:test - omitted for conflict with 2.1-rc3)
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.112 s
[INFO] Finished at: 2018-11-30T18:03:43Z
[INFO] ------------------------------------------------------------------------

I'll look to update the docs with this info.

garretwilson commented 5 years ago

If I change the order of dependency declaration … [t]hen the hamcrest-library version "wins" the precedence …

That was news to me, and I've been using Maven a very long time. Then again it makes sense, because they are both transitive dependencies at the same level, but I don't think I've seen this specific situation. I'm not sure I would want to rely on the order of declaration, though; that sounds a little brittle. I personally might stick to just declaring hamcrest-core explicitly.

But it's an interesting consideration; thanks for pointing this out.

olibye commented 5 years ago

Can I suggest the following:

Other than that my upgrades revealed to me how far the 2015 release of hamcrest was behind master. Upgrading from 1.3 the following have simply been deleted without deprecation. This is mean as it doesn't explain to the casual user how to upgrade.

Other breaking API changes with generics

On 29 Nov 2018 18:36, "Joe Schmetzer" notifications@github.com wrote:

I've just release version 2.1-rc3 to resolve the problem with maven transitive dependency resolution.

I'd be interested in hearing feedback from folks who have tried this in Gradle, Maven, or any other dependency management tool out there.

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/hamcrest/JavaHamcrest/issues/224#issuecomment-442944077, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQK0N2p_SlInjtWLJR_bi-FEj8g_S-tks5u0CkQgaJpZM4YNxB2 .

tumbarumba commented 5 years ago

Thanks @olibye. I've made a start at a pull request to describe some of the concerns (#PR #237). I don't think it's quite ready yet, but the distributables page can be previewed at https://tumbarumba.github.io/JavaHamcrest/distributables

Regarding the breaking changes, I didn't realise there were those ones until you mentioned them (though it should have been obvious in retrospect). This release was supposed to be more like a 1.4 release (increment minor version with no breaking changes). I only chose 2.1 because 2.0.0.0 had been accidentally released a few years back, and I thought we needed a clear statement that the version we're releasing now is newer.

For IsCollectionContaining and IsArrayContainingInOrder, it should be possible to re-create the original classes, but deprecate them in favour of the new classes. We can remove them if we get around to a 3.0 release.

I'm not sure what you mean by IsA. Which class is that?

olibye commented 5 years ago

I mean Matchers.isA and Is.isA. It used to be Matcher isA(Class type) It's now Matcher isA(Class<?> type)

Eclipse is over fussy about this but it can be easily changed to Matchers.any(Class type)

On Mon, 3 Dec 2018 at 23:05, Joe Schmetzer notifications@github.com wrote:

Thanks @olibye https://github.com/olibye. I've made a start at a pull request to describe some of the concerns (#PR #237 https://github.com/hamcrest/JavaHamcrest/pull/237). I don't think it's quite ready yet, but the distributables page can be previewed at https://tumbarumba.github.io/JavaHamcrest/distributables

Regarding the breaking changes, I didn't realise there were those ones until you mentioned them (though it should have been obvious in retrospect). This release was supposed to be more like a 1.4 release (increment minor version with no breaking changes). I only chose 2.1 because 2.0.0.0 had been accidentally released a few years back, and I thought we needed a clear statement that the version we're releasing now is newer.

For IsCollectionContaining and IsArrayContainingInOrder, it should be possible to re-create the original classes, but deprecate them in favour of the new classes. We can remove them if we get around to a 3.0 release.

I'm not sure what you mean by IsA. Which class is that?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hamcrest/JavaHamcrest/issues/224#issuecomment-443904729, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQK0C08R2u4w78Hm9F_8R1NmvLAonP3ks5u1a4lgaJpZM4YNxB2 .

sf105 commented 5 years ago

From what I remember, the trouble is that it's not always the case that the type being checked is the same as the type of the matcher. And, AFAIR, Intellj is fussy in the opposite direction. I lost years of my life to Java's half-baked generics.

marcphilipp commented 5 years ago

Interesting discussion! I'm certainly glad that Hamcrest is still alive. 🙂

Have you considered publishing a pom-only artifact that uses relocation instead of publishing empty jars?

tumbarumba commented 5 years ago

Ooh, I didn't know about relocation! Thanks, @marcphilipp! I think that's exactly what we were looking for. I tried to RTFM, but didn't quite hit upon the right search terms.

I might try a few experiments, and see if it's worth releasing another rc.