[3/3] clean: add `config.exclude` and `--remove-excluded`

Commit Message

Ivan Shapovalov Feb. 10, 2025, 7:14 p.m. UTC

Add `config.exclude` to configure "always excluded" files (same as `-e`
on the command line), and `--remove-excluded` (intentionally without a
short form) to "REALLY remove everything, dammit!"

This might seem like euphemism treadmill, but there is a specific
use-case for all of the exclusion methods and options:

.gitignore:     files that _the project_ does not want to track or touch
                (build artifacts)
clean.exclude:  files that _the user_ does not want to track or touch
                (IDE configuration)
git clean -x:   remove build artifacts, but keep precious files
                (when a pristine build is desired)
git clean -x --remove-excluded:
                remove everything, including precious files
                (e.g. for redistribution)

Signed-off-by: Ivan Shapovalov <[e-mail]>
---
 Documentation/config/clean.txt | 11 +++++++++++
 Documentation/git-clean.txt    | 22 +++++++++++++++-------
 builtin/clean.c                | 19 ++++++++++++++++---
 3 files changed, 42 insertions(+), 10 deletions(-)

Comments

Junio C Hamano Feb. 11, 2025, 11 p.m. UTC | #1

Ivan Shapovalov <[e-mail]> writes:

> Add `config.exclude` to configure "always excluded" files (same as `-e`
> on the command line), and `--remove-excluded` (intentionally without a
> short form) to "REALLY remove everything, dammit!"
>
> This might seem like euphemism treadmill, but there is a specific
> use-case for all of the exclusion methods and options:
>
> .gitignore:     files that _the project_ does not want to track or touch
>                 (build artifacts)
> clean.exclude:  files that _the user_ does not want to track or touch
>                 (IDE configuration)
> git clean -x:   remove build artifacts, but keep precious files
>                 (when a pristine build is desired)
> git clean -x --remove-excluded:
>                 remove everything, including precious files
>                 (e.g. for redistribution)
>
> Signed-off-by: Ivan Shapovalov <[e-mail]>
> ---
>  Documentation/config/clean.txt | 11 +++++++++++
>  Documentation/git-clean.txt    | 22 +++++++++++++++-------
>  builtin/clean.c                | 19 ++++++++++++++++---
>  3 files changed, 42 insertions(+), 10 deletions(-)

A few comments on the proposed semantics.

 - It is questionable to throw paths that match command line "-e"
   patterns into the 'precious' class.  It breaks backward
   compatibility and established end-user expectations, doesn't it?

 - It is nice to see that an effort is made to sift "excluded" into
   two classes.  The traditional "ignored/excluded are expendable,
   so "git clean" will remove them, "git switch", when path F is
   excluded and there is a file F, would remove it when it needs to
   check out a tree that has paths under directory F.  "git add F"
   and "git add ." would not add it unless forced.  We would want
   another class of files `precious` that are ignored in the same
   sense by "git add", but yet excempt from removal by "git clean"
   and things like "git switch".

 - On the other hand, it is a good idea to use a new source of
   patterns that the command never paid attention to, like a new
   configuration variable.  Since nobody has ever used it for
   "excluded and expendable", there is no risk of breaking end-user
   expectations.

 - This particular implementation falls far short of the ideal of
   "precious files" class, though.  Since "git clean" is the only
   thing that pays attention to clean.exclude, "git add ." would
   happily add those paths that match the pattern, and "git switch"
   to check out a directory whose pathname matches the pattern would
   happily nuke a precious file that is in the working tree at that
   path.

Earlier discussions proposed extended syntax added to .gitignore
mechanism and relevant codepaths, not just "git clean", all pay
attention to the new "precious" paths, but one idea proposed by
this series that is much better than the previous designs is the
use of separate sources of patterns---we do not have to worry about
backward compatibility issues at all with that design.

In my earlier review, I said it was clever to recognize that
precious would be of personal nature, but I now realize that there
are valid reasons why projects may _know_ what paths are precious
and would want to distribute that knowledge to its participants.
For example, our project would benefit from having config.mak marked
as precious for everybody, so that nobody commits such a file by
mistake and then ask us to pull from them.

As a configuration variable does not propagate across repositories
by design, it would not work as a way for the project to share its
idea of what paths are in the "precious" class, so in addition to
the clean.exclude (which probably is a bad name, as (1) we want not
just clean but things like add and switch also pay attention to it,
and (2) the class it defines is distinct from "exclude", and would
want to have the word "precious" in it) variable, we'd probably need
to have a way to record them in tracked files, either in .gitignore
files using some extended syntax, or separate .gitprecious files.

Thanks.

Patch

diff --git a/Documentation/config/clean.txt b/Documentation/config/clean.txt
index c0188ead4e..eb64ad26fa 100644
--- a/Documentation/config/clean.txt
+++ b/Documentation/config/clean.txt
@@ -1,3 +1,14 @@ 
 clean.requireForce::
 	A boolean to make git-clean refuse to delete files unless -f
 	is given. Defaults to true.
