Re: [PATCH 2/2] read-tree: at least one tree-ish argument is required

Johannes Sixt <j.sixt@viscovery.net> writes:

No. I think saying "is not allowed" is going a bit too far [*1*]. The documentation does not list it as a commonly useful thing, that's all.

When a user wants to have an empty index, and does not want to touch files under $GIT_DIR with any non-git tools (e.g. "rm -f $GIT_DIR/index) for some religious reasons, "read-tree" without a tree would be one valid way to do so [*2*]. That is not documented (the synopsis section even hints that read-tree wants to take at least one tree), and I think it is a problem that the documentation does not match what the program actually allows.

The approach taken by [2/2] is to match the program to the documentation by forbidding what has been allowed and what people may have been relying on being able to do,

It was obvious that the line removed by [1/2] was a no-op not only in the usual case but also in an error case (i.e. when the user does not have write access to the repository), as I wrote in my review of the patch. Compared to that, I do not think it is cut-and-dried clear that [2/2] is the right kind of improvement. An alternative approach to document the zero-tree case would be a valid way to address the documentation mismatch problem.

We need to make arguments like "'read-tree' given by mistake to empty the index is risky", "'read-tree' as 'rm -f $GIT_DIR/index' replacement has little value", and "'read-tree' as 'rm -f $GIT_DIR/index' replacement is the best way to get an empty index" to weigh pros and cons between two possible approaches before deciding which way to go, but in a pre-release freeze, I wasn't in the mood to be one who is doing the arguments myself.

[Footnote]

*1* Didn't I say that somewhere?

*2* I suspect "rm --cached ." might be another, but it would probably be much more expensive.