Uploaded image for project: 'Gradle'
  1. Gradle
  2. GRADLE-2579

SourceTask is always up-to-date after all source files are deleted

    Details

    • Type: Bug
    • Status: Resolved
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 3.2-rc-1
    • Gradle Forums topic Reference:

      Description

        1. Problem

      Gradle up-to-date check does not take into account deleted files. This has been confirmed for Gradle 1.3 with 'java' and 'groovy' plugins.

        1. Steps to reproduce:

      1. Create a build.gradle containing apply plugin: 'java'
      2. Create the directories src/main/java
      3. Create a file src/main/java/Foo.java containing public class Foo {}
      4. Execute gradle build - compileJava will be executed.
      5. Execute gradle build - compileJava will be UP-TO-DATE.
      6. Delete the file src/main/java/Foo.java.
      7. Execute gradle build - compileJava will still be UP-TO-DATE.

        1. Result
          Because of this, the deleted class will still be available in both build/classes/main and the created jar in build/lib.

        Issue Links

          Activity

          Hide
          jhuxhorn Joern Huxhorn added a comment -

          This will be a showstopper for Gradle 3.0 since it sabotages a 100% reliable cache, right?
          (According to http://www.infoq.com/presentations/future-gradle-javascript )

          Show
          jhuxhorn Joern Huxhorn added a comment - This will be a showstopper for Gradle 3.0 since it sabotages a 100% reliable cache, right? (According to http://www.infoq.com/presentations/future-gradle-javascript )
          Hide
          jhuxhorn Joern Huxhorn added a comment -

          Every few month, I decide to take a dive into the Gradle source code to see if I can find a way to fix this elegantly.

          This time around, I realized that the current behavior is actually documented in the public API so fixing the problem isn't just a patch but rather a political decision.

          So instead of trying to fix the problem itself I did the next best thing and wrote an integration test for the issue:

          StaleOutputTest.groovy
          package org.gradle.integtests
          
          import org.gradle.integtests.fixtures.AbstractIntegrationSpec
          import spock.lang.Issue
          
          class StaleOutputTest extends AbstractIntegrationSpec {
              @Issue(['GRADLE-2440', 'GRADLE-2579'])
              def 'Stale output is removed after input source directory is emptied.'() {
                  setup: 'a minimal java build'
                  buildScript("apply plugin: 'java'")
                  def fooJavaFile = file('src/main/java/Foo.java') << 'public class Foo {}'
                  def fooClassFile = file('build/classes/main/Foo.class')
          
                  when: 'a build is executed'
                  succeeds('clean', 'build')
          
                  then: 'class file exists as expected'
                  fooClassFile.exists()
          
                  when: 'only java source file is deleted, leaving empty input source directory'
                  fooJavaFile.delete()
          
                  and: 'another build is executed'
                  succeeds('build')
          
                  then: 'source file was actually deleted'
                  !fooClassFile.exists()
              }
          }
          

          This test confirmed my suspicion that simply removing SkipEmptySourceFilesTaskExecuter in the TaskExecutionServices.createTaskExecuter method wouldn't magically fix this problem.

          Doing so causes a java.lang.IllegalStateException: no source files in com.sun.tools.javac.main.Main - and this is just the special case of java compilation. Who knows which other tools might strictly require actual input files instead of defaulting to NOP in that case?

          Skipping tasks without input source files is indeed the correct behavior and getting rid of previously created output simply hasn't been adressed so far for this special use case.

          Tasks usually remove stale files on their own if they realize it's necessary (e.g. StaleClassCleaner) but I'm not aware of a general way to tell a task that it should just get rid of any previous output instead of executing its originally intended functionality.

          Talking about skipping...

          I'd suggest to change the line state.upToDate(); in SkipEmptySourceFilesTaskExecuter to state.skipped("SKIPPED"); regardless of any other fixes. Because that's what's happening.

          Well... not what I hoped for... but I hope this helps at least a little bit in getting this fixed before the Gradle 3.0 release.

          Show
          jhuxhorn Joern Huxhorn added a comment - Every few month, I decide to take a dive into the Gradle source code to see if I can find a way to fix this elegantly. This time around, I realized that the current behavior is actually documented in the public API so fixing the problem isn't just a patch but rather a political decision. So instead of trying to fix the problem itself I did the next best thing and wrote an integration test for the issue: StaleOutputTest.groovy package org.gradle.integtests import org.gradle.integtests.fixtures.AbstractIntegrationSpec import spock.lang.Issue class StaleOutputTest extends AbstractIntegrationSpec { @Issue(['GRADLE-2440', 'GRADLE-2579']) def 'Stale output is removed after input source directory is emptied.'() { setup: 'a minimal java build' buildScript( "apply plugin: 'java'" ) def fooJavaFile = file('src/main/java/Foo.java') << ' public class Foo {}' def fooClassFile = file('build/classes/main/Foo.class') when: 'a build is executed' succeeds('clean', 'build') then: 'class file exists as expected' fooClassFile.exists() when: 'only java source file is deleted, leaving empty input source directory' fooJavaFile.delete() and: 'another build is executed' succeeds('build') then: 'source file was actually deleted' !fooClassFile.exists() } } This test confirmed my suspicion that simply removing SkipEmptySourceFilesTaskExecuter in the TaskExecutionServices.createTaskExecuter method wouldn't magically fix this problem. Doing so causes a java.lang.IllegalStateException: no source files in com.sun.tools.javac.main.Main - and this is just the special case of java compilation. Who knows which other tools might strictly require actual input files instead of defaulting to NOP in that case? Skipping tasks without input source files is indeed the correct behavior and getting rid of previously created output simply hasn't been adressed so far for this special use case. Tasks usually remove stale files on their own if they realize it's necessary (e.g. StaleClassCleaner ) but I'm not aware of a general way to tell a task that it should just get rid of any previous output instead of executing its originally intended functionality. Talking about skipping... I'd suggest to change the line state.upToDate(); in SkipEmptySourceFilesTaskExecuter to state.skipped("SKIPPED"); regardless of any other fixes. Because that's what's happening. Well... not what I hoped for... but I hope this helps at least a little bit in getting this fixed before the Gradle 3.0 release.
          Hide
          jhuxhorn Joern Huxhorn added a comment - - edited

          I think I fixed this issue. Pull request

          Only change in behavior beside fixing the original issue is that a task without source files is now reporting SKIPPED instead of UP-TO-DATE.

          Please also take a look at this comment in SkipEmptySourceFilesTaskExecuter.

          I was unsure if this type of cleanup would justify a different skipped message, e.g. CLEANED, but decided to leave it out for now, using SKIPPED as usual.

          Show
          jhuxhorn Joern Huxhorn added a comment - - edited I think I fixed this issue. Pull request Only change in behavior beside fixing the original issue is that a task without source files is now reporting SKIPPED instead of UP-TO-DATE . Please also take a look at this comment in SkipEmptySourceFilesTaskExecuter . I was unsure if this type of cleanup would justify a different skipped message, e.g. CLEANED , but decided to leave it out for now, using SKIPPED as usual.
          Hide
          jhuxhorn Joern Huxhorn added a comment -

          Please consider taking a look at the pull request before going 3.0-rc.

          Show
          jhuxhorn Joern Huxhorn added a comment - Please consider taking a look at the pull request before going 3.0-rc.
          Hide
          lptr Lóránt Pintér added a comment -

          Now tasks clean up their previous output after all their sources are gone.

          Show
          lptr Lóránt Pintér added a comment - Now tasks clean up their previous output after all their sources are gone.

            People

            • Assignee:
              lptr Lóránt Pintér
              Reporter:
              forums Gradle Forums
            • Votes:
              21 Vote for this issue
              Watchers:
              25 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development