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 bootRepackage should use a classifier instead of an extension #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 bootRepackage should use a classifier instead of an extension #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.