+
+clean.exclude::
+	Additional exclude patterns that have higher priority than the standard
+	linkgit:gitignore[5] rules and will be honored in (almost) all cases,
+	even if the `-x` or `-X` options are given. These patterns are intended
+	to be used for user-specific "precious" files such as IDE configuration
+	that must not be removed even if a pristine build is desired. This list
+	has the same priority and semantics as the `-e` command line option.
+
+	The `--remove-excluded` command line option can be used to disregard
+	these exclude patterns (intentionally no short form).
diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index fd17165416..33d6fb7228 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -59,15 +59,10 @@  OPTIONS
 	Be quiet, only report errors, but not the files that are
 	successfully removed.
 
--e <pattern>::
---exclude=<pattern>::
-	Use the given exclude pattern in addition to the standard ignore rules
-	(see linkgit:gitignore[5]).
-
 -x::
 	Don't use the standard ignore rules (see linkgit:gitignore[5]), but
-	still use the ignore rules given with `-e` options from the command
-	line.  This allows removing all untracked
+	still use the ignore rules given with the `-e` command line option or the
+	`clean.exclude` configuration variable.  This allows removing all untracked
 	files, including build products.  This can be used (possibly in
 	conjunction with 'git restore' or 'git reset') to create a pristine
 	working directory to test a clean build.
@@ -76,6 +71,19 @@  OPTIONS
 	Remove only files ignored by Git.  This may be useful to rebuild
 	everything from scratch, but keep manually created files.
 
+-e <pattern>::
+--exclude=<pattern>::
+	Use the given exclude pattern in addition to the standard ignore rules
+	(see linkgit:gitignore[5]). Exclude patterns can also be configured
+	using the `clean.exclude` configuration variable. These patterns have
+	higher priority than the `-x` or `-X` options and will be honored
+	even in their presence.
+
+--remove-excluded::
+	Disregard the additional exclude patterns provided by `-e` or the
+	configuration variable `clean.exclude`. This flag has the highest
+	priority and intentionally does not have a short form.
+
 Interactive mode
 ----------------
 When the command enters the interactive mode, it shows the
diff --git a/builtin/clean.c b/builtin/clean.c
index ec58338049..eae22a1ec7 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -29,6 +29,7 @@ 
 static int require_force = -1; /* unset */
 static int interactive;
 static struct string_list del_list = STRING_LIST_INIT_DUP;
+static struct string_list config_exclude_list = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
 static const char *const builtin_clean_usage[] = {
@@ -133,6 +134,11 @@  static int git_clean_config(const char *var, const char *value,
 		return 0;
 	}
 
+	if (!strcmp(var, "clean.exclude")) {
+		string_list_append(&config_exclude_list, value);
+		return 0;
+	}
+
 	if (git_color_config(var, value, cb) < 0)
 		return -1;
 
@@ -925,6 +931,7 @@  int cmd_clean(int argc,
 	int i, res;
 	int dry_run = 0, remove_directories = 0, quiet = 0, remove_ignored = 0;
 	int ignored_only = 0, force = 0, errors = 0, gone = 1;
+	int remove_excluded = 0;
 	int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
 	struct strbuf abs_path = STRBUF_INIT;
 	struct dir_struct dir = DIR_INIT;
@@ -940,11 +947,13 @@  int cmd_clean(int argc,
 		OPT_BOOL('i', "interactive", &interactive, N_("interactive cleaning")),
 		OPT_BOOL('d', NULL, &remove_directories,
 				N_("remove whole directories")),
-		OPT_CALLBACK_F('e', "exclude", &exclude_list, N_("pattern"),
-		  N_("add <pattern> to ignore rules"), PARSE_OPT_NONEG, exclude_cb),
 		OPT_BOOL('x', NULL, &remove_ignored, N_("remove ignored files, too")),
 		OPT_BOOL('X', NULL, &ignored_only,
 				N_("remove only ignored files")),
+		OPT_CALLBACK_F('e', "exclude", &exclude_list, N_("pattern"),
+				N_("always exclude <pattern> from cleaning (overrides -x)"), PARSE_OPT_NONEG, exclude_cb),
+		OPT_BOOL(0, "remove-excluded", &remove_excluded,
+				N_("remove excluded files, too (overrides -e and clean.exclude)")),
 		OPT_END()
 	};
 
@@ -1016,7 +1025,10 @@  int cmd_clean(int argc,
 	if (repo_read_index(the_repository) < 0)
 		die(_("index file corrupt"));
 
-	add_patterns_from_string_list(&dir, EXC_CMDL, "--exclude option", &exclude_list);
+	if (!remove_excluded) {
+		add_patterns_from_string_list(&dir, EXC_CMDL, "--exclude option", &exclude_list);
+		add_patterns_from_string_list(&dir, EXC_CMDL, "clean.exclude", &config_exclude_list);
+	}
 
 	parse_pathspec(&pathspec, 0,
 		       PATHSPEC_PREFER_CWD,
@@ -1091,6 +1103,7 @@  int cmd_clean(int argc,
 	strbuf_release(&buf);
 	string_list_clear(&del_list, 0);
 	string_list_clear(&exclude_list, 0);
+	string_list_clear(&config_exclude_list, 0);
 	clear_pathspec(&pathspec);
 	return (errors != 0);
 }