# Re: [PATCH] Make git-clean a builtin

5 messages from 2007-10-07 to 2007-10-07. Participants: Linus Torvalds, Shawn Bohrer, Johannes Schindelin, rae l.
Thread: https://gitlist.dev/t/10176

## Shawn Bohrer, 2007-10-07 01:17

Subject: [PATCH] Make git-clean a builtin
Message-ID: <1191719841666-git-send-email-shawn.bohrer@gmail.com>
URL: https://gitlist.dev/e/1191719841666-git-send-email-shawn.bohrer%40gmail.com

```
This replaces git-clean.sh with builtin-clean.c, and moves git-clean.sh to the
examples.

Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>
---

Reworked patch with fixes to Frank's suggestions.

 Makefile                                      |    3 +-
 builtin-clean.c                               |  177 +++++++++++++++++++++++++
 builtin.h                                     |    1 +
 git-clean.sh => contrib/examples/git-clean.sh |    0 
 git.c                                         |    1 +
 5 files changed, 181 insertions(+), 1 deletions(-)
 create mode 100644 builtin-clean.c
 rename git-clean.sh => contrib/examples/git-clean.sh (100%)

diff --git a/Makefile b/Makefile
index 8db4dbe..2b3b8fb 100644
--- a/Makefile
+++ b/Makefile
@@ -206,7 +206,7 @@ BASIC_LDFLAGS =
 
 SCRIPT_SH = \
 	git-bisect.sh git-checkout.sh \
-	git-clean.sh git-clone.sh git-commit.sh \
+	git-clone.sh git-commit.sh \
 	git-fetch.sh \
 	git-ls-remote.sh \
 	git-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \
@@ -327,6 +327,7 @@ BUILTIN_OBJS = \
 	builtin-check-attr.o \
 	builtin-checkout-index.o \
 	builtin-check-ref-format.o \
+	builtin-clean.o \
 	builtin-commit-tree.o \
 	builtin-count-objects.o \
 	builtin-describe.o \
diff --git a/builtin-clean.c b/builtin-clean.c
new file mode 100644
index 0000000..4890c2d
--- /dev/null
+++ b/builtin-clean.c
@@ -0,0 +1,177 @@
+/*
+ * "git clean" builtin command
+ *
+ * Copyright (C) 2007 Shawn Bohrer
+ *
+ * Based on git-clean.sh by Pavel Roskin
+ */
+
+#include "builtin.h"
+#include "cache.h"
+#include "run-command.h"
+
+static int disabled = 0;
+static int show_only = 0;
+static int remove_directories = 0;
+static int quiet = 0;
+static int ignored = 0;
+static int ignored_only = 0;
+
+static const char builtin_clean_usage[] =
+"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...";
+
+static int git_clean_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "clean.requireforce")) {
+		disabled = git_config_bool(var, value);
+	}
+	return 0;
+}
+
+static int remove_directory(const char *path)
+{
+	DIR *d;
+	struct dirent *dir;
+	d = opendir(path);
+	if (d) {
+		chdir(path);
+		while ((dir = readdir(d)) != NULL) {
+			if(strcmp( dir->d_name, ".") == 0 ||
+			   strcmp( dir->d_name, ".." ) == 0 )
+				continue;
+			if (dir->d_type == DT_DIR)
+				remove_directory(dir->d_name);
+			else
+				unlink(dir->d_name);
+		}
+		chdir("..");
+	}
+	closedir(d);
+	return rmdir(path);
+}
+
+int cmd_clean(int argc, const char **argv, const char *prefix)
+{
+	int i;
+	int j;
+	struct child_process cmd;
+	const char **argv_ls_files;
+	char *buf = NULL;
+	char path[1024];
+	FILE *cmd_fout;
+
+	git_config(git_clean_config);
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+
+		if (arg[0] != '-')
+			break;
+		if (!strcmp(arg, "--")) {
+			i++;
+			break;
+		}
+		if (!strcmp(arg, "-n")) {
+			show_only = 1;
+			disabled = 0;
+			continue;
+		}
+		if (!strcmp(arg, "-f")) {
+			disabled = 0;
+			continue;
+		}
+		if (!strcmp(arg, "-d")) {
+			remove_directories = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-q")) {
+			quiet = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-x")) {
+			ignored = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-X")) {
+			ignored_only = 1;
+			continue;
+		}
+		usage(builtin_clean_usage);
+	}
+
+	if (ignored && ignored_only)
+		usage(builtin_clean_usage);
+
+	if (disabled) {
+		die("clean.requireForce set and -n or -f not given; refusing to clean");
+	}
+
+	/* Paths (argc - i) + 8 (Possible arguments)*/
+	argv_ls_files = xmalloc((argc - i + 8) * sizeof(const char *));
+	argv_ls_files[0] = "ls-files";
+	argv_ls_files[1] = "--others";
+	argv_ls_files[2] = "--directory";
+	j = 3;
+	if (!ignored) {
+		argv_ls_files[j++] = "--exclude-per-directory=.gitignore";
+		if (ignored_only)
+			argv_ls_files[j++] = "--ignored";
+		if (!access(git_path("info/exclude"), F_OK)) {
+			char *exclude_path = git_path("info/exclude");
+			int len = strlen(exclude_path);
+			buf = (char*)malloc(len+16);
+			sprintf(buf, "--exclude-from=%s", exclude_path);
+			argv_ls_files[j++] = buf;
+		}
+	}
+	argv_ls_files[j++] = "--";
+	/* Add remaining paths passed in as arguments */
+	if (argc - i)
+		memcpy(argv_ls_files + j++, argv + i, (argc - i) * sizeof(const char *));
+	argv_ls_files[j + argc - i] = NULL;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.argv = argv_ls_files;
+	cmd.git_cmd = 1;
+	cmd.out = -1;
+	if (start_command(&cmd))
+		die("Could not run sub-command: git ls-files");
+
+	cmd_fout = fdopen(cmd.out, "r");
+	while (fgets(path, sizeof(path), cmd_fout) != NULL) {
+		struct stat st;
+		char *p;
+		p = strrchr(path, '\n');
+		if ( p != NULL )
+			*p = '\0';
+		if (!lstat(path, &st) && (S_ISDIR(st.st_mode))) {
+			if (show_only && remove_directories) {
+				printf("Would remove %s\n", path);
+			} else if (quiet && remove_directories) {
+				remove_directory(path);
+			} else if (remove_directories) {
+				printf("Removing %s\n", path);
+				remove_directory(path);
+			} else if (show_only) {
+				printf("Would not remove %s\n", path);
+			} else {
+				printf("Not removing %s\n", path);
+			}
+		} else {
+			if (show_only) {
+				printf("Would remove %s\n", path);
+				continue;
+			} else if (!quiet) {
+				printf("Removing %s\n", path);
+			}
+			unlink(path);
+		}
+	}
+
+	fclose(cmd_fout);
+	finish_command(&cmd);
+	if (buf != NULL)
+		free(buf);
+	free(argv_ls_files);
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index d6f2c76..8c112f3 100644
--- a/builtin.h
+++ b/builtin.h
@@ -23,6 +23,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
+extern int cmd_clean(int argc, const char **argv, const char *prefix);
 extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
 extern int cmd_describe(int argc, const char **argv, const char *prefix);
diff --git a/git-clean.sh b/contrib/examples/git-clean.sh
similarity index 100%
rename from git-clean.sh
rename to contrib/examples/git-clean.sh
diff --git a/git.c b/git.c
index 9eaca1d..cda6344 100644
--- a/git.c
+++ b/git.c
@@ -320,6 +320,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
 		{ "cherry", cmd_cherry, RUN_SETUP },
 		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+		{ "clean", cmd_clean, RUN_SETUP },
 		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
 		{ "config", cmd_config },
 		{ "count-objects", cmd_count_objects, RUN_SETUP },
-- 
1.5.3.GIT

```

