git-commit: Allow partial commit of file removal.

When making a partial commit, git-commit uses git-ls-files with
the --error-unmatch option to expand and sanity check the user
supplied path patterns.  When any path pattern does not match
with the paths known to the index, it errors out, in order to
catch a common mistake to say "git commit Makefiel cache.h"
and end up with a commit that touches only cache.h (notice the
misspelled "Makefile").  This detection however does not work
well when the path has already been removed from the index.

If you drop a path from the index and try to commit that
partially, i.e.

$ git rm COPYING
$ git commit -m 'Remove COPYING' COPYING

the command complains because git does not know anything about
COPYING anymore.

This introduces a new option --with-tree to git-ls-files and
uses it in git-commit when we build a temporary index to
write a tree object for the partial commit.

When --with-tree=<tree-ish> option is specified, names from the
given tree are added to the set of names the index knows about,
so we can treat COPYING file in the example as known.

Of course, there is no reason to use "git rm" and git-aware
people have long time done:

$ rm COPYING
$ git commit -m 'Remove COPYING' COPYING

which works just fine.  But this caused a constant confusion.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index cce17b5ced173ff7ecc1348eb3163609b15982ca..6c1db86e8056285cb25359d55f5ebf9dd0e6f489 100644 (file)
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -9,6 +9,7 @@
 #include "quote.h"
 #include "dir.h"
 #include "builtin.h"
 #include "quote.h"
 #include "dir.h"
 #include "builtin.h"

+#include "tree.h"

 
 static int abbrev;
 static int show_deleted;
 
 static int abbrev;
 static int show_deleted;


@@ -26,6 +27,7 @@ static int prefix_offset;
 static const char **pathspec;
 static int error_unmatch;
 static char *ps_matched;
 static const char **pathspec;
 static int error_unmatch;
 static char *ps_matched;

+static const char *with_tree;

 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";
 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";


@@ -247,6 +249,8 @@ static void show_files(struct dir_struct *dir, const char *prefix)
                                continue;
                        if (show_unmerged && !ce_stage(ce))
                                continue;
                                continue;
                        if (show_unmerged && !ce_stage(ce))
                                continue;

+                       if (ce->ce_flags & htons(CE_UPDATE))
+                               continue;

                        show_ce_entry(ce_stage(ce) ? tag_unmerged : tag_cached, ce);
                }
        }
                        show_ce_entry(ce_stage(ce) ? tag_unmerged : tag_cached, ce);
                }
        }


@@ -332,6 +336,67 @@ static const char *verify_pathspec(const char *prefix)
        return real_prefix;
 }
 
        return real_prefix;
 }
 

+/*
+ * Read the tree specified with --with-tree option
+ * (typically, HEAD) into stage #1 and then
+ * squash them down to stage #0.  This is used for
+ * --error-unmatch to list and check the path patterns
+ * that were given from the command line.  We are not
+ * going to write this index out.
+ */
+static void overlay_tree(const char *tree_name, const char *prefix)
+{
+       struct tree *tree;
+       unsigned char sha1[20];
+       const char **match;
+       struct cache_entry *last_stage0 = NULL;
+       int i;
+
+       if (get_sha1(tree_name, sha1))
+               die("tree-ish %s not found.", tree_name);
+       tree = parse_tree_indirect(sha1);
+       if (!tree)
+               die("bad tree-ish %s", tree_name);
+
+       /* Hoist the unmerged entries up to stage #3 to make room */
+       for (i = 0; i < active_nr; i++) {
+               struct cache_entry *ce = active_cache[i];
+               if (!ce_stage(ce))
+                       continue;
+               ce->ce_flags |= htons(CE_STAGEMASK);
+       }
+
+       if (prefix) {
+               static const char *(matchbuf[2]);
+               matchbuf[0] = prefix;
+               matchbuf [1] = NULL;
+               match = matchbuf;
+       } else
+               match = NULL;
+       if (read_tree(tree, 1, match))
+               die("unable to read tree entries %s", tree_name);
+
+       for (i = 0; i < active_nr; i++) {
+               struct cache_entry *ce = active_cache[i];
+               switch (ce_stage(ce)) {
+               case 0:
+                       last_stage0 = ce;
+                       /* fallthru */
+               default:
+                       continue;
+               case 1:
+                       /*
+                        * If there is stage #0 entry for this, we do not
+                        * need to show it.  We use CE_UPDATE bit to mark
+                        * such an entry.
+                        */
+                       if (last_stage0 &&
+                           !strcmp(last_stage0->name, ce->name))
+                               ce->ce_flags |= htons(CE_UPDATE);
+               }
+       }
+}
+

 static const char ls_files_usage[] =
        "git-ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* "
        "[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "
 static const char ls_files_usage[] =
        "git-ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* "
        "[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "


