Re: Submodule/contents conflict

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 27, 2017, 00:25 UTC
Message-ID: <xmqqmvb2r91e.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kZEveo8jQodvd0n6fEXc1OXDVa26BCumM0etjst74F_Hw@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 6 quoted lines

> I assumed a use case like this:
>
>   A user may want to extract a file from a given tree-ish via
>   GIT_WORK_TREE=/tmp/place git checkout <tree> -- <file>
>   without modifying the repository (i.e. index) at all. For this
>   we'd need an option to modify the working tree only.

I somehow do not see that as a very useful use case. It is not just keeping the index, but it is depositing the file also out of the working tree, and if you do not want to work with the resulting <file> (actually, if it is a single file, "cat-file -t blob" is the most natural thing to use for such an operation, so you should say <pathspec> that may match multiple files and perhaps a collection of directories) within the context of your working tree, I would think that "archive | tar xf -" is a better choice.

The feature is more about in-tree case. You clobber what you did in your working tree by extracting a pristine copy out of some known tree, which may not be HEAD. And if the tree is not HEAD, you would want "git diff <pathspec>" would give a useful preview of what would happen when you do "git add" them. As "checkout" adds to the index before updating the working tree (which can be argued as an ancient design mistake, given that we typically do worktree-only by default and have "--index/--cached" to work with the index, e.g. "apply", "grep"), the user needs to do "git diff HEAD <pathspec>" instead in this case.

> I never realized this as a usual convention explicitly. Thanks for pointing
> it out.

I think it is somewhere in gitcli(7).

Show 6 quoted lines

>> +       if (opts->no_index && !opts->source_tree)
>> +               die(_("'--working-tree-only' cannot be used without tree-ish"));
>
> double negation, maybe:
>
>   "--working-tree-only requires tree-ish"

Much more concise.  Thanks.

Show 12 quoted lines

>> +test_expect_success 'working-tree-only option leaves checked out files unadded' '
>> +       git reset --hard &&
>> +       git checkout -b pu next &&
>> +       echo another >>file1 &&
>> +       echo exists >file3 &&
>> +       git add file3 &&
>> +       git commit -a -m another &&
>> +       git checkout next &&
>
> Up to here it is all preparation; I started to give an argument
> on why using "another" for both the commit message and the file content
> was suboptimal, but I was wrong. This seems to be best after some consideration.

It does add to the mental burden. Will think of a pair of different words and phrases.

Show 14 quoted lines

> The next paragraph checks for
> 'working-tree-only option populates the working tree, but doesn't touch index'
>
>> +       ! grep another file1 &&
>> +       git checkout --working-tree-only pu file1 file3 &&
>> +       grep another file1 &&
>> +       test_must_fail git grep --cached another file1 &&
>
> but only for file1, whereas the next paragraph checks it for file3.
>
>> +       grep exists file3 &&
>> +       git ls-files file3 >actual &&
>> +       >expect &&
>> +       test_cmp expect actual

Yes. This demonstrates that paths only in tree-ish is not added to the index (i.e. ls-files does not show it, because HEAD and the original index didn't have it). If you did the same "grep --cached" that expects a non-match, you cannot tell if we added the path with the --intent-to-add or if we didn't add it at all, so it deliberately checks different thing from the case for file1.