[PATCH] Use clean.requireforce to protect untracked files.

[PATCH] Use clean.requireforce to protect untracked files.

[Jiang Xin]

Untracked files may be significant for certain repositories, but if run the
command "git clean -fdx" by accident, all untracked files will be lost.

This hack adds three values in addtion to true/false to "clean.requireforce",
which can protect untracked files from cleaning:

* true or unset : can not clean without -f/--force option provided.
* false         : clean untracked files just as -f/--force option provided.
* lockignored   : can not clean untracked ignored files.
* lockunignored : can not clean untracked unignored files.
* lockall       : can not clean anything.

Signed-off-by: Jiang Xin <jiangxin@ossxp.com>
---
 Documentation/config.txt |    7 +++-
 builtin/clean.c          |   29 ++++++++++++++++++-
 t/t7300-clean.sh         |   69 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6b93777..b930f42 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -684,8 +684,11 @@ browser.<tool>.path::
 	working repository in gitweb (see linkgit:git-instaweb[1]).
 
 clean.requireForce::
-	A boolean to make git-clean do nothing unless given -f
-	or -n.   Defaults to true.
+	When set to `LockIgnored`, cleaning untracked ignored files is
+	denied. When set to `LockUnIgnored`, only allow cleaning untracked
+	ignored files using -X option. When set to `LockAll`, no files
+	can be cleaned until unset this variable. A boolean to make
+	git-clean do nothing unless given -f or -n. Defaults to true.
 
 color.branch::
 	A boolean to enable/disable color in the output of
diff --git a/builtin/clean.c b/builtin/clean.c
index 75697f7..441f35a 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -13,7 +13,11 @@
 #include "string-list.h"
 #include "quote.h"
 
+#define LOCK_IGNORED	01
+#define LOCK_UNIGNORED	02
+
 static int force = -1; /* unset */
