Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-22795

git plugin always cleans workspace, even when instructed not to

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open (View Workflow)
    • Priority: Blocker
    • Resolution: Unresolved
    • Component/s: git-plugin
    • Labels:
      None
    • Similar Issues:

      Description

      Expected behavior: having "Wipe out workspace before build" unchecked would mean that the git plugin would not try to delete files in the workspace before building
      Actual behavior: having "Wipe out workspace before build" unchecked does not affect whether or not the git plugin tries to delete files in the workspace before building

      On version 1.5.0 of the git plugin (and judging by the source code, all versions since), the plugin unconditionally calls deleteContentsRecursive during the clone. This causes problems if the jenkins job running built any artifacts as another user, as they won't be able to be cleaned up. As there are checkboxes to indicate whether or not cleaning should occur, it seems like it would be nice to honor them.

      From src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java, line 366-370:

      try {
          Util.deleteContentsRecursive(workspace);
      } catch (Exception e) {
          e.printStackTrace(listener.error("Failed to clean the workspace"));
          throw new GitException("Failed to delete workspace", e);
      }
      

      When there are files not able to be deleted by jenkins, the following stacktrace is helpfully spewed:

      ERROR: Failed to clean the workspace
      java.nio.file.AccessDeniedException: <REDACTED>
      	at sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)
      	at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
      	at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
      	at sun.nio.fs.UnixFileSystemProvider.implDelete(UnixFileSystemProvider.java:244)
      	at sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:103)
      	at java.nio.file.Files.delete(Files.java:1077)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:606)
      	at hudson.Util.deleteFile(Util.java:239)
      	at hudson.Util.deleteRecursive(Util.java:307)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:222)
      	at org.jenkinsci.plugins.gitclient.AbstractGitAPIImpl.clone(AbstractGitAPIImpl.java:59)
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.clone(CliGitAPIImpl.java:40)
      	at hudson.plugins.git.GitSCM$2.invoke(GitSCM.java:1012)
      	at hudson.plugins.git.GitSCM$2.invoke(GitSCM.java:948)
      	at hudson.FilePath$FileCallableWrapper.call(FilePath.java:2444)
      	at hudson.remoting.UserRequest.perform(UserRequest.java:118)
      	at hudson.remoting.UserRequest.perform(UserRequest.java:48)
      	at hudson.remoting.Request$2.run(Request.java:326)
      	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
      	at java.util.concurrent.FutureTask.run(FutureTask.java:262)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
      	at java.lang.Thread.run(Thread.java:744)
      ERROR: Error cloning remote repo 'origin' : Failed to delete workspace
      hudson.plugins.git.GitException: Failed to delete workspace
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:225)
      	at org.jenkinsci.plugins.gitclient.AbstractGitAPIImpl.clone(AbstractGitAPIImpl.java:59)
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.clone(CliGitAPIImpl.java:40)
      	at hudson.plugins.git.GitSCM$2.invoke(GitSCM.java:1012)
      	at hudson.plugins.git.GitSCM$2.invoke(GitSCM.java:948)
      	at hudson.FilePath$FileCallableWrapper.call(FilePath.java:2444)
      	at hudson.remoting.UserRequest.perform(UserRequest.java:118)
      	at hudson.remoting.UserRequest.perform(UserRequest.java:48)
      	at hudson.remoting.Request$2.run(Request.java:326)
      	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
      	at java.util.concurrent.FutureTask.run(FutureTask.java:262)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
      	at java.lang.Thread.run(Thread.java:744)
      Caused by: java.nio.file.AccessDeniedException: <REDACTED>
      	at sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)
      	at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
      	at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
      	at sun.nio.fs.UnixFileSystemProvider.implDelete(UnixFileSystemProvider.java:244)
      	at sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:103)
      	at java.nio.file.Files.delete(Files.java:1077)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:606)
      	at hudson.Util.deleteFile(Util.java:239)
      	at hudson.Util.deleteRecursive(Util.java:307)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at hudson.Util.deleteRecursive(Util.java:298)
      	at hudson.Util.deleteContentsRecursive(Util.java:204)
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:222)
      	... 13 more
      

        Attachments

          Issue Links

            Activity

            Hide
            sologics Cristian Cureliuc added a comment - - edited

            There should definitely be an option not to delete the workspace when not empty, and even more, this default behavior is quite dangerous!

            The fact that at command line git clone does not work on a non empty directory makes a lot of sense and I was quite surprised to find out that to delete any existing files was in fact the default behavior of the git client plugin.

            After a jenkins and all plugins update including git client plugin, we noticed the following error when trying to run a build on a live project:

            ERROR: Workspace has a .git repository, but it appears to be corrupt.

            followed by a complete wipe out of the workspace. 

            What means "first clone"? That .git is not there? I wonder what happens if somebody specify a wrong directory for the workspace, one or two levels above the intended workspace, would it simply delete everything? That's how it looks from our following tests after this incident on our local server

            I guess that the painful name of such option is not the issue here, more painful is to lose all data and even the risk for that to happen, and not have the option to prevent that.

            Show
            sologics Cristian Cureliuc added a comment - - edited There should definitely be an option not to delete the workspace when not empty, and even more, this default behavior is quite dangerous! The fact that at command line git clone does not work on a non empty directory makes a lot of sense and I was quite surprised to find out that to delete any existing files was in fact the default behavior of the git client plugin. After a jenkins and all plugins update including git client plugin, we noticed the following error when trying to run a build on a live project: ERROR: Workspace has a .git repository, but it appears to be corrupt. followed by a complete wipe out of the workspace.  What means "first clone"? That .git is not there? I wonder what happens if somebody specify a wrong directory for the workspace, one or two levels above the intended workspace, would it simply delete everything? That's how it looks from our following tests after this incident on our local server I guess that the painful name of such option is not the issue here, more painful is to lose all data and even the risk for that to happen, and not have the option to prevent that.
            Hide
            sologics Cristian Cureliuc added a comment -

            Can we please have a reply with the intentions here? We have disabled all our projects waiting for a reaction for fixing this big issue on the git client plugin!

            This kind of problem makes jenkins not interesting for us anymore and we are considering alternate solutions. 

            And I think everybody agrees that the risk to lose all data with absolutely no warning and as default behavior and without even the possibility to disable this is not acceptable. In our case, it was a problem due to a hoster memory limitations which could happen to anybody!

            And since this seems like a very simple thing to fix, we do not really understand the lack of feedback here. Maybe this tells a lot about the interest here.

            Show
            sologics Cristian Cureliuc added a comment - Can we please have a reply with the intentions here? We have disabled all our projects waiting for a reaction for fixing this big issue on the git client plugin! This kind of problem makes jenkins not interesting for us anymore and we are considering alternate solutions.  And I think everybody agrees that the risk to lose all data with absolutely no warning and as default behavior and without even the possibility to disable this is not acceptable. In our case, it was a problem due to a hoster memory limitations which could happen to anybody! And since this seems like a very simple thing to fix, we do not really understand the lack of feedback here. Maybe this tells a lot about the interest here.
            Hide
            markewaite Mark Waite added a comment - - edited

            Cristian Cureliuc I replied here and here. I am not working on this bug currently. I don't expect to work on this bug in the near future.

            I don't like the plugin behavior, but it is a pragmatic behavior, selected due to command line git behavior. When command line git is asked to fetch into an existing directory with existing files, there are many scenarios where command line git will fail the fetch. Rather than fail the fetch, stop the job, and report an error to the user, the git plugin decided many years ago to remove pre-existing files if it detected what appeared to be an incomplete or incorrect initial workspace.

            The earlier comments in the bug report show the difficulty I had duplicating the problem because that code is only executed on the path that creates the initial workspace. With a freestyle job, it can be duplicated by intentionally forcing an incorrect workspace location (bad practice) or by inserting a pre-SCM step which creates a file in the workspace before the git repository is fetched. Pipeline jobs can see the behavior by adding files to the workspace before the git repository is fetched.

            I've intentionally not modified this code. Changing the behavior of the git plugin for this case won't give command line git (or JGit) the ability to reliably fetch into a directory which includes unpredictable content. If the git plugin adds an option to "allow initial fetch to non-empty directory", there will still be use cases which can't be addressed because command line (or JGit) initial fetch fails with certain content in a directory.

            I'm sorry to hear you say:

            This kind of problem makes jenkins not interesting for us anymore and we are considering alternate solutions.

            Unfortunately, if I change the default behavior, I'm confident that I'll discover far more users who depend on the current behavior. They will assert strongly that the job of their continuous integration server is to checkout the code they said to checkout, and to continue behaving the way it did in the past.

            If an optional behavior is added, then that option needs to be selected by the user, and it then needs to be maintained and tested across multiple releases and multiple use cases. I haven't seen enough gain from the proposed optional behavior to justify the complexity and maintenance challenges of adding another optional behavior to the "Advanced checkout" options.

            Show
            markewaite Mark Waite added a comment - - edited Cristian Cureliuc I replied here and here . I am not working on this bug currently. I don't expect to work on this bug in the near future. I don't like the plugin behavior, but it is a pragmatic behavior, selected due to command line git behavior. When command line git is asked to fetch into an existing directory with existing files, there are many scenarios where command line git will fail the fetch. Rather than fail the fetch, stop the job, and report an error to the user, the git plugin decided many years ago to remove pre-existing files if it detected what appeared to be an incomplete or incorrect initial workspace. The earlier comments in the bug report show the difficulty I had duplicating the problem because that code is only executed on the path that creates the initial workspace. With a freestyle job, it can be duplicated by intentionally forcing an incorrect workspace location (bad practice) or by inserting a pre-SCM step which creates a file in the workspace before the git repository is fetched. Pipeline jobs can see the behavior by adding files to the workspace before the git repository is fetched. I've intentionally not modified this code. Changing the behavior of the git plugin for this case won't give command line git (or JGit) the ability to reliably fetch into a directory which includes unpredictable content. If the git plugin adds an option to "allow initial fetch to non-empty directory", there will still be use cases which can't be addressed because command line (or JGit) initial fetch fails with certain content in a directory. I'm sorry to hear you say: This kind of problem makes jenkins not interesting for us anymore and we are considering alternate solutions. Unfortunately, if I change the default behavior, I'm confident that I'll discover far more users who depend on the current behavior. They will assert strongly that the job of their continuous integration server is to checkout the code they said to checkout, and to continue behaving the way it did in the past. If an optional behavior is added, then that option needs to be selected by the user, and it then needs to be maintained and tested across multiple releases and multiple use cases. I haven't seen enough gain from the proposed optional behavior to justify the complexity and maintenance challenges of adding another optional behavior to the "Advanced checkout" options.
            Hide
            sologics Cristian Cureliuc added a comment -

            Hello Mark and thank you for your detailed reply,

            Makes sense of course to assure the stability of the plugin and to take all those aspects into consideration, but I suppose that it should not be such a struggle to add this option to the Advanced options, by default unchecked, for keeping the backwards compatibility and it should simply not allow for that deleteContentsRecursive function to execute under any conditions. Nothing to check across different platforms, future releases, etc.

            How to test this is very easy: we could reproduce this every single time by simply removing the .git metadata directory from the workspace, after adding a flag file for seeing if it survives the build. 

            If you allow me for a suggestion for the the option name, could be: "Prevent workspace cleanup if not empty", again, by default, unchecked.

            I think nobody would mind to have such an option there, hidden under Advanced, and I suppose it would make not only us very happy but also other users which for sure got into this but did not write here because, to be honest, this JIRA ticket was not one of the obvious of google search results, at least for me.

            Please consider this as one of the near future fixes, for us this is really a blocker in using the jenkins CI

            Best regards,

            Cristian

            Show
            sologics Cristian Cureliuc added a comment - Hello Mark and thank you for your detailed reply, Makes sense of course to assure the stability of the plugin and to take all those aspects into consideration, but I suppose that it should not be such a struggle to add this option to the Advanced options, by default unchecked, for keeping the backwards compatibility and it should simply not allow for that deleteContentsRecursive function to execute under any conditions. Nothing to check across different platforms, future releases, etc. How to test this is very easy: we could reproduce this every single time by simply removing the .git metadata directory from the workspace, after adding a flag file for seeing if it survives the build.  If you allow me for a suggestion for the the option name, could be: "Prevent workspace cleanup if not empty", again, by default, unchecked. I think nobody would mind to have such an option there, hidden under Advanced, and I suppose it would make not only us very happy but also other users which for sure got into this but did not write here because, to be honest, this JIRA ticket was not one of the obvious of google search results, at least for me. Please consider this as one of the near future fixes, for us this is really a blocker in using the jenkins CI Best regards, Cristian
            Hide
            markewaite Mark Waite added a comment -

            Cristian Cureliuc I'm willing to review a pull request that implements what you've proposed. I don't plan to implement that myself.

            Show
            markewaite Mark Waite added a comment - Cristian Cureliuc I'm willing to review a pull request that implements what you've proposed. I don't plan to implement that myself.

              People

              • Assignee:
                sologics Cristian Cureliuc
                Reporter:
                haus Matthaus Owens
              • Votes:
                3 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated: