{"thread":{"id":"6053","subject":"[RFH/RFT] git-rm: update to saner semantics","startedAt":"2006-12-21T09:18:13Z","lastAt":"2006-12-21T09:38:52Z","messageCount":2,"participants":["Junio C Hamano","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"29919","messageId":"7vwt4lboa2.fsf@assigned-by-dhcp.cox.net","threadId":"6053","inReplyTo":null,"subject":"[RFH/RFT] git-rm: update to saner semantics","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-21T09:18:13Z","receivedAt":"2006-12-21T09:18:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This updates the \"git rm\" command with saner semantics suggested\non the list earlier with:\n\n\tMessage-ID: \n\tMessage-ID: \n\nThe command still validates that the given paths all talk about\nsensible paths to avoid mistakes (e.g. \"git rm fiel\" when the\nuser meant to remove \"file\" would error out), and it has further\nsafety checks described next. The biggest difference is that\nthe paths are removed from both index and from the working tree\n(if you have an exotic need to remove paths only from the index,\nyou can use the --cached option).\n\nThe command refuses to remove if the copy on the working tree\ndoes not match the index, or if the index and the HEAD does not\nmatch. You can defeat this check with -f option.\n\nThis safety check has two exceptions: if the working tree file\ndoes not exist to begin with, that technically does not match\nthe index but it is allowed. This is to allow this CVS style\ncommand sequence:\n\n\trm && git rm \n\nAlso if the index is unmerged at the , you can use \"git rm\n\" to declare that the result of the merge loses that path,\nand the above safety check does not trigger; requiring the file\nto match the index in this case forces the user to do \"git\nupdate-index file && git rm file\", which is just crazy.\n\n---\n\n * This has seen only very limited test, so testing, adjusting the\n test case t/t3600-rm.sh (possibly others) to the updated\n semantics and documentation updates are still needed.\n\n Also it lacks the \"require -r for recursive behaviour\" safety\n valve. I think that is a sane thing to add.\n\n Volunteers?\n\n builtin-rm.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++----------\n 1 files changed, 85 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin-rm.c b/builtin-rm.c\nindex 33d04bd..3927d93 100644\n--- a/builtin-rm.c\n+++ b/builtin-rm.c\n@@ -9,7 +9,7 @@\n #include \"cache-tree.h\"\n \n static const char builtin_rm_usage[] =\n-\"git-rm [-n] [-v] [-f] ...\";\n+\"git-rm [-n] [-f] [--cached] ...\";\n \n static struct {\n \tint nr, alloc;\n@@ -41,12 +41,66 @@ static int remove_file(const char *name)\n \treturn ret;\n }\n \n+static int check_local_mod(unsigned char *head)\n+{\n+\t/* items in list are already sorted in the cache order,\n+\t * so we could do this a lot more efficiently by using\n+\t * tree_desc based traversal if we wanted to, but I am\n+\t * lazy, and who cares if removal of files is a tad slower\n+\t * than theoretical maximum speed?\n+\t */\n+\tint i, no_head;\n+\tint errs = 0;\n+\n+\tno_head = is_null_sha1(head);\n+\tfor (i = 0; i < list.nr; i++) {\n+\t\tstruct stat st;\n+\t\tint pos;\n+\t\tstruct cache_entry *ce;\n+\t\tconst char *name = list.name[i];\n+\t\tunsigned char sha1[20];\n+\t\tunsigned mode;\n+\n+\t\tpos = cache_name_pos(name, strlen(name));\n+\t\tif (pos < 0)\n+\t\t\tcontinue; /* removing unmerged entry */\n+\t\tce = active_cache[pos];\n+\n+\t\tif (lstat(ce->name, &st) < 0) {\n+\t\t\tif (errno != ENOENT)\n+\t\t\t\tfprintf(stderr, \"warning: '%s': %s\",\n+\t\t\t\t\tce->name, strerror(errno));\n+\t\t\t/* It already vanished from the working tree */\n+\t\t\tcontinue;\n+\t\t}\n+\t\telse if (S_ISDIR(st.st_mode)) {\n+\t\t\t/* if a file was removed and it is now a\n+\t\t\t * directory, that is the same as ENOENT as\n+\t\t\t * far as git is concerned; we do not track\n+\t\t\t * directories.\n+\t\t\t */\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (ce_match_stat(ce, &st, 0))\n+\t\t\terrs = error(\"'%s' has local modifications \"\n+\t\t\t\t \"(hint: try -f)\", ce->name);\n+\t\tif (no_head)\n+\t\t\tcontinue;\n+\t\tget_tree_entry(head, name, sha1, &mode);\n+\t\tif (ce->ce_mode != create_ce_mode(mode) ||\n+\t\t hashcmp(ce->sha1, sha1))\n+\t\t\terrs = error(\"'%s' has changes staged in the index \"\n+\t\t\t\t \"(hint: try -f)\", name);\n+\t}\n+\treturn errs;\n+}\n+\n static struct lock_file lock_file;\n \n int cmd_rm(int argc, const char **argv, const char *prefix)\n {\n \tint i, newfd;\n-\tint verbose = 0, show_only = 0, force = 0;\n+\tint show_only = 0, force = 0, index_only = 0;\n \tconst char **pathspec;\n \tchar *seen;\n \n@@ -62,23 +116,18 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \n \t\tif (*arg != '-')\n \t\t\tbreak;\n-\t\tif (!strcmp(arg, \"--\")) {\n+\t\telse if (!strcmp(arg, \"--\")) {\n \t\t\ti++;\n \t\t\tbreak;\n \t\t}\n-\t\tif (!strcmp(arg, \"-n\")) {\n+\t\telse if (!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\telse if (!strcmp(arg, \"--cached\"))\n+\t\t\tindex_only = 1;\n+\t\telse if (!strcmp(arg, \"-f\"))\n \t\t\tforce = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tusage(builtin_rm_usage);\n+\t\telse\n+\t\t\tusage(builtin_rm_usage);\n \t}\n \tif (argc <= i)\n \t\tusage(builtin_rm_usage);\n@@ -105,8 +154,26 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/*\n+\t * If not forced, the file, the index and the HEAD (if exists)\n+\t * must match; but the file can already been removed, since\n+\t * this sequence is a natural \"novice\" way:\n+\t *\n+\t *\trm F; git fm F\n+\t *\n+\t * Further, if HEAD commit exists, \"diff-index --cached\" must\n+\t * report no changes unless forced.\n+\t */\n+\tif (!force) {\n+\t\tunsigned char sha1[20];\n+\t\tif (get_sha1(\"HEAD\", sha1))\n+\t\t\thashclr(sha1);\n+\t\tif (check_local_mod(sha1))\n+\t\t\texit(1);\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 * 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@@ -121,14 +188,14 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t\treturn 0;\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 * Then, unless we used \"--cache\", remove the filenames from\n+\t * the 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 ourselves and can't fail\n \t * in the middle)\n \t */\n-\tif (force) {\n+\tif (!index_only) {\n \t\tint removed = 0;\n \t\tfor (i = 0; i < list.nr; i++) {\n \t\t\tconst char *path = list.name[i];\n"},{"id":"29925","messageId":"emdkib$fve$1@sea.gmane.org","threadId":"6053","inReplyTo":"7vwt4lboa2.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFH/RFT] git-rm: update to saner semantics","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-21T09:38:52Z","receivedAt":"2006-12-21T09:38:52Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> -\"git-rm [-n] [-v] [-f] ...\";\n> +\"git-rm [-n] [-f] [--cached] ...\";\n\nWhy do we lost -v (for verbose)?\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"}]}