Spring-boot: bootRepackage's jar rewriting defeats Gradle's up-to-date checks, causing unnecessary rebuilds

[toolforger]

bootRepackage overwrites build/libs/xxx.jar, keeping the original jar as build/libs/xxx.jar.original.

On the next build, Gradle will find that build/libs/xxx.jar has been changed compared to what the previous run's jar task left, so it will assume that build/libs/xxx.jar needs to be regenerated.

Possible solutions that I can see:
1) Don't overwrite build/libs/xxx.jar, generate build/libs/xxx.boot.jar. (Please see #1112 for another issue that affects the naming code.)
2) Modify Gradle's jar task so that it generates the uber jar right off the bat.

[dsyer]

It's possible we can modify the outputs of the JarTask I suppose. Seems like the best way to keep the existing behaviour.

Do you have a minimal project that we can use to test this behaviour? I'm curious as well: why does a Spring Boot app have anything in the build depend on it? I would have thought it would be the last thing in the chain to do.

[toolforger]

Modifying JarTask would be a possibility, but I see a risk and a downside.
The risk is that the plugin isn't showing a great deal of understanding of Gradle's workings (witness #1114); modifying a task certainly would require a better understanding.
The downside is that existing builds might already assume that a .jar.original exists. It would be a horrible hack to work with that, but we all know how horrible hacks come to existence :-)

Downstream tasks are commonplace actually.
There's the launch4j/nsis post-build wrapping that creates a Windows .exe and an installer.
There's the deployment stuff - upload to web sites, automatically announce updates, pre-release testing, all the automatable stuff that happens after the build.
As an example for that kind of stuff, here's the pertinent section of my build.gradle:

// Here's for Launch4j, which generates Windows .exe files
// This is useful because the Task Manager will show the application with its .exe file name instead of as just "java".
task launch4j(type: Exec, dependsOn: bootRepackage) {
    System.out.println(project.projectDir);
    System.out.println(new File(project.projectDir, 'launch4j.xml'));
    if (project.ext.isWindows) {
        commandLine 'cmd', '/c', 'launch4j', new File(project.projectDir, 'launch4j.xml')
    } else {
        commandLine 'launch4j', 'launch4j.xml'
    }
}

// Here's for NSIS. NSIS puts .exe files and assorted stuff into a Windows installer.

task copyForInstaller(type: Copy, dependsOn: launch4j) {
    from 'build/libs/saturn_ftp_client_service.jar'
    from 'build/libs/saturn_ftp_client_service.exe'
    from 'ftp-settings.xml'
    from 'nssm.exe'
    from 'saturn_ftp_client_service.nsi'
    into 'build/nsis/'
}

task makeInstaller(type: Exec, dependsOn: copyForInstaller) {
    workingDir 'build/nsis'
    if (project.ext.isWindows) {
        commandLine 'cmd', '/c', 'makensis', '/V2', 'saturn_ftp_client_service.nsi'
    } else {
        commandLine 'makensis', '-V2', 'saturn_ftp_client_service.nsi'
    }
}

// This is my (rather frugal) task to prepare a distribution file.
// Website upload isn't included here.

task buildDistribution (dependsOn: [
    bootRepackage,
    launch4j,
    makeInstaller,
    projectReport,
    propertyReport,
    taskReport,
    dependencyReport,
    htmlDependencyReport]) {
}

There's another use case that I don't have myself but see others doing:
Have a jar that's ordinarily used as a dependency in another build, but that contains a main program for testing and demonstration purposes.
The generated uber jar isn't suitable as a dependency, the dependent project would have to unpack the jar first.
So the generating project would probably create two Maven projects, one to generate the normal jar and one to generate the application, consisting of the thin main program and a single dependency on the jar. It's feasible but a bit proliferating on the number of projects in the workspace.

I agree that keeping the renaming as the default behaviour would be necessary; you'd lose quite a few users if their builds suddenly break because the jar name changes.
Things that could be done:

