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

Fail when applied on non-root project #47

Closed
szpak opened this issue Mar 24, 2017 · 26 comments
Closed

Fail when applied on non-root project #47

szpak opened this issue Mar 24, 2017 · 26 comments
Milestone

Comments

@szpak
Copy link
Member

szpak commented Mar 24, 2017

Currently there is a warning displayed, but there were invalid issues reported which tends that people do not read it. Error should be thrown with detailed information why it happened and a request to report an issue when it is really needed.

@szpak szpak added this to the 0.11.0 milestone Aug 18, 2017
@szpak szpak closed this as completed in ac44ef1 Aug 18, 2017
@UweTrottmann
Copy link

Within my project I only deploy a single module (API). I had to apply the plugin to the sub-module as otherwise credentials from the maven deployer config of the sub-module are not picked up. See UweTrottmann/SeriesGuide@ca33e8a.

@depryf
Copy link

depryf commented May 22, 2018

For the records, I had the same issue. Applying the plugin to the root means that Gradle doesn't find the tasks in the subprojects anymore. Applying the plugin to the subprojects triggers this new error. So I am not sure how this plugin is supposed to be used with subprojects.

Only way to make it work was to downgrade to version 0.10.0.

@szpak
Copy link
Member Author

szpak commented May 22, 2018

Thanks for information. I will probably add a switch to override it if really needed.

@dmarsche
Copy link

One more vote for adding of an override switch. Can take a look at it if pull requests are welcome.

I have a bunch of Java projects using the plugin, and I need to apply some common build logic (not related to artifacts distribution) to all of the projects. I tried to do it dynamically in an umbrella Gradle project, but bumped into the exception.
Downgrade to 0.10.0 helped in my case, thank you, @depryf!

@szpak
Copy link
Member Author

szpak commented Oct 20, 2018

@dmarsche Could you elaborate more on your use-case? Couldn't you just have "the common logic to apply on submodules" and "the common logic to apply on the root project"?

@Vampire
Copy link
Contributor

Vampire commented May 11, 2019

@szpak can you please revert this or make some configuration possibility?
I'm now doing new builds on in Kotlin and to not have a looooong build script but still have static accessors for nicer build scripts, I use pre-compiled script plugins.
For generating the static accessors for applied 3rd-party plugins Gradle applies the plugin to some test project, but to a subproject as it seems.
This then fails for this plugin and the whole process is busted: gradle/gradle#9386

@Vampire
Copy link
Contributor

Vampire commented May 11, 2019

Well, the build does not fail, but there is a big fat stack trace in the output and no static accessors available.

@Vampire
Copy link
Contributor

Vampire commented May 11, 2019

Not only for this plugin, but for all in this script

@szpak
Copy link
Member Author

szpak commented May 12, 2019

@Vampire A few years old Kotlin scripting in Gradle seems to still be somehow immature and troublesome :).

I plan to add an ability to override that restriction in the next version of the plugin.

@Vampire
Copy link
Contributor

Vampire commented May 12, 2019

I'd more say it is really awesome.
Of course it has bugs, as any software has.
Is there an ETA for a version with this override?

@szpak
Copy link
Member Author

szpak commented May 12, 2019

Maybe I will be convinced one day :).

The next planned release is 0.30.0 with rewriting the plugin internals to adjust it to the new Property/Provider infrastructure to manage plugin configuration in Gradle 4.8+. Nevertheless, Continuous Delivery is in place. Therefore, having a nice looking PR (in the similar way to that, but with error instead of warning and information how to override that behavior if really needed), some new minor release would not be a big effort for me :).

@Vampire
Copy link
Contributor

Vampire commented May 12, 2019

Hm, thinking about it, a property might not be the best solution.
At least not for the problem I have.
I'm not sure whether the properties are forwarded to the build that is used to generate the static accessors.

@szpak
Copy link
Member Author

szpak commented May 12, 2019

I don't know, you would need to check it first. But assuming it's similar to the build compiling buildSrc it could be (both read from project/usr-local gradle.properties and -P).

@Vampire
Copy link
Contributor

Vampire commented May 12, 2019

Is there a way in your build to publish to mavenLocal()? Using composite build does not work yet from buildSrc build.

@Vampire
Copy link
Contributor

Vampire commented May 12, 2019

