{"thread":{"id":"4216","subject":"[PATCH 1/2] Move pathspec matching from builtin-add.c into dir.c","startedAt":"2006-05-19T23:07:51Z","lastAt":"2006-05-21T08:20:08Z","messageCount":3,"participants":["Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"20307","messageId":"Pine.LNX.4.64.0605191607010.10823@g5.osdl.org","threadId":"4216","inReplyTo":null,"subject":"[PATCH 1/2] Move pathspec matching from builtin-add.c into dir.c","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-19T23:07:51Z","receivedAt":"2006-05-19T23:07:51Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nI'll use it for builtin-rm.c too.\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 2afb82d..02fe38b 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -13,86 +13,6 @@ #include \"cache-tree.h\"\n static const char builtin_add_usage[] =\n \"git-add [-n] [-v] <filepattern>...\";\n \n-static int common_prefix(const char **pathspec)\n-{\n-\tconst char *path, *slash, *next;\n-\tint prefix;\n-\n-\tif (!pathspec)\n-\t\treturn 0;\n-\n-\tpath = *pathspec;\n-\tslash = strrchr(path, '/');\n-\tif (!slash)\n-\t\treturn 0;\n-\n-\tprefix = slash - path + 1;\n-\twhile ((next = *++pathspec) != NULL) {\n-\t\tint len = strlen(next);\n-\t\tif (len >= prefix && !memcmp(path, next, len))\n-\t\t\tcontinue;\n-\t\tfor (;;) {\n-\t\t\tif (!len)\n-\t\t\t\treturn 0;\n-\t\t\tif (next[--len] != '/')\n-\t\t\t\tcontinue;\n-\t\t\tif (memcmp(path, next, len+1))\n-\t\t\t\tcontinue;\n-\t\t\tprefix = len + 1;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\treturn prefix;\n-}\n-\n-static int match_one(const char *match, const char *name, int namelen)\n-{\n-\tint matchlen;\n-\n-\t/* If the match was just the prefix, we matched */\n-\tmatchlen = strlen(match);\n-\tif (!matchlen)\n-\t\treturn 1;\n-\n-\t/*\n-\t * If we don't match the matchstring exactly,\n-\t * we need to match by fnmatch\n-\t */\n-\tif (strncmp(match, name, matchlen))\n-\t\treturn !fnmatch(match, name, 0);\n-\n-\t/*\n-\t * If we did match the string exactly, we still\n-\t * need to make sure that it happened on a path\n-\t * component boundary (ie either the last character\n-\t * of the match was '/', or the next character of\n-\t * the name was '/' or the terminating NUL.\n-\t */\n-\treturn\tmatch[matchlen-1] == '/' ||\n-\t\tname[matchlen] == '/' ||\n-\t\t!name[matchlen];\n-}\n-\n-static int match(const char **pathspec, const char *name, int namelen, int prefix, char *seen)\n-{\n-\tint retval;\n-\tconst char *match;\n-\n-\tname += prefix;\n-\tnamelen -= prefix;\n-\n-\tfor (retval = 0; (match = *pathspec++) != NULL; seen++) {\n-\t\tif (retval & *seen)\n-\t\t\tcontinue;\n-\t\tmatch += prefix;\n-\t\tif (match_one(match, name, namelen)) {\n-\t\t\tretval = 1;\n-\t\t\t*seen = 1;\n-\t\t}\n-\t}\n-\treturn retval;\n-}\n-\n static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n {\n \tchar *seen;\n@@ -108,7 +28,7 @@ static void prune_directory(struct dir_s\n \ti = dir->nr;\n \twhile (--i >= 0) {\n \t\tstruct dir_entry *entry = *src++;\n-\t\tif (!match(pathspec, entry->name, entry->len, prefix, seen)) {\n+\t\tif (!match_pathspec(pathspec, entry->name, entry->len, prefix, seen)) {\n \t\t\tfree(entry);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/dir.c b/dir.c\nindex d40b62e..d778ecd 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -11,6 +11,86 @@ #include <fnmatch.h>\n #include \"cache.h\"\n #include \"dir.h\"\n \n+int common_prefix(const char **pathspec)\n+{\n+\tconst char *path, *slash, *next;\n+\tint prefix;\n+\n+\tif (!pathspec)\n+\t\treturn 0;\n+\n+\tpath = *pathspec;\n+\tslash = strrchr(path, '/');\n+\tif (!slash)\n+\t\treturn 0;\n+\n+\tprefix = slash - path + 1;\n+\twhile ((next = *++pathspec) != NULL) {\n+\t\tint len = strlen(next);\n+\t\tif (len >= prefix && !memcmp(path, next, len))\n+\t\t\tcontinue;\n+\t\tfor (;;) {\n+\t\t\tif (!len)\n+\t\t\t\treturn 0;\n+\t\t\tif (next[--len] != '/')\n+\t\t\t\tcontinue;\n+\t\t\tif (memcmp(path, next, len+1))\n+\t\t\t\tcontinue;\n+\t\t\tprefix = len + 1;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\treturn prefix;\n+}\n+\n+static int match_one(const char *match, const char *name, int namelen)\n+{\n+\tint matchlen;\n+\n+\t/* If the match was just the prefix, we matched */\n+\tmatchlen = strlen(match);\n+\tif (!matchlen)\n+\t\treturn 1;\n+\n+\t/*\n+\t * If we don't match the matchstring exactly,\n+\t * we need to match by fnmatch\n+\t */\n+\tif (strncmp(match, name, matchlen))\n+\t\treturn !fnmatch(match, name, 0);\n+\n+\t/*\n+\t * If we did match the string exactly, we still\n+\t * need to make sure that it happened on a path\n+\t * component boundary (ie either the last character\n+\t * of the match was '/', or the next character of\n+\t * the name was '/' or the terminating NUL.\n+\t */\n+\treturn\tmatch[matchlen-1] == '/' ||\n+\t\tname[matchlen] == '/' ||\n+\t\t!name[matchlen];\n+}\n+\n+int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen)\n+{\n+\tint retval;\n+\tconst char *match;\n+\n+\tname += prefix;\n+\tnamelen -= prefix;\n+\n+\tfor (retval = 0; (match = *pathspec++) != NULL; seen++) {\n+\t\tif (retval & *seen)\n+\t\t\tcontinue;\n+\t\tmatch += prefix;\n+\t\tif (match_one(match, name, namelen)) {\n+\t\t\tretval = 1;\n+\t\t\t*seen = 1;\n+\t\t}\n+\t}\n+\treturn retval;\n+}\n+\n void add_exclude(const char *string, const char *base,\n \t\t int baselen, struct exclude_list *which)\n {\ndiff --git a/dir.h b/dir.h\nindex 4f65f57..56a1b7f 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -39,6 +39,9 @@ struct dir_struct {\n \tstruct exclude_list exclude_list[3];\n };\n \n+extern int common_prefix(const char **pathspec);\n+extern int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen);\n+\n extern int read_directory(struct dir_struct *, const char *path, const char *base, int baselen);\n extern int excluded(struct dir_struct *, const char *);\n extern void add_excludes_from_file(struct dir_struct *, const char *fname);\n"},{"id":"20308","messageId":"Pine.LNX.4.64.0605191607580.10823@g5.osdl.org","threadId":"4216","inReplyTo":"Pine.LNX.4.64.0605191607010.10823@g5.osdl.org","subject":"[PATCH 2/2] Add builtin \"git rm\" command","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-19T23:19:34Z","receivedAt":"2006-05-19T23:19:34Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThis changes semantics very subtly, because it adds a new atomicity \nguarantee.\n\nIn particular, if you \"git rm\" several files, it will now do all or \nnothing. The old shell-script really looped over the removed files one by \none, and would basically randomly fail in the middle if \"-f\" was used and \none of the files didn't exist in the working directory.\n\nThis C builtin one will not re-write the index after each remove, but \ninstead remove all files at once. However, that means that if \"-f\" is used \n(to also force removal of the file from the working directory), and some \nfiles have already been removed from the workspace, it won't stop in the \nmiddle in some half-way state like the old one did.\n\nSo what happens is that if the _first_ file fails to be removed with \"-f\", \nwe abort the whole \"git rm\". But once we've started removing, we don't \nleave anything half done. If some of the other files don't exist, we'll \njust ignore errors of removal from the working tree.\n\nThis is only an issue with \"-f\", of course.\n\nI think the new behaviour is strictly an improvement, but perhaps more \nimportantly, it is _different_. As a special case, the semantics are \nidentical for the single-file case (which is the only one our test-suite \nseems to test).\n\nThe other question is what to do with leading directories. The old \"git \nrm\" script didn't do anything, which is somewhat inconsistent. This one \nwill actually clean up directories that have become empty as a result of \nremoving the last file, but maybe we want to have a flag to decide the \nbehaviour?\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\n\ndiff --git a/Makefile b/Makefile\nindex 5373986..06b31d8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -120,7 +120,7 @@ SCRIPT_SH = \\\n \tgit-merge-one-file.sh git-parse-remote.sh \\\n \tgit-prune.sh git-pull.sh git-rebase.sh \\\n \tgit-repack.sh git-request-pull.sh git-reset.sh \\\n-\tgit-resolve.sh git-revert.sh git-rm.sh git-sh-setup.sh \\\n+\tgit-resolve.sh git-revert.sh git-sh-setup.sh \\\n \tgit-tag.sh git-verify-tag.sh \\\n \tgit-applymbox.sh git-applypatch.sh git-am.sh \\\n \tgit-merge.sh git-merge-stupid.sh git-merge-octopus.sh \\\n@@ -170,7 +170,8 @@ PROGRAMS = \\\n \n BUILT_INS = git-log$X git-whatchanged$X git-show$X \\\n \tgit-count-objects$X git-diff$X git-push$X \\\n-\tgit-grep$X git-add$X git-rev-list$X git-check-ref-format$X\n+\tgit-grep$X git-add$X git-rm$X git-rev-list$X \\\n+\tgit-check-ref-format$X\n \n # what 'all' will build and 'install' will install, in gitexecdir\n ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS)\n@@ -218,7 +219,8 @@ LIB_OBJS = \\\n \n BUILTIN_OBJS = \\\n \tbuiltin-log.o builtin-help.o builtin-count.o builtin-diff.o builtin-push.o \\\n-\tbuiltin-grep.o builtin-add.o builtin-rev-list.o builtin-check-ref-format.o\n+\tbuiltin-grep.o builtin-add.o builtin-rev-list.o builtin-check-ref-format.o \\\n+\tbuiltin-rm.o\n \n GITLIBS = $(LIB_FILE) $(XDIFF_LIB)\n LIBS = $(GITLIBS) -lz\ndiff --git a/builtin-rm.c b/builtin-rm.c\nnew file mode 100644\nindex 0000000..67e0f79\n--- /dev/null\n+++ b/builtin-rm.c\n@@ -0,0 +1,150 @@\n+/*\n+ * \"git rm\" builtin command\n+ *\n+ * Copyright (C) Linus Torvalds 2006\n+ */\n+#include \"cache.h\"\n+#include \"builtin.h\"\n+#include \"dir.h\"\n+\n+static const char builtin_rm_usage[] =\n+\"git-rm [-n] [-v] [-f] <filepattern>...\";\n+\n+static struct {\n+\tint nr, alloc;\n+\tconst char **name;\n+} list;\n+\n+static void add_list(const char *name)\n+{\n+\tif (list.nr >= list.alloc) {\n+\t\tlist.alloc = alloc_nr(list.alloc);\n+\t\tlist.name = xrealloc(list.name, list.alloc * sizeof(const char *));\n+\t}\n+\tlist.name[list.nr++] = name;\n+}\n+\n+static int remove_file(const char *name)\n+{\n+\tint ret;\n+\tchar *slash;\n+\n+\tret = unlink(name);\n+\tif (!ret && (slash = strrchr(name, '/'))) {\n+\t\tchar *n = strdup(name);\n+\t\tdo {\n+\t\t\tn[slash - name] = 0;\n+\t\t\tname = n;\n+\t\t} while (!rmdir(name) && (slash = strrchr(name, '/')));\n+\t}\n+\treturn ret;\n+}\n+\n+static struct cache_file cache_file;\n+\n+int cmd_rm(int argc, const char **argv, char **envp)\n+{\n+\tint i, newfd;\n+\tint verbose = 0, show_only = 0, force = 0;\n+\tconst char *prefix = setup_git_directory();\n+\tconst char **pathspec;\n+\tchar *seen;\n+\n+\tgit_config(git_default_config);\n+\n+\tnewfd = hold_index_file_for_update(&cache_file, get_index_file());\n+\tif (newfd < 0)\n+\t\tdie(\"unable to create new index file\");\n+\n+\tif (read_cache() < 0)\n+\t\tdie(\"index file corrupt\");\n+\n+\tfor (i = 1 ; i < argc ; i++) {\n+\t\tconst char *arg = argv[i];\n+\n+\t\tif (*arg != '-')\n+\t\t\tbreak;\n+\t\tif (!strcmp(arg, \"--\")) {\n+\t\t\ti++;\n+\t\t\tbreak;\n+\t\t}\n+\t\tif (!strcmp(arg, \"-n\")) {\n+\t\t\tshow_only = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"-v\")) {\n+\t\t\tverbose = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"-f\")) {\n+\t\t\tforce = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tdie(builtin_rm_usage);\n+\t}\n+\tpathspec = get_pathspec(prefix, argv + i);\n+\n+\tseen = NULL;\n+\tif (pathspec) {\n+\t\tfor (i = 0; pathspec[i] ; i++)\n+\t\t\t/* nothing */;\n+\t\tseen = xmalloc(i);\n+\t\tmemset(seen, 0, i);\n+\t}\n+\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tstruct cache_entry *ce = active_cache[i];\n+\t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))\n+\t\t\tcontinue;\n+\t\tadd_list(ce->name);\n+\t}\n+\n+\tif (pathspec) {\n+\t\tconst char *match;\n+\t\tfor (i = 0; (match = pathspec[i]) != NULL ; i++) {\n+\t\t\tif (*match && !seen[i])\n+\t\t\t\tdie(\"pathspec '%s' did not match any files\", match);\n+\t\t}\n+\t}\n+\n+\t/*\n+\t * First remove the names from the index: we won't commit\n+\t * the index unless all of them succeed\n+\t */\n+\tfor (i = 0; i < list.nr; i++) {\n+\t\tconst char *path = list.name[i];\n+\t\tprintf(\"rm '%s'\\n\", path);\n+\n+\t\tif (remove_file_from_cache(path))\n+\t\t\tdie(\"git rm: unable to remove %s\", path);\n+\t}\n+\n+\t/*\n+\t * Then, if we used \"-f\", remove the filenames from the\n+\t * workspace. If we fail to remove the first one, we\n+\t * abort the \"git rm\" (but once we've successfully removed\n+\t * any file at all, we'll go ahead and commit to it all:\n+\t * by then we've already committed ourself and can't fail\n+\t * in the middle)\n+\t */\n+\tif (force) {\n+\t\tint removed = 0;\n+\t\tfor (i = 0; i < list.nr; i++) {  \n+\t\t\tconst char *path = list.name[i];\n+\t\t\tif (!remove_file(path)) {\n+\t\t\t\tremoved = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (!removed)\n+\t\t\t\tdie(\"git rm: %s: %s\", path, strerror(errno));\n+\t\t}\n+\t}\n+\n+\tif (active_cache_changed) {\n+\t\tif (write_cache(newfd, active_cache, active_nr) ||\n+\t\t    commit_index_file(&cache_file))\n+\t\t\tdie(\"Unable to write new index file\");\n+\t}\n+\t\n+\treturn 0;\n+}\ndiff --git a/builtin.h b/builtin.h\nindex 78275ea..d150c7c 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -25,6 +25,7 @@ extern int cmd_count_objects(int argc, c\n \n extern int cmd_push(int argc, const char **argv, char **envp);\n extern int cmd_grep(int argc, const char **argv, char **envp);\n+extern int cmd_rm(int argc, const char **argv, char **envp);\n extern int cmd_add(int argc, const char **argv, char **envp);\n extern int cmd_rev_list(int argc, const char **argv, char **envp);\n extern int cmd_check_ref_format(int argc, const char **argv, char **envp);\ndiff --git a/git-rm.sh b/git-rm.sh\ndeleted file mode 100755\nindex fda4541..0000000\n--- a/git-rm.sh\n+++ /dev/null\n@@ -1,70 +0,0 @@\n-#!/bin/sh\n-\n-USAGE='[-f] [-n] [-v] [--] <file>...'\n-SUBDIRECTORY_OK='Yes'\n-. git-sh-setup\n-\n-remove_files=\n-show_only=\n-verbose=\n-while : ; do\n-  case \"$1\" in\n-    -f)\n-\tremove_files=true\n-\t;;\n-    -n)\n-\tshow_only=true\n-\t;;\n-    -v)\n-\tverbose=--verbose\n-\t;;\n-    --)\n-\tshift; break\n-\t;;\n-    -*)\n-\tusage\n-\t;;\n-    *)\n-\tbreak\n-\t;;\n-  esac\n-  shift\n-done\n-\n-# This is typo-proofing. If some paths match and some do not, we want\n-# to do nothing.\n-case \"$#\" in\n-0)\t;;\n-*)\n-\tgit-ls-files --error-unmatch -- \"$@\" >/dev/null || {\n-\t\techo >&2 \"Maybe you misspelled it?\"\n-\t\texit 1\n-\t}\n-\t;;\n-esac\n-\n-if test -f \"$GIT_DIR/info/exclude\"\n-then\n-\tgit-ls-files -z \\\n-\t--exclude-from=\"$GIT_DIR/info/exclude\" \\\n-\t--exclude-per-directory=.gitignore -- \"$@\"\n-else\n-\tgit-ls-files -z \\\n-\t--exclude-per-directory=.gitignore -- \"$@\"\n-fi |\n-case \"$show_only,$remove_files\" in\n-true,*)\n-\txargs -0 echo\n-\t;;\n-*,true)\n-\txargs -0 sh -c \"\n-\t\twhile [ \\$# -gt 0 ]; do\n-\t\t\tfile=\\$1; shift\n-\t\t\trm -- \\\"\\$file\\\" && git-update-index --remove $verbose \\\"\\$file\\\"\n-\t\tdone\n-\t\" inline\n-\t;;\n-*)\n-\tgit-update-index --force-remove $verbose -z --stdin\n-\t;;\n-esac\ndiff --git a/git.c b/git.c\nindex 7db5cc1..63aa311 100644\n--- a/git.c\n+++ b/git.c\n@@ -51,6 +51,7 @@ static void handle_internal_command(int \n \t\t{ \"count-objects\", cmd_count_objects },\n \t\t{ \"diff\", cmd_diff },\n \t\t{ \"grep\", cmd_grep },\n+\t\t{ \"rm\", cmd_rm },\n \t\t{ \"add\", cmd_add },\n \t\t{ \"rev-list\", cmd_rev_list },\n \t\t{ \"check-ref-format\", cmd_check_ref_format }\n"},{"id":"20380","messageId":"7vpsi7zt53.fsf@assigned-by-dhcp.cox.net","threadId":"4216","inReplyTo":"Pine.LNX.4.64.0605191607580.10823@g5.osdl.org","subject":"Re: [PATCH 2/2] Add builtin \"git rm\" command","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-21T08:20:08Z","receivedAt":"2006-05-21T08:20:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> I think the new behaviour is strictly an improvement, but perhaps more \n> importantly, it is _different_. As a special case, the semantics are \n> identical for the single-file case (which is the only one our test-suite \n> seems to test).\n>\n> The other question is what to do with leading directories. The old \"git \n> rm\" script didn't do anything, which is somewhat inconsistent. This one \n> will actually clean up directories that have become empty as a result of \n> removing the last file, but maybe we want to have a flag to decide the \n> behaviour?\n\nI too think these are improvements.  Thanks for the patch.\n\nBTW, this needed another \"evil merge\" into \"next\", so this time\nI made a separate evil merge branch that I speculated as a\npossibly better alternative approach in an earlier message.\n"}]}