Re: [PATCH] Introduce "precious" file concept

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 12, 2019, 21:54 UTC
Message-ID: <875zriga8f.fsf@evledraar.gmail.com>
In-Reply-To: <20190409102649.22115-1-pclouds@gmail.com>

On Tue, Apr 09 2019, Nguyễn Thái Ngọc Duy wrote:

>  Here's the replacement patch that keeps "git clean" behavior the same
>  as before and only checks 'precious' attribute when --keep-precous is
>  specified.

Cool to have the expected interaction with -x. Thanks.

> -'git clean' [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <path>...
> +'git clean' [<options>] [-d] [-f] [-i] [-n] [-x | -X] [--] <path>...

For e.g. git-status(1) we just say:

    git status [<options>...] [--] [<pathspec>...]

And for git-add(1) we do:

     git add [--verbose | -v] <very long exhaustive list of options
                              spanning 4 lines omitted>

Seems we should do one or the other here, i.e. either just add --keep-precious to the list, or leave it at just:

    git clean [<options>...] [--] [<pathspec>...]

> +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.

As noted upthread I think it's better to start with "clean" and "--keep-noclean", we can always alias it to "precious" later without squatting on that more general term when we (IMO) don't have the full picture yet & know if we even want that...

But anyway, with that out of the way and assuming this is kept-as is seems we could document this better if we're going to keep "precious", e.g. maybe:

    This attribute is set on files to indicate that they're important
    while not being tracked. This attribute is experimental and subject
    to future change as more commands are changed to support it.

    Now it's only supported by linkgit:git-clean[1] which'll skip
    cleaning files marked ith `precious` when given the
    `--keep-precious` option. This can be useful in combination with
    linkgit:gitignore[5] to e.g. mark `*.o` build assets as both ignored
    and precious.

I.e. say it's still early days, that it's "experimental" (not insisting on that phrasing, but somehow signaling to users that if they set this now it may do new/unexpected things in the future), and briefly describe how it works with "clean" and what the main intended use-case is.

Show 28 quoted lines

> +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
> +		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
> +	)
> +'

AFAICT this is the first attribute intended purely to be set on files that aren't tracked. I wonder if we should test for setting it on files that are tracked, and whether we should e.g. warn about that? Maybe not, but just raising it since I don't think it was discussed already...