Quoting Jeff King <p...@peff.net>:

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.

Well, to be honest, I thought it's not a big deal, too, but I also thought that the patch will be quickly shot down during review if I don't include some kind of a defense :) I'd be happier to reroll treating the current behavior in the clean orphan case as a bug, marking that test as expect_failure, and then just fixing it without the ORPHAN_OK thing.

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.

Yeah, right... It should be checked in the main code path, in git_dir_init(), but then it could hurt scripts that already do the right thing on an orphan branch because they don't set the ORPHAN_OK variable. Though that's probably not a big deal in practice either. Anyway, as it stands the documentation update of this patch is wrong.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to