## Linus Torvalds, 2007-10-07 01:31

Subject: Re: [PATCH] Make git-clean a builtin
Message-ID: <alpine.LFD.0.999.0710061827010.23684@woody.linux-foundation.org>
URL: https://gitlist.dev/e/alpine.LFD.0.999.0710061827010.23684%40woody.linux-foundation.org
In-Reply-To: <1191719841666-git-send-email-shawn.bohrer@gmail.com>

```


On Sat, 6 Oct 2007, Shawn Bohrer wrote:
>
> This replaces git-clean.sh with builtin-clean.c, and moves git-clean.sh to the
> examples.

This looks better, but I think you'd be even better off actually using the 
"read_directory()" interface directly, instead of exec'ing off "git 
ls-files" and parsing the line output.

I also would still worry a bit about 'chdir(x)' and 'chdir("..")', because 
quite frankly, they are *not* mirrors of each other (think symlinks, but 
also error behaviour due to directories that might be non-executable). 
Now, admittedly, if a directory isn't executable, I can imagine other git 
things having problems (anybody want to test?), but that whole pattern is 
just very fragile and not very reliable.

		Linus

```

## Shawn Bohrer, 2007-10-07 15:41

Subject: Re: [PATCH] Make git-clean a builtin
Message-ID: <20071007154126.GD5642@mediacenter.austin.rr.com>
URL: https://gitlist.dev/e/20071007154126.GD5642%40mediacenter.austin.rr.com
In-Reply-To: <alpine.LFD.0.999.0710061827010.23684@woody.linux-foundation.org>

