David Wang has implemented functionality to Clean Before Checkout, which actually helps fix this scenario. His pull request is here: https://github.com/jenkinsci/git-plugin/pull/220.

Considering the number of cases related to this topic over the course of the evolution of the git plugin, I'd very much prefer that we have a very deep set of tests created which will assure that submodules behave as desired. Some of the related bugs (JENKINS-8315, JENKINS-7445) are listed as fixed, yet those fixes either did not survive, or do not meet your need. I'd really like more tests that express the failure cases, since the git-client-plugin has very few submodule tests.

Ultimately, we may need a "clean before checkout" operation, but it seems like we should first invest effort to create unit tests which will assure that submodules are behaving as expected in the git client plugin (and possibly in the git plugin).

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. I would very much prefer to find a sequence which avoids requiring the addition of yet another form of clean. We have "wipe workspace" which removes everything, we have "clean after checkout", and this will add "clean before checkout".

Since you and David are active users of submodules, would you be willing to provide one or more test cases which illustrate this failure mode, then use that test case to evaluate alternatives to fix the test?

As an example of an alternative, would the "git clean -ffxd" command (double f arguments clean untracked directories managed by another repository) resolve the issue without requiring the addition of a new command?

I've been trying to understand the sequence of git operations which arrive at that state described in the bug report, and haven't yet understood how to do that. My attempts have persuaded me that I don't understand submodules. I used my fork of the git-client-plugin for my experiments.

The failing sequence I see currently is:

• checkout master - ok
• checkout tests/notSubmodules - ok
• checkout tests/getSubmodules - ok
• checkout tests/notSubmodules - fails

I defined a bash shell function "checkout" for my testing convenience. It is

checkout ()
{
    branch=$1;
    git checkout -f $branch;
    git clean -xffd;
    git submodule update --init --recursive;
    git status
}

Clone the repository - works

git clone git://github.com/MarkEWaite/git-client-plugin
cd git-client-plugin
ls modules/ntp # reports an error, no content in modules/ntp

Checkout tests/notSubmodules - works

checkout tests/notSubmodules

Checkout tests/getSubmodules - works

checkout tests/getSubmodules

Checkout tests/notSubmodules - works

checkout tests/notSubmodules

Checkout tests/getSubmodules - fails

checkout tests/getSubmodules

The "mostly" working sequence in my case seems to be "force checkout, git clean -xffd, submodule update". That seems to work for all the cases I've tested except the master to notSubmodules to getSubmodules to notSubmodules to getSubmodules case. However, since my case does not duplicate your message, I'm not sure if you'll see the same behavior as I see.

In that repository, the branch to branch transitions seem to behave like this:

However, the failure message I'm seeing is not the same as the failure message you're seeing. I'm confident that means I don't understand enough about submodules. Can you provide a similar series of steps (preferably using a fork of the git-client-plugin repository) to show your case?

If you're interested in my first attempt to express that failure mode as a unit test, refer to the proto-submodule-tests branch in my fork of the git-client-plugin source. The test is "not yet ready for prime time".

I think git.clean() can't be switched method to always use "clean -xffd" instead of "clean -xfd" because that may damage existing use cases which depend on the current behavior.

I haven't been able to find any better solution than the solution you've proposed. I agree with your recommendation that we should add "Clean before checkout" as an additional behavior. We'll rely on users to choose which ever of the two best meets the need of their use case.

I think we'll still need the switch branch of submodule before checkout pull request as well, but I think that "Clean before checkout" is also a needed addition to the git plugin.

Over the course of the next week, I'll perform some additional tests to better understand the impact of the change.

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.

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

Fix will be available in next release after git plugin 2.2.0

"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?

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)

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?

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