Re: sj/untracked-files-in-submodule-directory-is-not-dirty, was Re: What's cooking in git.git (Oct 2020, #04; Tue, 27)

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 3, 2020, 01:01 UTC
Message-ID: <xmqqa6vzqmmc.fsf@gitster.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.2011020251520.18437@tvgsbejvaqbjf.bet>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 14 quoted lines

>> * sj/untracked-files-in-submodule-directory-is-not-dirty (2020-10-26) 1 commit
>>  - diff: do not show submodule with untracked files as "-dirty"
>>
>>  "git diff" showed a submodule working tree with untracked cruft as
>>  "Submodule commit <objectname>-dirty", but a natural expectation is
>>  that the "-dirty" indicator would align with "git describe --dirty",
>>  which does not consider having untracked files in the working tree
>>  as source of dirtiness.  The inconsistency has been fixed.
>>
>>  Needs doc update.
>
> I *think* the original rationale for marking submodules with untracked
> (_un-ignored_) files was to avoid deleting a submodule that has
> uncommitted (because untracked) files.

I agree with you that that the motivation was exactly that, but I have a suspicion that its execution was misguided.

> This patch affects e.g. if some script wants to determine whether a
> submodule can be deleted safely and uses `git diff` to do so. With this
> patch, it is possible to lose data irretrievably.

Let's play devil's advocate.

When one has a subdirectory D where one might or might not have an untracked new file that one does not want to lose, but one otherwise would want to clean up (perhaps an errant process created tons of garbage files in the directory), one would not

	git diff D

to see if there are important changes in that directory before doing

	rm -rf D && git checkout D

to bring it back to pristine state, exactly because one would not want to lose newly-created but not added files. One would instead use

	git status D

for checking. Why would a user change the habit when D happens to be a submodule?

So I would say that "git status [D]", if it does not let the user notice that there is untracked contents in the submodule working tree, would be a bad idea. "git diff" that ignores untracked paths in the submodule working tree, on the other hand, is a good thing.

Now, the patch as-is may change the behaviour of "git status D" in this case, and the internal invocation of diff-files made somewhere in wt-status.c may have to be adjusted to keep such a submodule with forgotten newly created files shown as modified.

Further discussion on this should be done on the original review thread for continuity, not here, in any case.

Thanks.