• Deprecate the current behaviour, spitting out warnings for half a year. (Mitigates the impact but will still create considerable fallout.)
• Add options to specify whether the input file can be renamed, and possibly how to name the output. You'd stay independent of Gradle's jar task, and it wold technically work, but it would probably run counter to Spring Boot's goal, Simplifying Things and Automatically Doing the Right Thing(TM) for the users. (Actually, renaming an input file is the Wrong Thing to Do (TM) in Gradle, see #1112.)
• Create a variant of bootRepackage. Name it generateApp or something. No options, automatically does the right thing: Leave the input jar alone and generate a differently-named repackaged application jar.

[dsyer]

I see (I didn't think of downstream tasks, I was wrongly assuming that downstream projects would be the only problem).

A new task generateApp sounds like it might work and keep everybody fairly happy.

As far as the plugin "showing a great deal of understanding," I totally agree (no-one with _any_ Gradle knowledge has ever worked on it). If you want to fix it, you'd be more than welcome to propose changes. The Gradleware guys have shown some interest in getting involved as well, but we've yet to see any concrete action on that front.

[toolforger]

Heh. This is a classic case of the one-eyed in the land of the blind.
I.e. I'm just learning Gradle. Probably more than most do, but still pretty far behind, and zero experience with writing plugins. Actually I'm standing in awe before the Gradle people if they were able to set up a framework that people with little experience can write working plugins for.

Re the actual issue: I there's an "inputs" and an "outputs" property.
As far as I understand http://www.gradle.org/docs/current/userguide/more_about_tasks.html#sec:task_inputs_outputs , all the plugin needs to do is to take the file list(s?) it's processing and stuff these files into the inputs property inherited from the superclass.
Likewise, it should put the list of files written into the outputs property.

I'd be issuing a pull request if I understood the plugin code well enough to identify the input files with certainty. (My lack of understanding comes from the plugin accessing Gradle features that I do not understand well, not from the plugin's code qualities - I didn't see any quality issues in the code actually.)
IOW tell me at what point in the code the plugin has a full list of inputs, and I can do it if you'd like me to.

[dsyer]

The RepackageTask has one input (the original jar) and one output (the modified jar), or two if you count the copy of the original jar. You can find the file name in the Jar task. It's the RepackageAction attached to the Jar task that does the work though, and it can communicate back to the RepackageTask to say what its output file is because its an inner class. I tried it myself, but Gradle won't let you register output files _after_ a task has started executing, so I didn't find a great place to put the hook.

If we add a new GenerateExecutableArchiveTask task (or whatever we call it) then the same mechanism, and most of the same code can be used. Or we could add a new property to the RepackageTask to signal that you want to create a new file (not overwrite the old one).

See also #141 which is really the same issue (although the discussion got a bit off topic).

[toolforger]

Gradle has two phases: a planning phase and an execution phase. (Probably not the official terminology.)
Gradle determines in the planning phase what tasks to run, so it needs to have the dependency info, inputs and outputs before the task executes; I suspect the finding that "you can't register inputs after the task has started running" is related to that issue.
However, you can add code for both phases. I'm too fuzzy about the details; at the build.gradle level, the planning-phase stuff goes into the DSL (i.e. the "configuration-ey" part), the execution-phase stuff into doFirst or doLast.

[dsyer]

I think I figured something out (I'll push if the tests all pass in a minute). If you could try it out that would be great. I know for sure that the up-to-date check works, but I could be missing something else I guess.

[toolforger]

I just tested the build with 1.1.2 snapshot.
The classifier task is in and works as expected - which was that if classifieris present, bootRepackage keeps the original .jar unmodified.
Things still missing:
1) bootRepackage does not declare its outputs, so Gradle still re-runs it every time.
2) classifier is not (yet) documented on http://docs.spring.io/spring-boot/docs/1.1.2.BUILD-SNAPSHOT/reference/htmlsingle/#build-tool-plugins-gradle-configuration-options .

As it is, it's in conflict with Spring Boot's claim to do the right thing by default: Overwriting input files is definitely not the right thing to do for a Gradle plugin.
To avoid backward compatibility issues, I'd like to suggest to deprecate bootRepackage and add a new task bootGenerateApp (or whatsitsname) that's just like bootRepackage but with a non-null default for classifier.

[dsyer]

bootRepackage does not declare its outputs, so Gradle still re-runs it every time.