```
Thanks for the input.

On Sat, Oct 06, 2007 at 06:31:36PM -0700, Linus Torvalds wrote:
> This looks better, but I think you'd be even better off actually using the 
> "read_directory()" interface directly, instead of exec'ing off "git 
> ls-files" and parsing the line output.

Perhaps, I'll take a look at how git-ls-files does it and see if I can
do that directly.  Since I'm new to git (and C) it will probably take me
a while to re-implement though.

> I also would still worry a bit about 'chdir(x)' and 'chdir("..")', because 
> quite frankly, they are *not* mirrors of each other (think symlinks, but 
> also error behaviour due to directories that might be non-executable). 
> Now, admittedly, if a directory isn't executable, I can imagine other git 
> things having problems (anybody want to test?), but that whole pattern is 
> just very fragile and not very reliable.

Yes it does seem fragile, but 'chdir("-")' doesn't work in C and I
couldn't find any equivalents.  I actually did think about symlinks, and
my code does do the right thing since I test if it is a directory before
doing the 'chdir(x)'. Symlinks are therefore treated as normal files and
removed.

I did not think about non-executable directories, and you are correct
that my code will fail to remove a directory if it is non-executable. I
also tested a git-ls-files with non-executable directories, and it will
fail to show you any files that are more than one level deep for
example:

|-- docs
|   |-- contributing
|   |   `-- patches.txt
|   `-- manual.txt

If docs is non-executable it will only return 'docs/manual.txt'

--
Shawn

```

## Johannes Schindelin, 2007-10-07 16:38

Subject: Re: [PATCH] Make git-clean a builtin
Message-ID: <Pine.LNX.4.64.0710071737500.4174@racer.site>
URL: https://gitlist.dev/e/Pine.LNX.4.64.0710071737500.4174%40racer.site
In-Reply-To: <1191719841666-git-send-email-shawn.bohrer@gmail.com>

```
Hi,

On Sat, 6 Oct 2007, Shawn Bohrer wrote:

> +static int remove_directory(const char *path)

Please use remove_directory_recursively(), introduced in commit 
7155b727c9baae9ef6d7829370aefc09c4ab64e2 to 'next'.

I have not looked at the rest of your patch yet.

Ciao,
Dscho

```

## rae l, 2007-10-07 16:42

Subject: Re: [PATCH] Make git-clean a builtin
Message-ID: <91b13c310710070942q31cb4b5do81d1488bbd88dff@mail.gmail.com>
URL: https://gitlist.dev/e/91b13c310710070942q31cb4b5do81d1488bbd88dff%40mail.gmail.com
In-Reply-To: <20071007154126.GD5642@mediacenter.austin.rr.com>

```
On 10/7/07, Shawn Bohrer <shawn.bohrer@gmail.com> wrote:
> Thanks for the input.
>
> On Sat, Oct 06, 2007 at 06:31:36PM -0700, Linus Torvalds wrote:
> > This looks better, but I think you'd be even better off actually using the
> > "read_directory()" interface directly, instead of exec'ing off "git
> > ls-files" and parsing the line output.
>
> Perhaps, I'll take a look at how git-ls-files does it and see if I can
> do that directly.  Since I'm new to git (and C) it will probably take me
> a while to re-implement though.
>
> > I also would still worry a bit about 'chdir(x)' and 'chdir("..")', because
> > quite frankly, they are *not* mirrors of each other (think symlinks, but
> > also error behaviour due to directories that might be non-executable).
> > Now, admittedly, if a directory isn't executable, I can imagine other git
> > things having problems (anybody want to test?), but that whole pattern is
> > just very fragile and not very reliable.
>
> Yes it does seem fragile, but 'chdir("-")' doesn't work in C and I
> couldn't find any equivalents.  I actually did think about symlinks, and
> my code does do the right thing since I test if it is a directory before
> doing the 'chdir(x)'. Symlinks are therefore treated as normal files and
> removed.
'chdir -' is just supported by the shell, and the C interface could
use chdir(OLDPWD).

```