From: Junio C Hamano <gitster@pobox.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: avarab@gmail.com, git@vger.kernel.org
Subject: Re: [PATCH] Introduce "precious" file concept
Date: Tue, 09 Apr 2019 20:31:50 +0900 [thread overview]
Message-ID: <xmqqh8b75s9l.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190409102649.22115-1-pclouds@gmail.com> ("Nguyễn Thái Ngọc Duy"'s message of "Tue, 9 Apr 2019 17:26:49 +0700")
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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@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.
> +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
> +'
next prev parent reply other threads:[~2019-04-09 11:31 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-04 10:28 What's cooking in git.git (Apr 2019, #01; Thu, 4) Junio C Hamano
2019-04-04 11:08 ` Duy Nguyen
2019-04-04 21:29 ` Junio C Hamano
2019-04-06 20:28 ` Ævar Arnfjörð Bjarmason
2019-04-08 4:14 ` Junio C Hamano
2019-04-09 10:26 ` [PATCH] Introduce "precious" file concept Nguyễn Thái Ngọc Duy
2019-04-09 11:31 ` Junio C Hamano [this message]
2019-04-10 9:36 ` Duy Nguyen
2019-04-12 1:28 ` Junio C Hamano
2019-04-09 17:44 ` Eric Sunshine
2019-04-12 21:54 ` Ævar Arnfjörð Bjarmason
2019-04-13 10:19 ` Duy Nguyen
2019-04-05 1:05 ` What's cooking in git.git (Apr 2019, #01; Thu, 4) Todd Zullinger
2019-04-05 5:41 ` Junio C Hamano
2019-04-06 19:28 ` Ævar Arnfjörð Bjarmason
2019-04-08 4:18 ` Junio C Hamano
2019-04-06 19:57 ` Ævar Arnfjörð Bjarmason
2019-04-08 4:28 ` Junio C Hamano
2019-04-08 21:18 ` Josh Steadmon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqh8b75s9l.fsf@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).