Re: [PATCH 2/2] sh-setup: make require_clean_work_tree() work on orphan branches

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

 



Jeff King <peff@xxxxxxxx> writes:

> On Tue, Nov 24, 2015 at 03:45:45PM +0100, SZEDER Gábor wrote:
>
>> git-sh-setup's require_clean_work_tree() always exits with error on an
>> orphan branch, even when the index and worktree are both clean.  The
>> reason is that require_clean_work_tree() starts off with verifying
>> HEAD, to make sure that it can safely pass HEAD to 'git diff-index'
>> later when it comes to checking the cleanness of the index, and errors
>> out finding the invalid HEAD of the orphan branch.
>> 
>> There are scripts requiring a clean worktree that should work on an
>> orphan branch as well, and those should be able to use this function
>> instead of duplicating its functionality and nice error reporting in a
>> way that handles orphan branches.
>> 
>> Fixing this is easy: the index should be compared to the empty tree
>> while on an orphan branch, and to HEAD otherwise.
>> 
>> However, just fixing require_clean_work_tree() this way is also
>> dangerous, because scripts must take care to work properly on orphan
>> branches.  Currently a script calling require_clean_work_tree() would
>> exit on a clean orphan branch, but with the simple fix it would
>> continue executing and who knows what the consequences might be if
>> the script is not prepared for orphan branches.
>
> Hmm. I suspect this is not a big deal in practice. Lots of scripts
> (including some of our own, through history) get the orphan case wrong.

The state of the repository this topic wants to deal with better
would be the same as the state you would get after "tar xf ... &&
git init && git add .", no?  In the "orphan" case, unlike such a
plain vanilla "created a new repository" case where the index could
be empty for a long time before the initial 'git add', the index is
almost always populated, so most likely this change will not make
much difference in that require_clean_work_tree() would stop
operation until "rm --cached ." empties the index.

And if the user did "rm --cached ." to empty the index, it is fine
for us to happily consider all these files as untracked, even when
HEAD is not pointing at an existing ref.  So it is likely that the
added ORPHAN_OK knob would be unnecessary for "orphan" case.

However, I suspect that this would affect the users in the "I am a
new user, I just initialized my first Git repository and I haven't
done a 'git add' yet" status much more.  Do we have corner cases
where everyday commands do not work well while on an unborn branch
immediately after 'git init'?  That is the kind of bug that we need
to protect users from, either by keeping the "No HEAD yet, no operation
that requires clean working tree" logic, or by fixing such bugs.

> I'm not sure that require_clean_work_tree is necessarily the place to be
> enforcing it, even though it happened to have done so historically.

It is unclear to me what you meant by "it" in "enforcing it".

> Still, it may be prudent to err on the side of caution. I'm on the
> fence.

--
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]