+static int lock_flag = 0;
 
 static const char *const builtin_clean_usage[] = {
 	"git clean [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>...",
@@ -22,8 +26,17 @@ static const char *const builtin_clean_usage[] = {
 
 static int git_clean_config(const char *var, const char *value, void *cb)
 {
-	if (!strcmp(var, "clean.requireforce"))
-		force = !git_config_bool(var, value);
+	if (!strcmp(var, "clean.requireforce")) {
+		if (value && !strcasecmp(value, "LockIgnored"))
+			lock_flag = LOCK_IGNORED;
+		else if (value && !strcasecmp(value, "LockUnIgnored"))
+			lock_flag = LOCK_UNIGNORED;
+		else if (value && !strcasecmp(value, "LockAll"))
+			lock_flag = LOCK_IGNORED | LOCK_UNIGNORED;
+		else
+			force = !git_config_bool(var, value);
+		return 0;
+	}
 	return git_default_config(var, value, cb);
 }
 
@@ -77,6 +90,18 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	if (ignored && ignored_only)
 		die(_("-x and -X cannot be used together"));
 
+	if (!show_only && lock_flag) {
+		if (lock_flag & LOCK_IGNORED && lock_flag & LOCK_UNIGNORED)
+			die(_("clean.requireForce set to LockAll; "
+				  "refusing to clean until reset clean.requireForce"));
+		else if (lock_flag & LOCK_IGNORED && (ignored_only || ignored))
+			die(_("clean.requireForce set to LockIgnored and conflict with -x or -X; "
+				  "refusing to clean"));
+		else if (lock_flag & LOCK_UNIGNORED && !ignored_only)
+			die(_("clean.requireForce set to LockUnIgnored and can only work with -X; "
+				  "refusing to clean"));
+	}
+
 	if (!show_only && !force) {
 		if (config_set)
 			die(_("clean.requireForce set to true and neither -n nor -f given; "
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 800b536..b4f38dd 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -460,4 +460,73 @@ test_expect_success SANITY 'git clean -d with an unreadable empty directory' '
 	! test -d foo
 '
 
+test_expect_success 'clean.requireForce LockIgnored' '
+
+	git config clean.requireForce LockIgnored &&
+	mkdir -p build docs &&
+	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
+	test_must_fail git clean &&
+	test_must_fail git clean -f -x &&
+	test_must_fail git clean -f -X &&
+	git clean -f &&
+	test -f Makefile &&
+	test -f README &&
+	test -f src/part1.c &&
+	test -f src/part2.c &&
+	test ! -f a.out &&
+	test ! -f src/part3.c &&
+	test -f docs/manual.txt &&
+	test -f obj.o &&
+	test -f build/lib.so &&
+	git clean -f -d &&
+	test ! -f docs/manual.txt &&
+	test -f build/lib.so &&
+	git config clean.requireForce true
+'
+
+test_expect_success 'clean.requireForce LockUnIgnored' '
+
+	git config clean.requireForce LockUnIgnored &&
+	mkdir -p build docs &&
+	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
+	test_must_fail git clean &&
+	test_must_fail git clean -f &&
+	test_must_fail git clean -f -x &&
+	git clean -f -X &&
+	test -f Makefile &&
+	test -f README &&
+	test -f src/part1.c &&
+	test -f src/part2.c &&
+	test -f a.out &&
+	test -f src/part3.c &&
+	test -f docs/manual.txt &&
+	test ! -f obj.o &&
+	test -f build/lib.so &&
+	git clean -f -d -X &&
+	test ! -f build/lib.so &&
+	test -f src/part3.c &&
+	git config clean.requireForce true
+'
+
+test_expect_success 'clean.requireForce LockAll' '
+
+	git config clean.requireForce lockall &&
+	mkdir -p build docs &&
+	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
+	test_must_fail git clean -f &&
+	test_must_fail git clean -f -x &&
+	test_must_fail git clean -f -X &&
+	git clean -ndx &&
+	test -f Makefile &&
+	test -f README &&
+	test -f src/part1.c &&
+	test -f src/part2.c &&
+	test -f a.out &&
+	test -f src/part3.c &&
+	test -f docs/manual.txt &&
+	test -f obj.o &&
+	test -f build/lib.so &&
+	git config clean.requireForce true
+'
+
 test_done
-- 
1.7.5.4.1.g6c49.dirty

Re: [PATCH] Use clean.requireforce to protect untracked files.

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> Untracked files may be significant for certain repositories, but if run the
> command "git clean -fdx" by accident, all untracked files will be lost.

Don't add -x without thinking, then. It is the way to tell the command "I
want to remove all the untracked files and I REALLY MEAN IT".  It is often
used to say "I do not trust Makefile and I want to remove what 'make
clean' would leave behind".

A slightly related tangent is that we only have three classes of paths:

 - tracked ones
 - untracked ones, where there are two subclasses
   - unignored ones (e.g. new source file you haven't added)
   - ignored ones (e.g. build artifacts like *.o files)

and because of that, the general design is to consider "ignored" files
expendable during various operations. Sometimes people deliberately "ignore"
files that they consider not expendable, which is (by today's definition)
a wrong thing to do, but I think in the longer term we should add a way to
mark them as "ignored but precious".

  http://thread.gmane.org/gmane.comp.version-control.git/172818/focus=172846

Nobody has designed how this fourth class should behave (and how the
behaviour of the "ignored" should change, if any) yet, but a rough outline
would probably be:

 - precious files are the ones that are ignored (by today's definition,
   i.e. .gitignore mechanism consideres they are ignored) but marked as
   "precious" in some other way [*1*]. They will

   - not appear in "Untracked files:" section in "git status" output;
   - not be added by "git add" without "-f", just like other ignored files;
   - not be overwritten or removed to make room while switching branches;
   - not be removed with "clean -f -x" [*2*].

 - ignored files will stay to be "expendable".

I suspect there may be some codepaths that incorrectly treat them as not
expendable, and protect their lossage. We would want to fix them after we
introduce the "precious" class.

[Footnotes]

*1* We could invent a way to sneak such entries in .gitignore, but I am
inclined to think it would be cleaner to define "precious" attribute and
let the attributes mechanism handle this.

*2* This is just off the top of my head without thinking things
through. It might turn out that it makes more sense ot remove them.

Re: [PATCH] Use clean.requireforce to protect untracked files.

[Jiang Xin]

I do agree there are ignored-but-precious type of files exist in practice.
Introducing such ignored-but-precious class through attributes or .gitignore
is fine, and the definition of the class can broadcast to others through
.gitattributes or .gitignore, it's cool. But it sounds a bit complicated.

This afternoon's hack on config variable "clean.requestForce" feets my needs,
but it also has drawbacks:

1. Define a global "clean.requestForce" variable like this:

    $ git config --global clean.requestForce LockIgnored

2. Then in one repository,

    * You can : git clean -f
    * But you cannot : git clean -f -x

3. If want to override such global setting, simply

    $ git config clean.requestForce true

The side-effect of this hack is that an unhacked git will complain:

    fatal: bad config value for 'clean.requireforce' in .git/config


于 11-6-3 下午11:11, Junio C Hamano 写道:
> Jiang Xin <worldhello.net@gmail.com> writes:
> 
>> Untracked files may be significant for certain repositories, but if run the
>> command "git clean -fdx" by accident, all untracked files will be lost.
> 
> Don't add -x without thinking, then. It is the way to tell the command "I
> want to remove all the untracked files and I REALLY MEAN IT".  It is often
> used to say "I do not trust Makefile and I want to remove what 'make
> clean' would leave behind".
> 
> A slightly related tangent is that we only have three classes of paths:
> 
>  - tracked ones
>  - untracked ones, where there are two subclasses
>    - unignored ones (e.g. new source file you haven't added)
>    - ignored ones (e.g. build artifacts like *.o files)
> 
> and because of that, the general design is to consider "ignored" files
> expendable during various operations. Sometimes people deliberately "ignore"
> files that they consider not expendable, which is (by today's definition)
> a wrong thing to do, but I think in the longer term we should add a way to
> mark them as "ignored but precious".
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/172818/focus=172846
> 
> Nobody has designed how this fourth class should behave (and how the
> behaviour of the "ignored" should change, if any) yet, but a rough outline
> would probably be:
> 
>  - precious files are the ones that are ignored (by today's definition,
>    i.e. .gitignore mechanism consideres they are ignored) but marked as
>    "precious" in some other way [*1*]. They will
> 
>    - not appear in "Untracked files:" section in "git status" output;
>    - not be added by "git add" without "-f", just like other ignored files;
>    - not be overwritten or removed to make room while switching branches;
>    - not be removed with "clean -f -x" [*2*].
> 
>  - ignored files will stay to be "expendable".
> 
> I suspect there may be some codepaths that incorrectly treat them as not
> expendable, and protect their lossage. We would want to fix them after we
> introduce the "precious" class.
> 
> [Footnotes]
> 
> *1* We could invent a way to sneak such entries in .gitignore, but I am
> inclined to think it would be cleaner to define "precious" attribute and
> let the attributes mechanism handle this.
> 
> *2* This is just off the top of my head without thinking things
> through. It might turn out that it makes more sense ot remove them.


-- 
蒋鑫

北京群英汇信息技术有限公司
邮件: worldhello.net@gmail.com
网址: http://www.ossxp.com/
　　  http://blog.ossxp.com/
电话: 010-51262007, 13910430470
传真: 010-51262007