Yes it does, and it works for me (unless I'm misreading the output, which is entirely possible):

$ gradle build
...
:jar
:bootRepackage
...
$ gradle build
:compileJava UP-TO-DATE
:processResources UP-TO-DATE
:classes UP-TO-DATE
:jar UP-TO-DATE
:bootRepackage UP-TO-DATE
...

Can you explain what's missing?

classifier is not (yet) documented on http://docs.spring.io/spring-boot/docs/1.1.2.BUILD-SNAPSHOT/reference/htmlsingle/#build-tool-plugins-gradle-configuration-options .

Also not entirely correct: https://github.com/spring-projects/spring-boot/blob/master/spring-boot-docs/src/main/asciidoc/build-tool-plugins.adoc#repackage-configuration. Pull requests are welcome if you need more.

Overwriting input files is definitely not the right thing to do for a Gradle plugin.

Maybe, but I think we are in a happier place and I'm not proposing any changes right now. You can open new issues if you like, but only if you feel really strongly, please.

[toolforger]

You're right.

I inadvertently looked into the log for a project that didn't use the snapshot version of bootRepackage.

And I wasn't aware that the docs.spring.io site could be out of date.
Looks nice, the text is fine for me.

---

Some more testing:

:saturn_ftp_client_service:jar (Thread[Daemon Thread 4,5,main]) started.
:saturn_ftp_client_service:jar
Executing task ':saturn_ftp_client_service:jar' (up-to-date check took 0.004 secs) due to:
  Input file /home/jo/Projekte/Powerservice/workspace/saturn_ftp_client_service/build/classes/main/ftpClientService/gui/MainFrame.class has changed.
:saturn_ftp_client_service:jar (Thread[Daemon Thread 4,5,main]) completed. Took 0.046 secs.
:saturn_ftp_client_service:bootRepackage (Thread[Daemon Thread 4,5,main]) started.
:saturn_ftp_client_service:bootRepackage
Skipping task ':saturn_ftp_client_service:bootRepackage' as it is up-to-date (took 0.001 secs).
:saturn_ftp_client_service:bootRepackage UP-TO-DATE
:saturn_ftp_client_service:bootRepackage (Thread[Daemon Thread 4,5,main]) completed. Took 0.001 secs.

It thinks bootRepackage is up-to-date even though its input (saturn_ftp_client_service.jar) changed.
Does bootRepackage declare its inputs? Declaring an output triggers up-to-date checking in the first place, and it will go wrong if Gradle doesn't know about the inputs, which it doesn't unless you tell Gradle about them.
Note that it's better to err by including too many inputs rather than to err by including too few. If too many, Gradle will rerun the task in those case where the gratuitious inputs changed, which is a waste of CPU cycles; if too few, Gradle will not re-run the task if just some forgotten inputs changed, generating a wrong result.

[dsyer]

Inputs declared now. Thanks for the tip (not sure I get what it does, but it doesn't break anything at least).

[toolforger]

For an explanation, see http://www.gradle.org/docs/current/userguide/more_about_tasks.html , section 9 "Skipping tasks that are up-to-date".
The short explanation is in subsection 9.2 "How does it work?"
(Currently http://www.gradle.org/docs/current/userguide/more_about_tasks.html#N10F86 )

[toolforger]

I'm now consistently getting a groupId must not be null on builds with 1.1.2.BUILD-SNAPSHOT that have been working fine with 1.0.2.RELEASE.
I don't know whether that's related to this change or something that happened afterwards. _Update: I'm getting this on 1.1.1.RELEASE as well, i.e. probably an unrelated problem._
_Update 2: Reported as #1133_

[toolforger]

Did some more testing.

Changes in the main jar now correctly cause a rebuild.
Changes in a dependent jar, however, still do not cause rebuilds.

The cause seems to be that line 96 adds just the main archive, not all the other archives that bootRepackage includes.

On an aside note, I noticed that the setClassifier parameter task is redundant with the member variable task, i.e. you can simply drop the parameter. One name less to figure out the semantics for during code reading is a Good Thing IMHO.

[dsyer]

Isn't the main archive the one that is repackaged? So unless that changes, why would you want a rebuild? Maybe you can provide some examples (better yet write a test - see spring-boot-integration-tests for some existing gradle tests).

[toolforger]

bootRepackage includes jars from dependencies as well (that's the whole point of bootRepackage after all).
If these change, the repackaged jar needs to be rebuilt.

I'd be willing to write a test, but I have no idea how to set up a multi-project build during testing.
Or how to check the contents of the built artifact (could be done using zip, could be done by running it and checking outputs, but I have no idea how to write any of the two without embarking on a multi-day project, which I can't squeeze into my time budget anymore).
Is there any code I can copy/inherit from?

[dsyer]

That last commit (08ae390) makes the changes you suggested but doesn't add a test case (contributions gratefully accepted - if you look in the spring-boot-integration-tests you will find some tests that assert the contents of archives already I believe).

[toolforger]

There's a single manual smoke test in https://github.com/spring-projects/spring-boot/tree/master/spring-boot-tools/spring-boot-gradle-plugin/src/manual-test.
Which is hardcoded to use 1.1.0-SNAPSHOT, so it's already getting out of date (it should probably retrieve the pom.xml and extract the version from there).

I'll try to contribute at least a manual test, but no promises - I'm on a rather tight schedule and this could easily get pushed off my radar.
It's still not working, unfortunately, but I can see that it's hard to test for you without a test case.

[dsyer]

The tests are in spring-boot-integration-tests (that's the third time I told you, is there a problem finding them?): https://github.com/spring-projects/spring-boot/tree/master/spring-boot-integration-tests/src/test/java/org/springframework/boot/gradle. Don't worry, if it works, we'll get some more tests in there eventually.

[toolforger]

Yeah there was, I was looking for tests for the Gradle plugin in, well, spring-boot-gradle-plugin.

[toolforger]

Okay, finished testing. I can now confirm that direct .jar dependencies are properly included.
(At one point, I mistakenly thought it would not actually have changed in any way, but then I remembered that SNAPSHOT stuff isn't reloaded on every run, so I blew my .gradle cache away and then it worked.)

Thanks for all the work!

I hope that until mid-week, I can build a Gradle multi-project build that could be used for a test case.
I'm awfully sorry I can't build the full unit test myself; I'd have loved to, but too little time, too many things to do :-(
Is there a specific directory I should put the directory into for a pull request?

[youribonnaffe]

If I understand correctly, is setting a classifier for bootRepackagethe only way to get an (almost) incremental build with Gradle?
I tried with the spring-boot-sample-simple project with version 1.3.0-RELEASE and without

bootRepackage {
        classifier = "full"
}

jar and bootRepackage tasks will always be executed.
Even with this setup, the task findMainClass is always executed.

[wilkinsona]

If I understand correctly, is setting a classifier for bootRepackagethe only way to get an (almost) incremental build with Gradle?

Yes. Gradle re-runs that jar task as the repackage task has overwritten its output. You can see this by running your build with --info. For example:

Executing task ':jar' (up-to-date check took 0.031 secs) due to:
  Output file /Users/awilkinson/dev/spring/spring-boot/spring-boot-samples/spring-boot-sample-simple/build/libs/spring-boot-sample-simple-0.0.0.jar has changed.

Setting a classifier prevents this as it causes the repackage task to write the repackaged jar to a separate location. I'm not sure that requiring a classifier was the right way to solve the problem that was reported in this issue. Producing an entirely separate artifact may have been better. If you'd like us to revisit this, please open a new issue.

Even with this setup, the task findMainClass is always executed.

findMainClass doesn't declare any outputs so Gradle always assumes that it's out of date and needs to be run. If you'd like that to be improved please open a new issue or, even better, a pull request.

[bademux]

Overriding original jar is pure evil, try to

testCompile project(':my_spring_boot_application')

... and build can't find original sources.
imho it was really bad decision, because it overrides default gradle behavior

[philwebb]

@bademux I don't think it's _"pure evil"_ or a _"really bad decision"_ and comments like this on a issues that's been closed a year aren't really constructive. Have you tried reading this section of the documentation which describes how you can use a Spring Boot application as a dependency?

[bademux]

@philwebb changing default behavior of my building system isn't exactly what I expect from spring plugin. Thanks for the link.