From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [BUG] Incosistent repository state when trying to rename HEAD in the middle of a rebase
Date: Fri, 3 Nov 2017 14:53:37 +0530 [thread overview]
Message-ID: <49563f7c-354e-334e-03a6-c3a40884b6d0@gmail.com> (raw)
In-Reply-To: <xmqqd151qft5.fsf@gitster.mtv.corp.google.com>
On Thursday 02 November 2017 01:21 PM, Junio C Hamano wrote:
> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:
>
>> I was able to spare some time to dig into this and found a few things.
>>
>> First, it seems that the issue is more generic and the BUG kicks in
>> whenever HEAD is not a symbolic ref.
>
> Interesting.
>
Let me detail a little more about my observations just for the sake of
completeness. The change that forbid refs/heads/HEAD caused issues only
when HEAD wasn't a symbolic link because of the following reasons,
1) The change resulted in 'strbuf_check_branch_ref' returning with
failure when the name it received (sb) was HEAD *without* interpreting
it as "refs/heads/HEAD" into 'ref'. This resulted in the violation of
the expectation of it's callers that it would have interpret 'ref' which
was the major cause of the issue.
It wouldn't have been an issue if we had checked for the existence of a
"branch" (refs/heads/) rather than checking for the existence of a "ref"
(which allowed HEAD to pass the test).
2) This did not cause issues when HEAD was a symbolic ref because there
was a check for attempting to rename in a symbolic ref in
'files_copy_or_rename_ref'. The check throws an error when trying to
rename a symbolic ref which resulted in suspicious error message I got.
So, IIUC the issue doesn't occur when 'ref' is intrepreted before the
check for 'HEAD'. That's possibly why the diff patch I sent in the
previous mail fixed the issue.
Also, it would be nice if we check for the existence of a "branch" when
we want to know whether a branch exists rather than simply doing a
'ref_exists' on the interpreted ref.
> Shortly we'll be rewinding the tip of 'next', and it is a good
> opportunity to kick any not-so-well-cooked topic back to 'pu',
> so let's figure out what is going on after that happens (at which
> point let's eject the "branch name sanity" topic out of 'next').
Does the above explanation give an idea about what's happening?
---
Kaartic
prev parent reply other threads:[~2017-11-03 9:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-28 16:58 [BUG] Incosistent repository state when trying to rename HEAD in the middle of a rebase Kaartic Sivaraam
2017-11-02 5:33 ` Kaartic Sivaraam
2017-11-02 7:51 ` Junio C Hamano
2017-11-03 9:23 ` Kaartic Sivaraam [this message]
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49563f7c-354e-334e-03a6-c3a40884b6d0@gmail.com \
--to=kaarticsivaraam91196@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).