Re: [PATCH 05/31] rebase: improve detection of rebase in progress

On Tue, 28 Dec 2010, Martin von Zweigbergk wrote:

On Tue, 28 Dec 2010, Junio C Hamano wrote:

quoted

Martin von Zweigbergk [off-list ref] writes:

quoted

Currently, the existence of rebase-merge/ is tested with 'test -d',
while the existence of rebase-apply/ is tested by creating the
directory and then deleting it again. Any good reason for this?

I don't recall how the merge side reached the current shape of the code,
but I think the rebase-apply one was that we wanted to make sure not only
we don't have a directory but also we actually _can_ create one.  If
somebody had a bad permission set, "test -d" wouldn't help us much.  We
would fail later and error diagnosis codepath should do the right thing
anyway, so it is not a correctness issue, but is more about attempting to
notice an error early rather than late.

Yeah, I was wondering if that might have been the reason. Why would
anyone set such permissions on .git (keep in mind that I am no
Linux/Unix expert)?

I saw that the code was introduced [1] when the directory was called
.dotest and I believe this directory was created at the top level
(i.e. a sibling to .git). Could it be that it was reasonable for the
user to set such permissions back then (on the top level dir), but not
any longer (on the .git dir)?


Either way, if it is good to have the that check for rebase-apply/,
wouldn't it be good to have for rebase-merge/ as well?

/Martin


[1] 7f4bd5d (rebase: one safety net, one bugfix and one optimization.,
2005-11-28)

I'm working on a re-roll. Any input on this?