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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:

> You have queued 1/2 (filter-branch: remove an unnecessary use of
> 'git read-tree') of this 2-patch series, but I haven't seen any comments
> about this 2/2 nor is it queued. Did it fall through the cracks?

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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]