bndtools / bnd

Bnd/Bndtools. Tooling to build OSGi bundles including Eclipse, Maven, and Gradle plugins.
https://bndtools.org
Other
532 stars 305 forks source link

Manifest Last Modified #3118

Closed swimmesberger closed 5 years ago

swimmesberger commented 5 years ago

It seems that the last modified date of the generated jar of the bnd (gradle) jar task is not updated when only the MANIFEST.MF file is changed*. This is can lead to issues with gradle file caching. For our gradle plugin we rely on the gradle reporting if the generated jar is up to date or not and based on that reporting we do some additional work. When the bnd plugin is used this do not work because sometimes (when only the manifest is changed) gradle reports that the jar file is still up to date (even if it's clearly not because gradle uses last modified before the content is checked).

It know that I could define a manifest file via "-manifest" and then the last modified reporting would work but that is very unfortunate because I want to rely on the bnd manifest generation. Maybe it's possible to compare the previous calculated manifest with the newly calculated manifest and only call the "updateModified" (with the current timestamp) when a change is detected.

As a workaround for now we added some additional properties to our custom gradle task which ensures that it's reexecuted but it would be much cleaner if we could simply rely on the generated jar.

Example of a flawed gradle build cycle with bnd: Task :com.example.bundle:jar Task ':com.example.bundle:jar' is not up-to-date because Value of input property 'someProp' has changed

Task :com.example.bundle:customTask UP-TO-DATE
Skipping task '::com.example.bundle:customTask' as it is up-to-date.

The customTask task defines an InputFile dependency on the generated jar file but even though the jar task of bnd is reexecuted and the file is newly written with an updated Manifest our customTask is not reexecuted because gradle does not detect the file change because the lastModified file property did not change.

pkriens commented 5 years ago

I thought Gradle used hashes nowadays? I know BJ removed a lot of timestamp functions from bnd because Gradle did not care about dates?

I think the best solution is to find out why the manifest changes, if it had the same inputs it should generate the same manifest? If the contents depends on an outside source you could define this in a gradle dependency or include this source in your JAR?

Kind regards,

Peter Kriens

On 2 Apr 2019, at 09:58, S.W. notifications@github.com wrote:

It seems that the last modified date of the generated jar of the bnd (gradle) jar task is not updated when only the MANIFEST.MF file is changed*. This is can lead to issues with gradle file caching. For our gradle plugin we rely on the gradle reporting if the generated jar is up to date or not and based on that reporting we do some additional work. When the bnd plugin is used this do not work because sometimes (when only the manifest is changed) gradle reports that the jar file is still up to date (even if it's clearly not because gradle uses last modified before the content is checked).

It know that I could define a manifest file via "-manifest" and then the last modified reporting would work but that is very unfortunate because I want to rely on the bnd manifest generation. Maybe it's possible to compare the previous calculated manifest with the newly calculated manifest and only call the "updateModified" (with the current timestamp) when a change is detected.

As a workaround for now we added some additional properties to our custom gradle task which ensures that it's reexecuted but it would be much cleaner if we could simply rely on the generated jar.

Example of a flawed gradle build cycle with bnd: Task :com.example.bundle:jar Task ':com.example.bundle:jar' is not up-to-date because Value of input property 'someProp' has changed

Task :com.example.bundle:customTask UP-TO-DATE Skipping task '::com.example.bundle:customTask' as it is up-to-date.

The customTask task defines an InputFile dependency on the generated jar file but even though the jar task of bnd is reexecuted and the file is newly written with an updated Manifest our customTask is not reexecuted because gradle does not detect the file change because the lastModified file property did not change.

https://github.com/bndtools/bnd/blame/master/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java#L121 https://github.com/bndtools/bnd/blame/master/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java#L121 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bndtools/bnd/issues/3118, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMPLnQHEtZWLpLyRfbAflh6sfaqtzjjks5vcw27gaJpZM4cXYeE.

swimmesberger commented 5 years ago

I use Gradle 5.3 and can say that when within the same gradle process (same build cycle) when a additional task uses the output of the bnd jar task as input, gradle considers the file UP-TO-DATE even tough if I examine the content of the jar - the file has changed.

I think the best solution is to find out why the manifest changes, if it had the same inputs it should generate the same manifest? If the contents depends on an outside source you could define this in a gradle dependency or include this source in your JAR?

We have some custom properties in the manifest (like the build which produced the artifact) which can change between each build. We already defines such external properties as dependencies for the bnd gradle jar task but we don't want to duplicate these dependencies for each task which depends on the output jar. So we simply want to rely on the jar task output that when the file is changed in someway gradle reports it as modified but this does not work with bnd.

This would also break plugins which rely on the default behavior of the jar task - when you add attributes to the manifest to the default gradle jar task gradle correctly writes the file modified property of the file bnd does not... Unfortunately this behavior makes writing gradle plugins which rely on bnd much less fun.

pkriens commented 5 years ago

I am going to wait for @bjhargrave since he worked on related issues and knows more about Gradle

bjhargrave commented 5 years ago

This seems to me an issue with how you have expressed your dependencies in gradle. Your customTask should do inputs.files tasks.named('jar'). That is, the customTask's inputs are the jar task itself and not the files you think the jar task is making.

If that does not help, can you please provide a small github repo which demonstrates the problem? Then I can investigate. Thanks.

swimmesberger commented 5 years ago

I have created a small self contained example which reproduces the problem (on Windows 10 x64): https://github.com/swimmesberger/bnd-manifest-bug-test

bjhargrave commented 5 years ago

So I played with your example and the issue seems to be with gradle.

[hargrave@mbp2017 bnd-manifest-bug-test (master)]$ export BND_TEST=3
[hargrave@mbp2017 bnd-manifest-bug-test (master)]$ ./gradlew bndBugTest --info --console=verbose && shasum TestProject/generated/TestProject.jar 

[elided]

> Task :TestProject:jar
Task ':TestProject:jar' is not up-to-date because:
  Value of input property 'externalProperty' has changed for task ':TestProject:jar'
Generated bundles: [/Users/hargrave/git/tmp/bnd-manifest-bug-test/TestProject/generated/TestProject.jar]
:TestProject:jar (Thread[Execution worker for ':' Thread 7,5,main]) completed. Took 0.025 secs.
:TestProject:bndBugTest (Thread[Execution worker for ':' Thread 7,5,main]) started.

> Task :TestProject:bndBugTest
Task ':TestProject:bndBugTest' is not up-to-date because:
  Input property '$1' file /Users/hargrave/git/tmp/bnd-manifest-bug-test/TestProject/generated/TestProject.jar has changed.
Executed!
:TestProject:bndBugTest (Thread[Execution worker for ':' Thread 7,5,main]) completed. Took 0.002 secs.

BUILD SUCCESSFUL in 0s
3 actionable tasks: 2 executed, 1 up-to-date
d7442736734256cef359edf3548f776565175cd4  TestProject/generated/TestProject.jar
[hargrave@mbp2017 bnd-manifest-bug-test (master)]$ export BND_TEST=4
[hargrave@mbp2017 bnd-manifest-bug-test (master)]$ ./gradlew bndBugTest --info --console=verbose && shasum TestProject/generated/TestProject.jar 

[elided]

> Task :TestProject:jar
Task ':TestProject:jar' is not up-to-date because:
  Value of input property 'externalProperty' has changed for task ':TestProject:jar'
Generated bundles: [/Users/hargrave/git/tmp/bnd-manifest-bug-test/TestProject/generated/TestProject.jar]
:TestProject:jar (Thread[Execution worker for ':',5,main]) completed. Took 0.024 secs.
:TestProject:bndBugTest (Thread[Execution worker for ':',5,main]) started.

> Task :TestProject:bndBugTest UP-TO-DATE
Skipping task ':TestProject:bndBugTest' as it is up-to-date.
:TestProject:bndBugTest (Thread[Execution worker for ':',5,main]) completed. Took 0.001 secs.

BUILD SUCCESSFUL in 0s
3 actionable tasks: 1 executed, 2 up-to-date
ae9fef145200937a9a179657a5d1a22f077c8db2  TestProject/generated/TestProject.jar
[hargrave@mbp2017 bnd-manifest-bug-test (master)]$ 

You can see that gradle ran the jar task both time and the the SHA of the jar generated by Bnd is different. Yet, in the second execution gradle does not declare the bndBugTest task to be out of date.

So to me, this looks like an issue with gradle and not with Bnd since it always makes the jar and the jar's SHA changes.

PS. I moved the build.gradle file from the root into TestProject since it only affects that project and cleaned up the script:

def jarTask = tasks.named('jar') {
  inputs.property('externalProperty', System.getenv('BND_TEST'))
}

tasks.register('bndBugTest') {
  def outputFile = new File(project.buildDir, 'outputFile.txt')
  inputs.files(jarTask)
  outputs.file(outputFile)
  doLast {
    project.delete(outputFile)
    outputFile.createNewFile()
    System.out.println("Executed!")
  }
}
swimmesberger commented 5 years ago

Thanks for the detailed explanation. Is it possible that gradle sometimes tries to not calculate a new hash when the file last modified property is not altered?

Code like this makes me suspicious: https://github.com/gradle/gradle/blob/master/subprojects/core/src/main/java/org/gradle/api/internal/changedetection/state/CachingFileHasher.java#L85

Also if I do something like this:

jarTask.doLast(t -> {
      final Jar jarTask = (Jar) t;
      final File jarFile = jarTask.getArchiveFile().get().getAsFile();
      jarFile.setLastModified(System.currentTimeMillis());
});

It works as expected

bjhargrave commented 5 years ago

I would submit that Gradle is still wrong here according to its own documentation.

https://docs.gradle.org/current/userguide/more_about_tasks.html#sec:how_does_it_work

https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/InputFile.html

Fingerprinting talks about the hash of the file contents and nothing about the timestamp. A changed timestamp could be a way to declare the file changed and avoid the expense of hash computation, but an unchanged timestamp does not mean you can skip hash computation and declare the file unchanged. We see that in the example here, it works as expected for several iterations and then it stops.

swimmesberger commented 5 years ago

I have created an issue in the gradle github project let's see what this leads us to 😃

swimmesberger commented 5 years ago

It seems that gradle is indeed using the "file modification date and file size" before even comparing the hash (Gradle Documentation will be updated). Which brings us back to exactly this issue - we HAVE TO update the last modified date when the MANIFEST.MF file changes to conform to gradles up-to-date checking 🤔

bjhargrave commented 5 years ago

we HAVE TO update the last modified date when the MANIFEST.MF file changes to conform to gradles up-to-date

MANIFEST.MF is generated. Is it not "changed". So under your suggestion, the MANIFEST.MF resource always has the current time as the last modified date and thus the jar always has the current time as the last modified date. This then obviates all the work being done by Jar to capture the last modified date of the resources which contribute to the jar.

swimmesberger commented 5 years ago

Yeah I understand that it might be pretty tricky because the MANIFEST.MF is generated. It might be possible to detect changes between the last MANIFEST.MF and the newly calculated one and only change the lastModified if something has changed but for that to work you would have to know the previous state. I have no idea how easy/difficult that would be to implement in the current bnd codebase.

bjhargrave commented 5 years ago

It might be possible to detect changes between the last MANIFEST.MF and the newly calculated one and only change the lastModified if something has changed but for that to work you would have to know what the previous state.

But we generally do not "know" the previous state. And it seems wrong to add it just for generated resources like the manifest.

bjhargrave commented 5 years ago

I will update Bnd to allow built jars to get the current time. Call Jar.updateModified with TSTAMP value.

rotty3000 commented 5 years ago

@bjhargrave when tackling this please consider this weird logic in the bnd-maven-plugin https://github.com/bndtools/bnd/blob/master/maven/bnd-maven-plugin/src/main/java/aQute/bnd/maven/plugin/BndMavenPlugin.java#L421-L434

bjhargrave commented 5 years ago

when tackling this please consider this weird logic in the bnd-maven-plugin

Since bnd-maven-plugin does not write the final jar (maven-jar-plugin does that), I don't think anything special is needed to maven.

I will not be changing the any of the classes in bndlib regarding lastmodified. There is much code which relies on this. So I will just update the gradle plugin to update the jar file once written.