git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Introduce "precious" file concept

From
Junio C Hamano <gitster@pobox.com>
Date
Apr 9, 2019, 11:31 UTC
Message-ID
<xmqqh8b75s9l.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20190409102649.22115-1-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 5 quoted lines
> 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
Show 8 quoted lines
> "git clean" will leave precious files alone if --keep-precious is
> specified.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  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.
Show 9 quoted lines
> +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.

Show 15 quoted lines
> @@ -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, &quoted);
> +			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.

Show 12 quoted lines
> +		} 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.
Show 10 quoted lines
> @@ -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.
Show 21 quoted lines
> 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).

Show 15 quoted lines
> +		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.

Show 10 quoted lines
> +	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
> +'
Previous: Nguyễn Thái Ngọc DuyNext: Duy Nguyen
Message 7 of 19 in “What's cooking in git.git (Apr 2019, #01; Thu, 4)”
  1. Junio C HamanoApr 4, 2019
  2. Duy NguyenApr 4, 2019
  3. Junio C HamanoApr 4, 2019
  4. Ævar Arnfjörð BjarmasonApr 6, 2019
  5. Junio C HamanoApr 8, 2019
  6. Introduce "precious" file conceptNguyễn Thái Ngọc Duy, Apr 9, 2019
  7. Junio C HamanoApr 9, 2019
  8. Duy NguyenApr 10, 2019
  9. Junio C HamanoApr 12, 2019
  10. Eric SunshineApr 9, 2019
  11. Ævar Arnfjörð BjarmasonApr 12, 2019
  12. Duy NguyenApr 13, 2019
  13. Todd ZullingerApr 5, 2019
  14. Junio C HamanoApr 5, 2019
  15. Ævar Arnfjörð BjarmasonApr 6, 2019
  16. Junio C HamanoApr 8, 2019
  17. Ævar Arnfjörð BjarmasonApr 6, 2019
  18. Junio C HamanoApr 8, 2019
  19. Josh SteadmonApr 8, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.