search

unsaved / gradle-ivyxml-plugin

Gradle plugin that loads dependencies according to Ivy dependency files
13 stars 7 forks source link

Excludes as Children of Ivy Dependencies Element #3

Closed stevendick closed 12 years ago

stevendick commented 12 years ago

I added support for global excludes, for example this now works:

<ivy-module ...>
  ...
  <dependencies>
    ...

    <exclude org="org.hamcrest" module="hamcrest-core"/>

  </dependencies>
</ivy-module>

The code gets all exclude rules from the module descriptor and applies each rule to every Gradle configuration.

unsaved commented 12 years ago

Since you obviously have it working, I think it would be easier for you to write a test for it. Could you please add a test method (or more than one if behavior differs in different combinations) to src/test/groovy/com/admc/gradle/IvyxmlPluginTest.groovy then resubmit inclusive pull request?

unsaved commented 12 years ago

BTW, thanks for the enhancement. If you can't write the test, I will, but if you do I can publish 0.3.1 soon.

stevendick commented 12 years ago

Do the tests run in your environment? Thinking I introduced a problem, I created a branch based on 3.0 release, but that also has 3 failing tests (as does head).

Head also reports an out of memory exception, thinking this was related to the testing, I bumped the heap to 1g for the test phase, but that doesn't help.

unsaved commented 12 years ago

Re. the 3 failing tests, please read https://github.com/unsaved/gradle-ivyxml-plugin/raw/master/doc/upgrade.txt .

I run the tests all the time and have never had an OOME. Your unit tests apparently completed or you could not have gotten the expected result of 3 failures out of over 80 tests. What task are you running when you get the OOME, what version of Java, what operating system? If your own setup, please supply the files so I can try to reproduce. Also, your invocation command showing whether you are using the provided gradlew, etc.

stevendick commented 12 years ago

The out of memory problem only appears on my work PC - hmm, not sure what's up with that.

stevendick commented 12 years ago

I added a test dependency on Hamcrest for the separation of the assertion and matching logic - you get precise failure messages. I'd also recommend looking at the Spock test framework (sits on top of JUnit) - it makes use of Groovy's power assertions and you get a very clear and detailed explanation of the failure and the values involved.

stevendick commented 12 years ago

As the OOM only happens on my office PC (home PC is fine), I'd ignore this as an isolated environmental issue.

On 29 December 2011 17:18, Blaine Simpson reply@reply.github.com wrote:

Re. the 3 failing tests, please read https://github.com/unsaved/gradle-ivyxml-plugin/raw/master/doc/upgrade.txt .

I run the tests all the time and have never had an OOME.  Your unit tests apparently completed or you could not have gotten the expected result of 3 failures out of over 80 tests.  What task are you running when you get the OOME, what version of Java, what operating system?  If your own setup, please supply the files so I can try to reproduce.

Reply to this email directly or view it on GitHub: https://github.com/unsaved/gradle-ivyxml-plugin/pull/3#issuecomment-3302084

stevendick commented 12 years ago

Those 3 failing tests seem logically flawed - I see the excludes on a dependency only affecting upstream dependencies: I would not expect them to exclude the dependency they are a child of.

unsaved commented 12 years ago

Ah... I'm replacing this post because I confused your point with a closely-related one.

Agreed. I never thought that it was smart for an exclusion inside a specific dependency to effect other dependencies, but this is indeed how it worked with Gradle 1.5. It should behave however raw, native Ivy behaves. I will test that today.

unsaved commented 12 years ago

I was thinking that the tests modified unrelated dependencies, and that was wrong. As you explained, the tests only expect the exclusions to affect parent elements. I have definitely tested that dependency-nested exclusions do not affect unrelated dependencies and that is all working as it should. Back on to the real point, about an exclusion affecting a parent dependency...

I reviewed the tests, I found my work area where I had tested this with native Ivy before writing those tests, and I repeated the tests to see what native Ivy does.

Understanding that the goal of Ivyxml is to emulate Ivy's behavior, not improve upon it, the 3 failing tests are not logically flawed. Those tests require the behavior of native Ivy, and you are expecting the wrong behavior because Ivy definitely does exclude dependencies that the exclusion is a child of. Gradle used to match this same behavior with API methods for their own Configuration and Dependency objects, but no longer in milestone 6. The tests fail, as they should to indicate failure to match native Ivy behavior, until I learn how to use the Gradle API to duplicate native Ivy behavior again.

This isn't a trivial difference. Based on your assumption, if you depend on group/organization x's a.jar, and a.jar depends on b.jar and c.jar, and you want to exclude b.jar and c.jar here, but not effect other dependencies, you could add an exclusion for org "x" under the a.jar dependency. But that would not work in native Ivy because the exclusion would also exclude a.jar.

It is probably true that the needed work-around would require modifying sets or closures that Gradle assumes are internally-managed, or other similar tactics which would be brittle due to very tight coupling with Gradle internals. If I learn enough to confirm that, I'll remove the unit tests and just document this deviation from native Ivy behavior.

So for now, the behavior of Ivyxml and its unit tests seems to be correct. I will, however, add tests to either confirm or deny the primary use case for dependency-nested exclusions: transitive dependencies.

stevendick commented 12 years ago

You can mark the tests with @Ignore so JUnit skips them for now.

On 29 December 2011 21:25, Blaine Simpson reply@reply.github.com wrote:

I was thinking that the tests modified unrelated dependencies, and that was wrong.  As you explained, the tests only expect the exclusions to affect parent elements.  I have definitely tested that dependency-nested exclusions do not affect unrelated dependencies and that is all working as it should.  Back on to the real point, about an exclusion affecting a parent dependency...

I reviewed the tests, I found my work area where I had tested this with native Ivy before writing those tests, and I repeated the tests to see what native Ivy does.

Understanding that the goal of Ivyxml is to emulate Ivy's behavior, not improve upon it, the 3 failing tests are not logically flawed.  Those tests require the behavior of native Ivy, and you are expecting the wrong behavior because Ivy definitely does exclude dependencies that the exclusion is a child of.  Gradle used to match this same behavior with API methods for their own Configuration and Dependency objects, but no longer in milestone 6.  The tests fail, as they should to indicate failure to match native Ivy behavior, until I learn how to use the Gradle API to duplicate native Ivy behavior again.

This isn't a trivial difference.  Based on your assumption, if you depend on group/organization x's a.jar, and a.jar depends on b.jar and c.jar, and you want to exclude b.jar and c.jar here, but not effect other dependencies, you could add an exclusion for org "x" under the a.jar dependency.  But that would not work in native Ivy because the exclusion would also exclude a.jar.

It is probably true that the needed work-around would require modifying sets or closures that Gradle assumes are internally-managed, or other similar tactics which would be brittle due to very tight coupling with Gradle internals.  If I learn enough to confirm that, I'll remove the unit tests and just document this deviation from native Ivy behavior.

So for now, the behavior of Ivyxml and its unit tests seems to be correct.  I will, however, add tests to either confirm or deny the primary use case for dependency-nested exclusions:  transitive dependencies.

Reply to this email directly or view it on GitHub: https://github.com/unsaved/gradle-ivyxml-plugin/pull/3#issuecomment-3304833

unsaved commented 12 years ago

Great suggestion. Will do.

I appreciate the Spock suggestion too, but I'm not willing to spend time to learn it now. Today's investigation has taken time that I needed to be spending on other projects. I'm going to try to wrap up the current batch of Ivyxml work in the next couple hours. (Unfortunately, I can't concentrate on this exclusively as I'm collaborating with other people on unrelated work).