Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> A new attribute "precious" is added to indicate that certain files
> have valuable content and should not be easily discarded even if they
> are ignored or untracked.
>
> So far there are one part of Git that are made aware of precious files:
s/are/is/g
> "git clean" will leave precious files alone if --keep-precious is
> specified.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> Here's the replacement patch that keeps "git clean" behavior the same
> as before and only checks 'precious' attribute when --keep-precous is
> specified.
OK. So this is even gentler introduction, which we could turn into
the default once more commands start honoring the attribute and
wider adoption proves that it is a good feature, while allowing us a
room to back out by keeping it an optional feature and eventually
deprecate and remove if the experiment did not pan out.
> +--keep-precious::
> + Do not remove untracked or ignored files if they have
> + `precious` attribute.
> +Precious files
> +~~~~~~~~~~~~~~
> +
> +`precious`
> +^^^^^^^^^^
> +
> +This attribute is set on files to indicate that their content is
> +valuable. Some commands will behave slightly different on precious
> +files. linkgit:git-clean[1] may leave precious files alone.
When the second thing that cares about the attribute comes along,
the mention of 'git clean' here will become the first item in a
bullet list, while the second and subsequent ones are listed next to
it. It may not be bad to start that enumerated list of count 1 from
the get-go, but for now this will do.
> @@ -193,9 +203,16 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
>
> strbuf_setlen(path, len);
> strbuf_addstr(path, e->d_name);
> - if (lstat(path->buf, &st))
> + if (lstat(path->buf, &st)) {
> ; /* fall thru */
> - else if (S_ISDIR(st.st_mode)) {
> + } else if ((!prefix && skip_precious_file(&the_index, path->buf)) ||
> + (prefix && skip_prefix(path->buf, prefix, &rel_path) &&
> + skip_precious_file(&the_index, rel_path))) {
> + quote_path_relative(path->buf, prefix, "ed);
> + printf(dry_run ? _(msg_would_skip_precious) : _(msg_skip_precious), quoted.buf);
> + *dir_gone = 0;
> + continue;
An attribute is given to something that can be tracked, and a
directory would not get an attribute, because Git does not track
directories (there is a reason why skip_precious_file() takes
&the_index that is passed down the callchain to git_check_attr()).
Triggering this logic before excluding S_ISDIR(st.st_mode) feels
iffy.
But let's assume that being able to say "this directory and anything
(recursively) inside are precious" is a good idea and read on.
> + } else if (S_ISDIR(st.st_mode)) {
> if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
> ret = 1;
> if (gone) {
> @@ -915,6 +932,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> OPT_BOOL('x', NULL, &ignored, N_("remove ignored files, too")),
> OPT_BOOL('X', NULL, &ignored_only,
> N_("remove only ignored files")),
> + OPT_BOOL(0, "keep-precious", &keep_precious,
> + N_("do not remove files with 'precious' attribute")),
> OPT_END()
> };
OK.
> @@ -1019,7 +1038,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> if (lstat(abs_path.buf, &st))
> continue;
>
> - if (S_ISDIR(st.st_mode)) {
> + if (skip_precious_file(&the_index, item->string)) {
> + qname = quote_path_relative(item->string, NULL, &buf);
> + printf(dry_run ? _(msg_would_skip_precious) : _(msg_skip_precious), qname);
> + } else if (S_ISDIR(st.st_mode)) {
> if (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone))
Likewise.
> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
> index 7b36954d63..ae600dafb5 100755
> --- a/t/t7300-clean.sh
> +++ b/t/t7300-clean.sh
> @@ -669,4 +669,44 @@ test_expect_success 'git clean -d skips untracked dirs containing ignored files'
> test_path_is_missing foo/b/bb
> '
>
> +test_expect_success 'git clean -xd --keep-precious leaves precious files alone' '
> + git init precious &&
> + (
> + cd precious &&
> + test_commit one &&
> + cat >.gitignore <<-\EOF &&
> + *.o
> + *.mak
> + EOF
> + cat >.gitattributes <<-\EOF &&
> + *.mak precious
> + .gitattributes precious
> + *.precious precious
If the checking with skip_precious() before S_ISDIR() was
intentional (I cannot quite tell if it is), then this should also
have a pattern that matches a directory and mark it as precious.
On the other hand, if it was merely a thinko and the change does not
intend to allow attaching attributes to directories, then this test
is probably OK as-is. We also _could_ have a pattern that matches a
directory and attempt to make it precious and then make sure the
directory goes away (i.e. any attribute, including 'precious', on
the directory was meaningless).
> + EOF
> + mkdir sub &&
> + touch one.o sub/two.o one.mak sub/two.mak &&
> + touch one.untracked two.precious sub/also.precious &&
> + git clean -fdx --keep-precious &&
> + test_path_is_missing one.o &&
> + test_path_is_missing sub/two.o &&
> + test_path_is_missing one.untracked &&
> + test_path_is_file .gitattributes &&
> + test_path_is_file one.mak &&
> + test_path_is_file sub/two.mak &&
> + test_path_is_file two.precious &&
> + test_path_is_file sub/also.precious
> + )
> +'
> +test_expect_success 'git clean -xd still deletes them all' '
OK, so this is exactly the same command as above, but without the
"--keep-precious" option. It is good to test both positive and
negative.
> + test_path_is_file precious/one.mak &&
> + test_path_is_file precious/sub/two.mak &&
> + test_path_is_file precious/two.precious &&
> + test_path_is_file precious/sub/also.precious &&
> + git -C precious clean -fdx &&
> + test_path_is_missing precious/one.mak &&
> + test_path_is_missing precious/sub/two.mak &&
> + test_path_is_missing precious/two.precious &&
> + test_path_is_missing precious/sub/also.precious
> +'
