FibreFoX / javafx-gradle-plugin

Gradle plugin for JavaFX
Apache License 2.0
428 stars 59 forks source link

Issue with --daemon switch #12

Closed purringpigeon closed 8 years ago

purringpigeon commented 8 years ago

I am not sure if this is an issue with the plug in - or gradle. However when I run like this:

gradle jfxNative

Everything works like a champ!

when I use it like this: gradle jfxNative --daemon

It throws errors when attempting to copy the files to the lib folder:

Couldn't copy dependency commons-codec:commons-codec:1.6 java.nio.file.NoSuchFileException: C:\Users\user.m2\repository\commons-codec\commons-codec\1.6\commons-codec-1.6.jar -> build\jfx\app\lib\commons-codec-1.6.jar at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:79) at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97) at sun.nio.fs.WindowsFileCopy.copy(WindowsFileCopy.java:205) at sun.nio.fs.WindowsFileSystemProvider.copy(WindowsFileSystemProvider.java:278) at java.nio.file.Files.copy(Files.java:1274) at de.dynamicfiles.projects.gradle.plugins.javafx.tasks.JfxJarTask.lambda$null$1(JfxJarTask.java:123) at java.lang.Iterable.forEach(Iterable.java:75) at de.dynamicfiles.projects.gradle.plugins.javafx.tasks.JfxJarTask.lambda$jfxjar$2(JfxJarTask.java:119) at java.lang.Iterable.forEach(Iterable.java:75) at de.dynamicfiles.projects.gradle.plugins.javafx.tasks.JfxJarTask.jfxjar(JfxJarTask.java:117) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:497) at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:75) at org.gradle.api.internal.project.taskfactory.AnnotationProcessingTaskFactory$StandardTaskAction.doExecute(AnnotationProcessingTaskFactory.java:227) at org.gradle.api.internal.project.taskfactory.AnnotationProcessingTaskFactory$StandardTaskAction.execute(AnnotationProcessingTaskFactory.java:220) at org.gradle.api.internal.project.taskfactory.AnnotationProcessingTaskFactory$StandardTaskAction.execute(AnnotationProcessingTaskFactory.java:209) etc....

Wondering if this a problem with Gradle, or something that needs to be addressed here? So I am posting it. Thanks!

It fails on every jar dependency.

(I have also posted on gradle).

toolforger commented 8 years ago

Gradle's daemon mode is pretty much mandatory, Gradle's startup can take quite a while and daemon mode fixes that. I.e. "killing gradle-daemon manually before building" is not a realistic option unless you can make sure that the jfx build is outside of any fast edit-compile-debug cycle.

Assuming that CI environments do not use daemon mode is, at best, a heuristic.

I have another stop-gap proposal: Restore the clean functionality, but when running in daemon mode and bundling for Windows, issue a warning that this will run into a resource leak. ("Bundling for Windows" may not be the same as "running on Windows" in the long term, BTW.)

FibreFoX commented 8 years ago

@toolforger I can't just "restore" the clean-functionality, because this is a different gradle-task which I'm not involved into. The proposed "solutions" are no real solutions, they are WORKAROUNDS. The real fix would have to be done by Oracle, not me.

This is no normal memory-leak, it is a file-handle lock! Welcome to the world of file-locking on windows-systems. The tooling provided by the JDK does NOT have permanent JVMs in mind, they think of process-termination after the bundling-process, this is a break-up with the paradigm introduced by gradle.

Personally I don't think the daemon mode does that much increase speedup, especially creating bundles should not be seen as "incremental normal development tasks", they are generating some output which has to be executed on the targeted systems after that normal development-phase.

Especially regarding the CI-environment: this is even proposed by the gradle-documentations, that CI-envs should not use daemon-mode.

toolforger commented 8 years ago

Well, somebody is suppressing the clean task and emitting a warning, I've been assuming that it's the javafx Gradle plugin.

Is this really a file locking scenario? Not with that Files.list() call I'd assume: Windows' locking policy, while overly strict, is consistent; so if Files.list() places a lock, then one that prevents just writing, so future Files.list() and open-existing-file-for-reading calls should go through even if a lock from Files.list() leaks. My (unvalidated) assumption is that Files.list() leaks file handles, which would get cleaned up during next GC but that might not happen soon enough.

WRT the speedup: Yes it is substantial. Even my minimal JavaFX test-drive project requires 8 seconds to build the model, and the times used to be much worse though I can't say how much of that is due to project size (those were life-size projects, not the toy I'm testing with right now) and much is because optimizations hadn't happened yet. However, idiocy isn't the key merit of Gradle engineers, so if they think that daemon mode is the way to go, I'm assuming that they know use cases where model building takes a lot longer than 8 seconds, even with today's infrastructure. YMMV.

FibreFoX commented 8 years ago

@toolforger the clean-task is broken after the first jfxNative-build, because (as mentioned inside the reported and accepted bug and inside this "issue-thread") windows is holding some file-handle, which only will released on JVM-termination. This is even described inside Java-Docs:

A DirectoryStream is opened upon creation and is closed by invoking the close method. Closing a directory stream releases any resources associated with the stream. Failure to close the stream may result in a resource leak. The try-with-resources statement provides a useful construct to ensure that the stream is closed

To avoid false-positives (and to make CLEAN work), the current implementation inside my plugin simply does not work when daemon-mode is detected, because they are NOT target of the GC.

WRT the speedup: Yes it is substantial. Even my minimal JavaFX test-drive project requires 8 seconds to build the model, and the times used to be much worse though I can't say how much of that is due to project size (those were life-size projects, not the toy I'm testing with right now) and much is because optimizations hadn't happened yet.

Sorry to sound rude, but I don't care about this. Creating a DISTRIBUTION BUNDLE should not be part of the normal development-cycle, calling jfxJar-task should be used for development and jfxNative as the last part of the whole project-development. I'm just sitting between the contradicting paradigms of gradle vs javapackager, so please don't expect some holy-grail from me.

If you find something new I haven't mentioned inside https://github.com/FibreFoX/javafx-gradle-plugin/issues/12#issuecomment-235292829 please don't hesitate to propose some pull-request.

FibreFoX commented 8 years ago

To all watchers: I might have finally beaten this DAEMON to the ground, and now can eat 🍰

Can you please retry using latest 8.5.3-SNAPSHOT? It should result in downloading javafx-gradle-plugin-8.5.3-20160829.142917-5.jar from sonatype.

Please make sure to have this inside your buildscript for using latest SNAPSHOT-versions:

buildscript {
    dependencies {
        classpath group: 'de.dynamicfiles.projects.gradle.plugins', name: 'javafx-gradle-plugin', version: '8.5.3-SNAPSHOT'
    }
    repositories {
        mavenCentral()
        maven { url "https://oss.sonatype.org/content/repositories/snapshots" }
    }
}

This only works when using gradle daemon mode (which is default on gradle 3), and running on windows using some JDK above and including 1.8.0_60, but ONLY when NOT having runtime: null inside bundleArguments (because this was the use-case not working in the first place, so this is just to recap).

This version includes the adjustments required to fix #30 too.

(If working, I will push the required changes to github)

FibreFoX commented 8 years ago

@purringpigeon can you recheck this?

FibreFoX commented 8 years ago

The fixes provided for this will be released soon.

FibreFoX commented 8 years ago

I'm very happy to finally close this bug! Thanks for all people who supported me on this one !

eskatos commented 8 years ago

Awesome! 👍