search

uklance / gradle-dependency-export

Export maven dependencies from a gradle project to the file system
27 stars 14 forks source link

Update implementation #2

Closed stefanleh closed 5 years ago

stefanleh commented 5 years ago

Sorry for the german comments. Will fix that in 0.3 ...

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 0.0% when pulling a04faee5395047e57a02178e608d82f8580ac394 on stefanleh:master into ef5b2a7db66a9798be14b475162f4e4ea116da66 on uklance:master.

uklance commented 5 years ago

Thanks for your contribution. I'll hopefully try it out soon and push out a release for you.

Cheers, Lance

uklance commented 5 years ago

I built and published version 0.2 of this plugin to the plugin portal. Apparently it can take up to 24 hours to be approved.

stefanleh commented 5 years ago

Thanks. I appreciate your effort for releasing.

uklance commented 5 years ago

I made a couple of changes and release version 0.3

  1. I removed defaultConfigurations.findAll { it.canBeResolved } logic. If you need a custom collection of configurations, please manually set the configurations (see readme)
  2. I added a systemProperties Map to the task (defaults to System.getProperties()) which can be customized.
stefanleh commented 5 years ago

Great News. Will send a PR soon for the logging and comments in english.

stefanleh commented 5 years ago

I just saw that you left the defaultConfugurations handling in place. The defaults dont work with current gradle relaeses, thats the cause i added defaultConfigurations.findAll { it.canBeResolved }. All configuraitons considered have the resolvable. The current default handling will break all the time and is therefore useless. I suggest to remove it completely and rely on proper configuration by the user.

uklance commented 5 years ago

The test case works perfectly with default configurations. Can you tell me a bit more about these unresolvable configurations you have?

I'd prefer my build to fail if it can't export a dependency rather than skipping it silently only for something else to fail building offline.

On Sun, 24 Feb 2019, 6:56 pm Stefan, notifications@github.com wrote:

I just saw that you left the defaultConfugurations handling in place. The defaults dont work with current gradle relaeses, thats the cause i added defaultConfigurations.findAll { it.canBeResolved }. All configuraitons considered have the resolvable. The current default handling will break all the time and is therefore useless. I suggest to remove it completely and rely on propert configuration by the user.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/uklance/gradle-dependency-export/pull/2#issuecomment-466805082, or mute the thread https://github.com/notifications/unsubscribe-auth/ABW1JC1iwYAec_IRGDw8HUgUlRrlOWsaks5vQuBfgaJpZM4bJMxd .

stefanleh commented 5 years ago

For me it was:

FAILURE: Build failed with an exception.

Guess you dont have testRuntimeOnly in you testcase?

uklance commented 5 years ago

Whoops, I was wrong

https://github.com/uklance/gradle-dependency-export/blob/master/src/test/groovy/com/lazan/dependency/export/MavenDependencyExportTest.groovy#L52

The test sets a configuration which means the default logic doesn't fire

On Sun, 24 Feb 2019, 7:10 pm Stefan, notifications@github.com wrote:

For me it was:

FAILURE: Build failed with an exception.

  • What went wrong: Could not determine the dependencies of task ':mavenDependencyExport'.

Resolving configuration 'testRuntimeOnly' directly is not allowed

Guess you dont have testRuntimeOnly in you testcase?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/uklance/gradle-dependency-export/pull/2#issuecomment-466806544, or mute the thread https://github.com/notifications/unsubscribe-auth/ABW1JFqYzBv429VdFGTXdptLfsGAD4Xcks5vQuOagaJpZM4bJMxd .

stefanleh commented 5 years ago

Currently i have to use:

mavenDependencyExport {

    for (Configuration conf : project.buildscript.configurations.findAll{ it.canBeResolved }) {
        configuration conf
    }
    for (Configuration conf : project.configurations.findAll{ it.canBeResolved }) {
        configuration conf
    }

}

to make it work. Maybe making the configurations non private would be fine? Then i can remove at least the foreach loops.

uklance commented 5 years ago

I didn't make configurations public/settable because it might get confused with project.configurations (people often use "configurations" in a gradle script without qualifying it)

Let me do some research on canBeResolved flag. Perhaps your logic is more sensible.

On Sun, 24 Feb 2019, 7:17 pm Stefan, notifications@github.com wrote:

Currently i have to use:

mavenDependencyExport {

for (Configuration conf : project.buildscript.configurations.findAll{ it.canBeResolved }) {
    configuration conf
}
for (Configuration conf : project.configurations.findAll{ it.canBeResolved }) {
    configuration conf
}

}

to make it work. Maybe making the configurations non private would be fine? Then i can remove at least the foreach loops.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/uklance/gradle-dependency-export/pull/2#issuecomment-466807248, or mute the thread https://github.com/notifications/unsubscribe-auth/ABW1JAbbllq73bpzuMPXUPi8u2yEI4hQks5vQuVYgaJpZM4bJMxd .

stefanleh commented 5 years ago

I see following approaches:

uklance commented 5 years ago

Ok, looking at the javadocs

https://docs.gradle.org/current/javadoc/org/gradle/api/artifacts/Configuration.html#isCanBeResolved--

"Returns true if it is allowed to query or resolve this configuration. Defaults to true."

I'm happy to put your logic back in the default method. I agree that showing this exclusion in the logs is a good idea.

I'm also happy to log the configuration name and also the filenames via the Gradle logger. This would need to be in English rather than German.

On Sun, 24 Feb 2019, 7:40 pm Stefan, notifications@github.com wrote:

I see following approaches:

  • removing default handling and adding some hints to documentation
  • filtering the defaults with the canBeResolved and add logging about which configs are considered (and also maybe which were dropped)
  • removing default handling and provide an allResolvableConfigurations method which can be used in the plugin configuration in the mavenDependencyExport closure

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/uklance/gradle-dependency-export/pull/2#issuecomment-466809401, or mute the thread https://github.com/notifications/unsubscribe-auth/ABW1JHfp9iFE5VnXhP6Gy2DGWn0lroaOks5vQuqygaJpZM4bJMxd .