Created attachment 219284 [details] [review]
distclean: Use git clean -d -f -x

Many forms of build failure are due to unclean source trees.  The
autotools are particularly susceptible to this.  For example,
previously when one did:

$ jhbuild buildone -afc glib

The jhbuild 'clean' operation ran *after* we ran configure, so
there can easily be stale libraries or binaries lying around
that were in the *old* Makefiles but not the *new* ones.

However, it would be dangerous to change the "clean" operation to be
"git clean -dfx" for developers; if they have unstaged/uncommitted
files lying around, they'll be deleted on a 'jhbuild make -afc'.

Therefore, I've added a new --distclean option.

This should *definitely* be used for tinderboxes.

Review of attachment 219284 [details] [review]:

Thank you for the patch.

What about --distclean option for jhbuild tinderbox?

::: jhbuild/commands/base.py
@@ +129,3 @@
+            make_option('--distclean',
+                        action='store_true', dest='distclean', default=False,
+                        help=_('Completely clean source tree')),

To be consistent, help here doesn't start with a capital leter.

::: jhbuild/modtypes/autotools.py
@@ +308,3 @@
+        if hasattr(self.branch, 'clean'):
+            self.branch.clean(buildscript)
+        elif not self.skip_clean(buildscript, PHASE_CHECKOUT):

This line causes the following error:

+ Trace 230571

Shouldn't be calling self.skip_clean in this manner. Define a self.skip_distclean, or ensure self.skip_clean is called by a method higher in the call stack (skip_phase method maybe)

::: jhbuild/versioncontrol/git.py
@@ +449,3 @@
         Branch.checkout(self, buildscript)
 
+    def clean(self, buildscript):

The method name 'clean' confused me. I first thought this method will run during a clean (-c), but it doesn't. Select a better method name.

@@ +450,3 @@
 
+    def clean(self, buildscript):
+        git_extra_args = {'cwd': os.getcwd(), 'extra_env': get_git_extra_env()}

The use of os.getcwd() is inconsistent with the rest of git.py. The rest of git.py uses self.get_checkoutdir() - and it looks like the right way to do it.

@@ +452,3 @@
+        git_extra_args = {'cwd': os.getcwd(), 'extra_env': get_git_extra_env()}
+        buildscript.execute(['git', 'clean', '-d', '-f', '-x',
+                             self.module], **git_extra_args)

The use of self.module results in the following command:
'git clean -d -f -x http://git.gnome.org/browse/gnome-common'
which probably isn't the command you want.

Created attachment 219682 [details] [review]
distclean: Use git clean -d -f -x

Should address all comments

Review of attachment 219682 [details] [review]:

Thank you for the update. Looks good. It's good for commit.

A minor problem (that can be looked at in a separate commit): if you specify 'jhbuild build -c --distclean', i.e. both clean and distclean, I'd expect JHBuild to only distclean, but with the patch JHBuild only cleans.

JHBuild manual requires an update, but I can handle that.

Attachment 219682 [details] pushed as 9d900c2 - distclean: Use git clean -d -f -x

I committed a further enhancement:
Make --distclean available to jhbuild tinderbox too
http://git.gnome.org/browse/jhbuild/commit/?id=2a192c3582b4e0bb1c2dd6182272138c0b1f9978

the downside is that 'clean' and 'cleanone' options are not consistent now. I guess it's partly because 'cleanone' is in base.py together with other commands, and 'clean' is alone in clean.py and rarely gets updated.