@@ -452,6 +517,10 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
                        error_unmatch = 1;
                        continue;
                }
                        error_unmatch = 1;
                        continue;
                }

+               if (!prefixcmp(arg, "--with-tree=")) {
+                       with_tree = arg + 12;
+                       continue;
+               }

                if (!prefixcmp(arg, "--abbrev=")) {
                        abbrev = strtoul(arg+9, NULL, 10);
                        if (abbrev && abbrev < MINIMUM_ABBREV)
                if (!prefixcmp(arg, "--abbrev=")) {
                        abbrev = strtoul(arg+9, NULL, 10);
                        if (abbrev && abbrev < MINIMUM_ABBREV)


@@ -503,6 +572,15 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
        read_cache();
        if (prefix)
                prune_cache(prefix);
        read_cache();
        if (prefix)
                prune_cache(prefix);

+       if (with_tree) {
+               /*
+                * Basic sanity check; show-stages and show-unmerged
+                * would not make any sense with this option.
+                */
+               if (show_stage || show_unmerged)
+                       die("ls-files --with-tree is incompatible with -s or -u");
+               overlay_tree(with_tree, prefix);
+       }

        show_files(&dir, prefix);
 
        if (ps_matched) {
        show_files(&dir, prefix);
 
        if (ps_matched) {

diff --git a/git-commit.sh b/git-commit.sh
index 41538f16e5dd8a6dea55933fa32926d758d24ea0..5ea3fd00768dbb5e27ee1b8b9f3285ee35640021 100755 (executable)
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -379,8 +379,11 @@ t,)
                then
                        refuse_partial "Cannot do a partial commit during a merge."
                fi
                then
                        refuse_partial "Cannot do a partial commit during a merge."
                fi

+

                TMP_INDEX="$GIT_DIR/tmp-index$$"
                TMP_INDEX="$GIT_DIR/tmp-index$$"

-               commit_only=`git ls-files --error-unmatch -- "$@"` || exit
+               W=
+               test -z "$initial_commit" && W=--with-tree=HEAD
+               commit_only=`git ls-files --error-unmatch $W -- "$@"` || exit

 
                # Build a temporary index and update the real index
                # the same way.
 
                # Build a temporary index and update the real index
                # the same way.

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 6bd3c9e3e0e1f4433a0b71dc8b930c77736e460d..f178f56208ce84c273fd251c4420a7b6f86c3c86 100644 (file)
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -131,4 +131,25 @@ test_expect_success \
     'validate git-rev-list output.' \
     'diff current expected'
 
     'validate git-rev-list output.' \
     'diff current expected'
 

+test_expect_success 'partial commit that involve removal (1)' '
+
+       git rm --cached file &&
+       mv file elif &&
+       git add elif &&
+       git commit -m "Partial: add elif" elif &&
+       git diff-tree --name-status HEAD^ HEAD >current &&
+       echo "A elif" >expected &&
+       diff expected current
+
+'
+
+test_expect_success 'partial commit that involve removal (2)' '
+
+       git commit -m "Partial: remove file" file &&
+       git diff-tree --name-status HEAD^ HEAD >current &&
+       echo "D file" >expected &&
+       diff expected current
+
+'
+

 test_done
 test_done