{"thread":{"id":"10176","subject":"Re: [PATCH] Make git-clean a builtin","startedAt":"2007-10-07T01:17:21Z","lastAt":"2007-10-07T16:42:10Z","messageCount":5,"participants":["Linus Torvalds","Shawn Bohrer","Johannes Schindelin","rae l"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"55036","messageId":"1191719841666-git-send-email-shawn.bohrer@gmail.com","threadId":"10176","inReplyTo":null,"subject":"[PATCH] Make git-clean a builtin","fromName":"Shawn Bohrer","fromEmail":"shawn.bohrer@gmail.com","sentAt":"2007-10-07T01:17:21Z","receivedAt":"2007-10-07T01:17:21Z","isPatch":true,"sender":{"key":"shawn.bohrer@gmail.com","avatar":"https://gravatar.com/avatar/6eb093ef7d276306d18366254e0c95ff6a5db58231ac7e82fe78c2800aaae1b6?d=mp&s=160"},"body":"This replaces git-clean.sh with builtin-clean.c, and moves git-clean.sh to the\nexamples.\n\nSigned-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>\n---\n\nReworked patch with fixes to Frank's suggestions.\n\n Makefile                                      |    3 +-\n builtin-clean.c                               |  177 +++++++++++++++++++++++++\n builtin.h                                     |    1 +\n git-clean.sh => contrib/examples/git-clean.sh |    0 \n git.c                                         |    1 +\n 5 files changed, 181 insertions(+), 1 deletions(-)\n create mode 100644 builtin-clean.c\n rename git-clean.sh => contrib/examples/git-clean.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex 8db4dbe..2b3b8fb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -206,7 +206,7 @@ BASIC_LDFLAGS =\n \n SCRIPT_SH = \\\n \tgit-bisect.sh git-checkout.sh \\\n-\tgit-clean.sh git-clone.sh git-commit.sh \\\n+\tgit-clone.sh git-commit.sh \\\n \tgit-fetch.sh \\\n \tgit-ls-remote.sh \\\n \tgit-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \\\n@@ -327,6 +327,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-check-attr.o \\\n \tbuiltin-checkout-index.o \\\n \tbuiltin-check-ref-format.o \\\n+\tbuiltin-clean.o \\\n \tbuiltin-commit-tree.o \\\n \tbuiltin-count-objects.o \\\n \tbuiltin-describe.o \\\ndiff --git a/builtin-clean.c b/builtin-clean.c\nnew file mode 100644\nindex 0000000..4890c2d\n--- /dev/null\n+++ b/builtin-clean.c\n@@ -0,0 +1,177 @@\n+/*\n+ * \"git clean\" builtin command\n+ *\n+ * Copyright (C) 2007 Shawn Bohrer\n+ *\n+ * Based on git-clean.sh by Pavel Roskin\n+ */\n+\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"run-command.h\"\n+\n+static int disabled = 0;\n+static int show_only = 0;\n+static int remove_directories = 0;\n+static int quiet = 0;\n+static int ignored = 0;\n+static int ignored_only = 0;\n+\n+static const char builtin_clean_usage[] =\n+\"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...\";\n+\n+static int git_clean_config(const char *var, const char *value)\n+{\n+\tif (!strcmp(var, \"clean.requireforce\")) {\n+\t\tdisabled = git_config_bool(var, value);\n+\t}\n+\treturn 0;\n+}\n+\n+static int remove_directory(const char *path)\n+{\n+\tDIR *d;\n+\tstruct dirent *dir;\n+\td = opendir(path);\n+\tif (d) {\n+\t\tchdir(path);\n+\t\twhile ((dir = readdir(d)) != NULL) {\n+\t\t\tif(strcmp( dir->d_name, \".\") == 0 ||\n+\t\t\t   strcmp( dir->d_name, \"..\" ) == 0 )\n+\t\t\t\tcontinue;\n+\t\t\tif (dir->d_type == DT_DIR)\n+\t\t\t\tremove_directory(dir->d_name);\n+\t\t\telse\n+\t\t\t\tunlink(dir->d_name);\n+\t\t}\n+\t\tchdir(\"..\");\n+\t}\n+\tclosedir(d);\n+\treturn rmdir(path);\n+}\n+\n+int cmd_clean(int argc, const char **argv, const char *prefix)\n+{\n+\tint i;\n+\tint j;\n+\tstruct child_process cmd;\n+\tconst char **argv_ls_files;\n+\tchar *buf = NULL;\n+\tchar path[1024];\n+\tFILE *cmd_fout;\n+\n+\tgit_config(git_clean_config);\n+\n+\tfor (i = 1; i < argc; i++) {\n+\t\tconst char *arg = argv[i];\n+\n+\t\tif (arg[0] != '-')\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\tdisabled = 0;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"-f\")) {\n+\t\t\tdisabled = 0;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"-d\")) {\n+\t\t\tremove_directories = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"-q\")) {\n+\t\t\tquiet = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"-x\")) {\n+\t\t\tignored = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"-X\")) {\n+\t\t\tignored_only = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tusage(builtin_clean_usage);\n+\t}\n+\n+\tif (ignored && ignored_only)\n+\t\tusage(builtin_clean_usage);\n+\n+\tif (disabled) {\n+\t\tdie(\"clean.requireForce set and -n or -f not given; refusing to clean\");\n+\t}\n+\n+\t/* Paths (argc - i) + 8 (Possible arguments)*/\n+\targv_ls_files = xmalloc((argc - i + 8) * sizeof(const char *));\n+\targv_ls_files[0] = \"ls-files\";\n+\targv_ls_files[1] = \"--others\";\n+\targv_ls_files[2] = \"--directory\";\n+\tj = 3;\n+\tif (!ignored) {\n+\t\targv_ls_files[j++] = \"--exclude-per-directory=.gitignore\";\n+\t\tif (ignored_only)\n+\t\t\targv_ls_files[j++] = \"--ignored\";\n+\t\tif (!access(git_path(\"info/exclude\"), F_OK)) {\n+\t\t\tchar *exclude_path = git_path(\"info/exclude\");\n+\t\t\tint len = strlen(exclude_path);\n+\t\t\tbuf = (char*)malloc(len+16);\n+\t\t\tsprintf(buf, \"--exclude-from=%s\", exclude_path);\n+\t\t\targv_ls_files[j++] = buf;\n+\t\t}\n+\t}\n+\targv_ls_files[j++] = \"--\";\n+\t/* Add remaining paths passed in as arguments */\n+\tif (argc - i)\n+\t\tmemcpy(argv_ls_files + j++, argv + i, (argc - i) * sizeof(const char *));\n+\targv_ls_files[j + argc - i] = NULL;\n+\n+\tmemset(&cmd, 0, sizeof(cmd));\n+\tcmd.argv = argv_ls_files;\n+\tcmd.git_cmd = 1;\n+\tcmd.out = -1;\n+\tif (start_command(&cmd))\n+\t\tdie(\"Could not run sub-command: git ls-files\");\n+\n+\tcmd_fout = fdopen(cmd.out, \"r\");\n+\twhile (fgets(path, sizeof(path), cmd_fout) != NULL) {\n+\t\tstruct stat st;\n+\t\tchar *p;\n+\t\tp = strrchr(path, '\\n');\n+\t\tif ( p != NULL )\n+\t\t\t*p = '\\0';\n+\t\tif (!lstat(path, &st) && (S_ISDIR(st.st_mode))) {\n+\t\t\tif (show_only && remove_directories) {\n+\t\t\t\tprintf(\"Would remove %s\\n\", path);\n+\t\t\t} else if (quiet && remove_directories) {\n+\t\t\t\tremove_directory(path);\n+\t\t\t} else if (remove_directories) {\n+\t\t\t\tprintf(\"Removing %s\\n\", path);\n+\t\t\t\tremove_directory(path);\n+\t\t\t} else if (show_only) {\n+\t\t\t\tprintf(\"Would not remove %s\\n\", path);\n+\t\t\t} else {\n+\t\t\t\tprintf(\"Not removing %s\\n\", path);\n+\t\t\t}\n+\t\t} else {\n+\t\t\tif (show_only) {\n+\t\t\t\tprintf(\"Would remove %s\\n\", path);\n+\t\t\t\tcontinue;\n+\t\t\t} else if (!quiet) {\n+\t\t\t\tprintf(\"Removing %s\\n\", path);\n+\t\t\t}\n+\t\t\tunlink(path);\n+\t\t}\n+\t}\n+\n+\tfclose(cmd_fout);\n+\tfinish_command(&cmd);\n+\tif (buf != NULL)\n+\t\tfree(buf);\n+\tfree(argv_ls_files);\n+\treturn 0;\n+}\ndiff --git a/builtin.h b/builtin.h\nindex d6f2c76..8c112f3 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -23,6 +23,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\n+extern int cmd_clean(int argc, const char **argv, const char *prefix);\n extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);\n extern int cmd_count_objects(int argc, const char **argv, const char *prefix);\n extern int cmd_describe(int argc, const char **argv, const char *prefix);\ndiff --git a/git-clean.sh b/contrib/examples/git-clean.sh\nsimilarity index 100%\nrename from git-clean.sh\nrename to contrib/examples/git-clean.sh\ndiff --git a/git.c b/git.c\nindex 9eaca1d..cda6344 100644\n--- a/git.c\n+++ b/git.c\n@@ -320,6 +320,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"cherry\", cmd_cherry, RUN_SETUP },\n \t\t{ \"cherry-pick\", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },\n+\t\t{ \"clean\", cmd_clean, RUN_SETUP },\n \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n \t\t{ \"config\", cmd_config },\n \t\t{ \"count-objects\", cmd_count_objects, RUN_SETUP },\n-- \n1.5.3.GIT\n"},{"id":"55033","messageId":"alpine.LFD.0.999.0710061827010.23684@woody.linux-foundation.org","threadId":"10176","inReplyTo":"1191719841666-git-send-email-shawn.bohrer@gmail.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-07T01:31:36Z","receivedAt":"2007-10-07T01:31:36Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 6 Oct 2007, Shawn Bohrer wrote:\n>\n> This replaces git-clean.sh with builtin-clean.c, and moves git-clean.sh to the\n> examples.\n\nThis looks better, but I think you'd be even better off actually using the \n\"read_directory()\" interface directly, instead of exec'ing off \"git \nls-files\" and parsing the line output.\n\nI also would still worry a bit about 'chdir(x)' and 'chdir(\"..\")', because \nquite frankly, they are *not* mirrors of each other (think symlinks, but \nalso error behaviour due to directories that might be non-executable). \nNow, admittedly, if a directory isn't executable, I can imagine other git \nthings having problems (anybody want to test?), but that whole pattern is \njust very fragile and not very reliable.\n\n\t\tLinus\n"},{"id":"55054","messageId":"20071007154126.GD5642@mediacenter.austin.rr.com","threadId":"10176","inReplyTo":"alpine.LFD.0.999.0710061827010.23684@woody.linux-foundation.org","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Shawn Bohrer","fromEmail":"shawn.bohrer@gmail.com","sentAt":"2007-10-07T15:41:26Z","receivedAt":"2007-10-07T15:41:26Z","isPatch":true,"sender":{"key":"shawn.bohrer@gmail.com","avatar":"https://gravatar.com/avatar/6eb093ef7d276306d18366254e0c95ff6a5db58231ac7e82fe78c2800aaae1b6?d=mp&s=160"},"body":"Thanks for the input.\n\nOn Sat, Oct 06, 2007 at 06:31:36PM -0700, Linus Torvalds wrote:\n> This looks better, but I think you'd be even better off actually using the \n> \"read_directory()\" interface directly, instead of exec'ing off \"git \n> ls-files\" and parsing the line output.\n\nPerhaps, I'll take a look at how git-ls-files does it and see if I can\ndo that directly.  Since I'm new to git (and C) it will probably take me\na while to re-implement though.\n\n> I also would still worry a bit about 'chdir(x)' and 'chdir(\"..\")', because \n> quite frankly, they are *not* mirrors of each other (think symlinks, but \n> also error behaviour due to directories that might be non-executable). \n> Now, admittedly, if a directory isn't executable, I can imagine other git \n> things having problems (anybody want to test?), but that whole pattern is \n> just very fragile and not very reliable.\n\nYes it does seem fragile, but 'chdir(\"-\")' doesn't work in C and I\ncouldn't find any equivalents.  I actually did think about symlinks, and\nmy code does do the right thing since I test if it is a directory before\ndoing the 'chdir(x)'. Symlinks are therefore treated as normal files and\nremoved.\n\nI did not think about non-executable directories, and you are correct\nthat my code will fail to remove a directory if it is non-executable. I\nalso tested a git-ls-files with non-executable directories, and it will\nfail to show you any files that are more than one level deep for\nexample:\n\n|-- docs\n|   |-- contributing\n|   |   `-- patches.txt\n|   `-- manual.txt\n\nIf docs is non-executable it will only return 'docs/manual.txt'\n\n--\nShawn\n"},{"id":"55065","messageId":"Pine.LNX.4.64.0710071737500.4174@racer.site","threadId":"10176","inReplyTo":"1191719841666-git-send-email-shawn.bohrer@gmail.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-07T16:38:58Z","receivedAt":"2007-10-07T16:38:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 6 Oct 2007, Shawn Bohrer wrote:\n\n> +static int remove_directory(const char *path)\n\nPlease use remove_directory_recursively(), introduced in commit \n7155b727c9baae9ef6d7829370aefc09c4ab64e2 to 'next'.\n\nI have not looked at the rest of your patch yet.\n\nCiao,\nDscho\n"},{"id":"55067","messageId":"91b13c310710070942q31cb4b5do81d1488bbd88dff@mail.gmail.com","threadId":"10176","inReplyTo":"20071007154126.GD5642@mediacenter.austin.rr.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"rae l","fromEmail":"crquan@gmail.com","sentAt":"2007-10-07T16:42:10Z","receivedAt":"2007-10-07T16:42:10Z","isPatch":true,"sender":{"key":"crquan@gmail.com","avatar":"https://gravatar.com/avatar/8689a8a26f5d1c7515ca8226258710684dac5c5208b84a9e16a086377f2d3ded?d=mp&s=160"},"body":"On 10/7/07, Shawn Bohrer <shawn.bohrer@gmail.com> wrote:\n> Thanks for the input.\n>\n> On Sat, Oct 06, 2007 at 06:31:36PM -0700, Linus Torvalds wrote:\n> > This looks better, but I think you'd be even better off actually using the\n> > \"read_directory()\" interface directly, instead of exec'ing off \"git\n> > ls-files\" and parsing the line output.\n>\n> Perhaps, I'll take a look at how git-ls-files does it and see if I can\n> do that directly.  Since I'm new to git (and C) it will probably take me\n> a while to re-implement though.\n>\n> > I also would still worry a bit about 'chdir(x)' and 'chdir(\"..\")', because\n> > quite frankly, they are *not* mirrors of each other (think symlinks, but\n> > also error behaviour due to directories that might be non-executable).\n> > Now, admittedly, if a directory isn't executable, I can imagine other git\n> > things having problems (anybody want to test?), but that whole pattern is\n> > just very fragile and not very reliable.\n>\n> Yes it does seem fragile, but 'chdir(\"-\")' doesn't work in C and I\n> couldn't find any equivalents.  I actually did think about symlinks, and\n> my code does do the right thing since I test if it is a directory before\n> doing the 'chdir(x)'. Symlinks are therefore treated as normal files and\n> removed.\n'chdir -' is just supported by the shell, and the C interface could\nuse chdir(OLDPWD).\n"}]}