However, I just built the JAR and depended on the JAR in the filesystem.
Unfortunatley property will not work for my use-case.
Neither specifying it with -P, nor in <project-dir>/gradle.properties, nor in ~/.gradle/gradle.properties had any effect. :-(

@Vampire
Copy link
Contributor

Vampire commented May 12, 2019

How about this:

diff --git a/src/main/groovy/io/codearte/gradle/nexus/NexusStagingPlugin.groovy b/src/main/groovy/io/codearte/gradle/nexus/NexusStagingPlugin.groovy
index 2bcac95..39de20f 100644
--- a/src/main/groovy/io/codearte/gradle/nexus/NexusStagingPlugin.groovy
+++ b/src/main/groovy/io/codearte/gradle/nexus/NexusStagingPlugin.groovy
@@ -64,7 +64,7 @@ class NexusStagingPlugin implements Plugin<Project> {
     }

     private void failBuildWithMeaningfulErrorIfAppliedNotOnRootProject(Project project) {
-        if (project != project.rootProject) {
+        if ((project != project.rootProject) && (!(project.projectDir.absolutePath =~'([\\\\/])build\\1tmp\\1generatePrecompiledScriptPluginAccessors\\1'))) {
             throw new GradleException("Nexus staging plugin should ONLY be applied on the ROOT project in a build. " +
                 "See https://github.com/Codearte/gradle-nexus-staging-plugin/issues/47 for explanation. Feel free to comment there if you really" +
                 "need to have it applied on subproject.")

This will recognize that it is applied in the static accessors determination step and will not emit the error.
With this single change, it works perfectly fine and still has the safe-guard for the actual project.

@szpak
Copy link
Member Author

szpak commented May 14, 2019

You probably agree that it is a "hack" which is also somehow fragile as it depends on Gradle "internals". You hit two issues in Gradle and I'm reluctant to add that "hack" to the project to mitigate it (I still plan to add -P).
As it is not a critical issue (a build goes on), I propose you to live with it until the Gradle team fix it or use a Jitpack version made code modified by you for you experiments. Would it be sufficient for you?

Is there a way in your build to publish to mavenLocal()?

Sure. I use the old good maven plugin for publishing. Therefore, gw install is something that you're looking for.

@Vampire
Copy link
Contributor

Vampire commented May 14, 2019

I agree it is a hack and I agree it is somewhat fragile if Gradle internals change.
Yet I think the hack is necessary and even if fragile and the check fails you didn't loose anything as you are then in the same situation you are in now.
The only difference is, for the Gradle versions where the hack applies it just works fine.

As it is not a critical issue (a build goes on), I propose you to live with it until the Gradle team fix it or use a Jitpack version made code modified by you for you experiments. Would it be sufficient for yo

The issue is critical, the build does not go on, except you rewrite your build to not use static accessors as soon as you add the plugin as all static accessors will suddenly stop being available due to this exception.

Besides that you have a big fat stacktrace in your build output each time the build script needs to be re-evaluated.

Having the uglier build script without static accessors and ignoring the stacktrace in the build output are imho also bad hacks, just that every user of your plugin that wants to use it in Kotlin DSL has to apply these noisy hacks, so I'd greatly prefer having one central hack that maybe needs some tweaking from Gradle version to Gradle version but does not do any harm.

or use a Jitpack version made code modified by you for you experiments. Would it be sufficient for you?

Honestly said no, I'd prefer having an official version with that fix that makes it compatible to how the future of Gradle works and not use a fork, as I'm above "experiments" and rather would like to release this project soon.

@szpak
Copy link
Member Author

szpak commented May 14, 2019

Ehhh, I have already some Nexus related hacks to one more should not be a big problem :).

Please provide a PR with:

  • the hack extracted to a separate method with a sensible name (and a link to the issue in a comment)
  • a functional test (with an @Issue anotation) to detect regressions in the future Gradle versions (you can try to use ProjectBuilder (like in LegacyTasksSpec) to reproduce the problem, but I don't know if it operates at that level; alternatively add a test to GradleVersionFuncSpec with some basic setup - I'm curious if Kotlin configurations can be defined inline with nebula-test).

@Vampire
Copy link
Contributor

Vampire commented May 14, 2019

I think I got it more or less setup, except for one thing.
The buildSrc build in the tested project does not compile because it does not know the plugin by id.
Can you tell me how I make it that it gets to know the plugin?

@Vampire
Copy link
Contributor

Vampire commented May 15, 2019

I don't think this is gonna work.
As far as I can see there is no way the nebula-test can inject the plugin classes into the buildSrc build.
It configures the buildscript classpath using an init script, but init scripts that are given as commandline option or similar are only effective on the main build, not on the buildSrc build. (There is an open issue for that)
The nebula-test plugin would need to use a custom Gradle distribution with the init script in there to be effective in the buildSrc build also.
And even then I'm not sure whether it would work as for this use-case it is necessary to apply the plugin using the plugins-DSL block which I think would not work that way.

So to make this work, I think the nebula-test plugin would need to build a maven repository with the built artifact and configure that maven repository as plugins repository via an init script in a custom gradle distribution and as far as I can see, it is not capable of that, so how do you think we should handle this situation?

@szpak
Copy link
Member Author

szpak commented May 15, 2019

I haven't tried to apply my plugin in buildSrc before, especially under test.

As I proposed in the related issue it would be good to get know if TestKit supports it at all. If not, we can probably only report a feature request for that (I wonder how they test it themselves?) and pass the change as is. However, it would mean that you are the only person who knows if it works and in addition there will be no failing tests once it is broken in some future Gradle version. Not the perfect solution, but at least you and your project will be happy ;).

@Vampire
Copy link
Contributor

Vampire commented May 17, 2019

Well, I don't know whether testkit supports it.
One could probably do some other fancy integration testing with first deploying the plugin to a local maven repository and then configure it as plugin repository like suggested in that related issue.
But as you seem to be ok with only me complaining when it fails (or others that adapt precompiled script plugins), I'll provide a PR that adds the work-around without test for now and hope for a fast release. :-)

@Vampire
Copy link
Contributor

Vampire commented May 17, 2019

Here you have it: #117 :-)

@szpak
Copy link
Member Author

szpak commented May 19, 2019

@Vampire Please check 0.21.0.

@szpak
Copy link
Member Author

szpak commented May 19, 2019

And along the way I implemented also a switch to allow to apply on subprojects (#116) for others.

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

No branches or pull requests

5 participants