From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: "SZEDER Gábor" <szeder@ira•uka.de>, git@vger•kernel.org
Subject: Re: [PATCH 2/2] sh-setup: make require_clean_work_tree() work on orphan branches
Date: Wed, 02 Dec 2015 15:07:05 -0800 [thread overview]
Message-ID: <xmqqwpsw4fhi.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20151124205036.GF7174@sigill.intra.peff.net> (Jeff King's message of "Tue, 24 Nov 2015 15:50:37 -0500")
Jeff King <peff@peff•net> 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.
next prev parent reply other threads:[~2015-12-02 23:07 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-24 14:45 [PATCH 1/2] Add tests for git-sh-setup's require_clean_work_tree() SZEDER Gábor
2015-11-24 14:45 ` [PATCH 2/2] sh-setup: make require_clean_work_tree() work on orphan branches SZEDER Gábor
2015-11-24 20:50 ` Jeff King
2015-11-30 12:37 ` SZEDER Gábor
2015-12-02 23:07 ` Junio C Hamano [this message]
2015-11-25 6:51 ` Johannes Sixt
2015-11-30 12:24 ` SZEDER Gábor
2016-01-21 0:06 ` Junio C Hamano
2016-01-20 23:59 ` [PATCH 1/2] Add tests for git-sh-setup's require_clean_work_tree() Junio C Hamano
2016-01-21 1:27 ` SZEDER Gábor
2016-01-21 4:52 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwpsw4fhi.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=peff@peff$(echo .)net \
--cc=szeder@ira$(echo .)uka.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox