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

Clean After Checkout Results in Failed to Checkout Revision

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • git-plugin
    • None

      We're executing a job, and trying to perform a git clean. One of our submodules is dirty, and unfortunately since the Jenkins GIT plugin cleans after checkout, we get the following error:

      FATAL: Command "git submodule update --init --recursive" returned status code 1:
      stdout:
      stderr: error: Your local changes to the following files would be overwritten by checkout:
      minified/sidecar.js
      minified/sidecar.lite.js
      minified/sidecar.lite.min.js
      minified/sidecar.min.js
      Please, commit your changes or stash them before you can switch branches.
      Aborting
      Unable to checkout 'd58c3304230e4bf26ffb3fa6986d4b6ba90d4c66' in submodule path 'sugarcrm/sidecar'

      I tested using the Pre-SCM Buildstep plugin and performing a git clean before the checkout fixes the problem. Specifically having it perform:
      git submodule foreach --recursive git clean -fdx
      git submodule foreach --recursive git reset --hard

      before we do the checkout fixes it.

          [JENKINS-22510] Clean After Checkout Results in Failed to Checkout Revision

          Ray Sennewald added a comment - - edited

          Mark, I appreciate your willingness to help us out here. TBH, I don't feel that my situation is unique to submodules. It appears to me that the problem would arise anytime that you have a conflict of a file between what you have in your working copy, and what your trying to checkout. I'd love to be able to add some tests around this, but have been quite busy at work in other regards lately. My 2 cents are, do we really need clean after checkout? Is there any situation that this behavior is needed, that can't be taken care of by doing a clean before checkout? Perhaps we should change Clean After Checkout to be called Clean Before Checkout, and change its behavior to do what we implemented in clean before checkout? What do you think? I'm more than happy to have David or I help out here.

          I should mention, what worked for us was the correct order of operations. We didn't need a git clean -ffdx, we just needed git clean -fdx and git reset --hard to be executed on all submodules BEFORE we do a checkout on those submodules.

          Ray Sennewald added a comment - - edited Mark, I appreciate your willingness to help us out here. TBH, I don't feel that my situation is unique to submodules. It appears to me that the problem would arise anytime that you have a conflict of a file between what you have in your working copy, and what your trying to checkout. I'd love to be able to add some tests around this, but have been quite busy at work in other regards lately. My 2 cents are, do we really need clean after checkout? Is there any situation that this behavior is needed, that can't be taken care of by doing a clean before checkout? Perhaps we should change Clean After Checkout to be called Clean Before Checkout, and change its behavior to do what we implemented in clean before checkout? What do you think? I'm more than happy to have David or I help out here. I should mention, what worked for us was the correct order of operations. We didn't need a git clean -ffdx, we just needed git clean -fdx and git reset --hard to be executed on all submodules BEFORE we do a checkout on those submodules.

          Mark Waite added a comment -

          I'm somewhat fanatical about backward compatibility, and removing "Clean after checkout" (or mutating it to be "Clean before checkout") would risk all sorts of compatibility problems. The git plugin (and the git client plugin) are two of the most used Jenkins plugins. Many users depend on them to remain compatible from one version to the next, and to retain their old behavior (no matter how odd that behavior may appear).

          I'm glad the "git clean -fdx" was sufficient, since that retains compatibility with the existing code and existing job definitions.

          Mark Waite added a comment - I'm somewhat fanatical about backward compatibility, and removing "Clean after checkout" (or mutating it to be "Clean before checkout") would risk all sorts of compatibility problems. The git plugin (and the git client plugin) are two of the most used Jenkins plugins. Many users depend on them to remain compatible from one version to the next, and to retain their old behavior (no matter how odd that behavior may appear). I'm glad the "git clean -fdx" was sufficient, since that retains compatibility with the existing code and existing job definitions.

          Code changed in jenkins
          User: David S Wang
          Path:
          src/main/java/hudson/plugins/git/extensions/impl/CleanBeforeCheckout.java
          src/main/resources/hudson/plugins/git/extensions/impl/CleanBeforeCheckout/help.html
          src/test/java/hudson/plugins/git/AbstractGitTestCase.java
          src/test/java/hudson/plugins/git/GitSCMTest.java
          http://jenkins-ci.org/commit/git-plugin/fb12fed4c1c8af11db68ec236961401d3d11eb7b
          Log:
          Files/changes added to allow clean before checkout resolving JENKINS-22510

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: David S Wang Path: src/main/java/hudson/plugins/git/extensions/impl/CleanBeforeCheckout.java src/main/resources/hudson/plugins/git/extensions/impl/CleanBeforeCheckout/help.html src/test/java/hudson/plugins/git/AbstractGitTestCase.java src/test/java/hudson/plugins/git/GitSCMTest.java http://jenkins-ci.org/commit/git-plugin/fb12fed4c1c8af11db68ec236961401d3d11eb7b Log: Files/changes added to allow clean before checkout resolving JENKINS-22510

          Code changed in jenkins
          User: Mark Waite
          Path:
          src/main/java/hudson/plugins/git/extensions/impl/CleanBeforeCheckout.java
          src/main/resources/hudson/plugins/git/extensions/impl/CleanBeforeCheckout/help.html
          src/test/java/hudson/plugins/git/AbstractGitTestCase.java
          src/test/java/hudson/plugins/git/GitSCMTest.java
          http://jenkins-ci.org/commit/git-plugin/743d715f21183e473adae4b4d969f3ccf16d3578
          Log:
          Merge branch 'master' of https://github.com/ds2wang/git-plugin

          [Fixed JENKINS-7376] Clean after checkout cleans submodules
          [Fixed JENKINS-13910] Clean operates on submodules
          [Fixed JENKINS-22510] Clean after checkout causes checkout failure

          Compare: https://github.com/jenkinsci/git-plugin/compare/fd2bc21c8951...743d715f2118

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Mark Waite Path: src/main/java/hudson/plugins/git/extensions/impl/CleanBeforeCheckout.java src/main/resources/hudson/plugins/git/extensions/impl/CleanBeforeCheckout/help.html src/test/java/hudson/plugins/git/AbstractGitTestCase.java src/test/java/hudson/plugins/git/GitSCMTest.java http://jenkins-ci.org/commit/git-plugin/743d715f21183e473adae4b4d969f3ccf16d3578 Log: Merge branch 'master' of https://github.com/ds2wang/git-plugin [Fixed JENKINS-7376] Clean after checkout cleans submodules [Fixed JENKINS-13910] Clean operates on submodules [Fixed JENKINS-22510] Clean after checkout causes checkout failure Compare: https://github.com/jenkinsci/git-plugin/compare/fd2bc21c8951...743d715f2118

          Mark Waite added a comment -

          Fix will be available in next release after git plugin 2.2.0

          Mark Waite added a comment - Fix will be available in next release after git plugin 2.2.0

          mcrooney added a comment -

          "Relying on the user to know when they should select "clean before checkout" and when they should "clean after checkout" seems like we're expecting too much from the users."

          Totally, I agree, I ended up having to search on Google and then read through this bug report, to figure out what the difference was, and I'm still not sure I understand what the difference would be using one or the other. The root of this problem is that the help texts for these checkboxes are exactly the same except for the word "before" and "after" I'm happy to file an issue for improving the help texts if that seems reasonable?

          mcrooney added a comment - "Relying on the user to know when they should select "clean before checkout" and when they should "clean after checkout" seems like we're expecting too much from the users." Totally, I agree, I ended up having to search on Google and then read through this bug report, to figure out what the difference was, and I'm still not sure I understand what the difference would be using one or the other. The root of this problem is that the help texts for these checkboxes are exactly the same except for the word "before" and "after" I'm happy to file an issue for improving the help texts if that seems reasonable?

          Mark Waite added a comment -

          That would be fine. I'm not sure how you'll make the help text describe the things you have learned, but you'll make my life easier evaluating your proposed changes if you submit your proposed change as a pull request. You can do that by:

          Mark Waite added a comment - That would be fine. I'm not sure how you'll make the help text describe the things you have learned, but you'll make my life easier evaluating your proposed changes if you submit your proposed change as a pull request. You can do that by: Create a free GitHub account for yourself Fork the git plugin repository with the "Fork" button on the left of the repo Edit the help text (from the web interface is easy enough) in your forked copy of the repository ( https://github.com/YourUserNameGoesHere/git-plugin/blob/master/src/main/resources/hudson/plugins/git/extensions/impl/CleanBeforeCheckout/help.html and https://github.com/YourUserNameGoesHere/git-plugin/blob/master/src/main/resources/hudson/plugins/git/extensions/impl/CleanCheckout/help.html ) Save your changes (from the web Submit a pull request (using the GitHub submit pull request from the web)

          mcrooney added a comment -

          Thanks Mark! I guess my takeaway from this is: "clean after was originally the only cleaning behavior, is the only one offered by job-dsl, and should generally work. Use clean before if clean after isn't working for you." Though, that's pretty ambiguous. I still don't totally understand myself, except that that understanding was enough for me to decide to use "after". What does "after" solve that "before" doesn't? Is "before" strictly better?

          mcrooney added a comment - Thanks Mark! I guess my takeaway from this is: "clean after was originally the only cleaning behavior, is the only one offered by job-dsl, and should generally work. Use clean before if clean after isn't working for you." Though, that's pretty ambiguous. I still don't totally understand myself, except that that understanding was enough for me to decide to use "after". What does "after" solve that "before" doesn't? Is "before" strictly better?

          Mark Waite added a comment -

          One is not better than the other (as far as I can tell). You're correct that "Clean after checkout" was the first clean behavior, and was implemented very early in the development of the plugin.

          The specific benefit sought by the original submitter of the bug report was to better handle clean after a recursive submodule checkout had left the repository in a "dirty" state. In the submodule case, performing the clean before the checkout seems to have a better chance of succeeding than performing the clean after the checkout. I assume one of the reasons for a better chance of success is that the recursive submodule checkout may fail due to the "dirty" state of the working directory, which then might prevent the "clean after" from even being executed.

          I assume the job DSL includes one and not the other because it was initially created when "clean after" was the only option available.

          Mark Waite added a comment - One is not better than the other (as far as I can tell). You're correct that "Clean after checkout" was the first clean behavior, and was implemented very early in the development of the plugin. The specific benefit sought by the original submitter of the bug report was to better handle clean after a recursive submodule checkout had left the repository in a "dirty" state. In the submodule case, performing the clean before the checkout seems to have a better chance of succeeding than performing the clean after the checkout. I assume one of the reasons for a better chance of success is that the recursive submodule checkout may fail due to the "dirty" state of the working directory, which then might prevent the "clean after" from even being executed. I assume the job DSL includes one and not the other because it was initially created when "clean after" was the only option available.

          Code changed in jenkins
          User: Mark Waite
          Path:
          src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestCase.java
          http://jenkins-ci.org/commit/git-client-plugin/eec1a16bc7ddff1ffd022f5c9e5f631f1a63f329
          Log:
          Add two detailed submodule tests

          Tests describe the implementation as it currently exists, in hopes
          of detecting future regressions with test execution. The tests show
          inconsistencies between the CliGitAPIImpl and JGitAPIImpl classes,
          and inconsistencies between command line git and JGit behavior.

          Command line git clean as implemented in CliGitAPIImpl does not remove
          untracked submodules or files contained in untracked submodule dirs.
          JGit clean as implemented in JGitAPIImpl removes untracked submodules.
          This test captures that surprising difference between the implementations.

          CliGitAPIImpl supports renamed submodules. JGitAPIImpl does not support
          renamed submodules. One of these tests captures that difference.

          See bug reports such as:
          JENKINS-22510 - Clean After Checkout Results in Failed to Checkout Revision
          JENKINS-8053 - Git submodules are cloned too early and not removed once the revToBuild has been checked out
          JENKINS-14083 - Build can't recover from broken submodule path
          JENKINS-15399 - Changing remote URL doesn't update submodules

          Compare: https://github.com/jenkinsci/git-client-plugin/compare/cdcbfd56f49f...eec1a16bc7dd

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Mark Waite Path: src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestCase.java http://jenkins-ci.org/commit/git-client-plugin/eec1a16bc7ddff1ffd022f5c9e5f631f1a63f329 Log: Add two detailed submodule tests Tests describe the implementation as it currently exists, in hopes of detecting future regressions with test execution. The tests show inconsistencies between the CliGitAPIImpl and JGitAPIImpl classes, and inconsistencies between command line git and JGit behavior. Command line git clean as implemented in CliGitAPIImpl does not remove untracked submodules or files contained in untracked submodule dirs. JGit clean as implemented in JGitAPIImpl removes untracked submodules. This test captures that surprising difference between the implementations. CliGitAPIImpl supports renamed submodules. JGitAPIImpl does not support renamed submodules. One of these tests captures that difference. See bug reports such as: JENKINS-22510 - Clean After Checkout Results in Failed to Checkout Revision JENKINS-8053 - Git submodules are cloned too early and not removed once the revToBuild has been checked out JENKINS-14083 - Build can't recover from broken submodule path JENKINS-15399 - Changing remote URL doesn't update submodules Compare: https://github.com/jenkinsci/git-client-plugin/compare/cdcbfd56f49f...eec1a16bc7dd

            ndeloof Nicolas De Loof
            rsennewald Ray Sennewald
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: