Bug in "revision.c: --all adds HEAD from all worktrees" ?

Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Luke Diamand]

Hi!

I think there may be a regression caused by this change which means
that "git fetch origin" doesn't work:

commit d0c39a49ccb5dfe7feba4325c3374d99ab123c59 (refs/bisect/bad)
Author: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date:   Wed Aug 23 19:36:59 2017 +0700

    revision.c: --all adds HEAD from all worktrees

$ git fetch origin
fatal: bad object HEAD
error: ssh://my_remote_host/reponame did not send all necessary objects

I used git bisect to find the problem, and it seems pretty consistent.
"git fetch" with the previous revision works fine.

FWIW, I've got a lot of git worktrees associated with this repo, so
that may be why it's failing. The remote repo is actually a git-p4
clone, so HEAD there actually ends up pointing at
refs/remote/p4/master.

Thanks,
Luke

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Luke Diamand]

On 13 November 2017 at 19:51, Luke Diamand <luke@diamand.org> wrote:
> Hi!
>
> I think there may be a regression caused by this change which means
> that "git fetch origin" doesn't work:
>
> commit d0c39a49ccb5dfe7feba4325c3374d99ab123c59 (refs/bisect/bad)
> Author: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Date:   Wed Aug 23 19:36:59 2017 +0700
>
>     revision.c: --all adds HEAD from all worktrees
>
> $ git fetch origin
> fatal: bad object HEAD
> error: ssh://my_remote_host/reponame did not send all necessary objects
>
> I used git bisect to find the problem, and it seems pretty consistent.
> "git fetch" with the previous revision works fine.
>
> FWIW, I've got a lot of git worktrees associated with this repo, so
> that may be why it's failing. The remote repo is actually a git-p4
> clone, so HEAD there actually ends up pointing at
> refs/remote/p4/master.
>
> Thanks,
> Luke

Quite a few of the worktrees have expired - their head revision has
been GC'd and no longer points to anything sensible
(gc.worktreePruneExpire). The function other_head_refs() in worktree.c
bails out if there's an error, which I think is the problem. I wonder
if it should instead just report something and then keep going.

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Stefan Beller]

On Mon, Nov 13, 2017 at 2:03 PM, Luke Diamand <luke@diamand.org> wrote:
> On 13 November 2017 at 19:51, Luke Diamand <luke@diamand.org> wrote:
>> Hi!
>>
>> I think there may be a regression caused by this change which means
>> that "git fetch origin" doesn't work:
>>
>> commit d0c39a49ccb5dfe7feba4325c3374d99ab123c59 (refs/bisect/bad)
>> Author: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> Date:   Wed Aug 23 19:36:59 2017 +0700
>>
>>     revision.c: --all adds HEAD from all worktrees
>>
>> $ git fetch origin
>> fatal: bad object HEAD
>> error: ssh://my_remote_host/reponame did not send all necessary objects
>>
>> I used git bisect to find the problem, and it seems pretty consistent.
>> "git fetch" with the previous revision works fine.
>>
>> FWIW, I've got a lot of git worktrees associated with this repo, so
>> that may be why it's failing. The remote repo is actually a git-p4
>> clone, so HEAD there actually ends up pointing at
>> refs/remote/p4/master.
>>
>> Thanks,
>> Luke
>
> Quite a few of the worktrees have expired - their head revision has
> been GC'd and no longer points to anything sensible
> (gc.worktreePruneExpire). The function other_head_refs() in worktree.c
> bails out if there's an error, which I think is the problem. I wonder
> if it should instead just report something and then keep going.

Also see
https://public-inbox.org/git/CAGZ79kYP0z1G_H3nwfmSHraWHMBOcik5LepUXKj0nveeBrihiw@mail.gmail.com/

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Luke Diamand]

+Jeff King

On 13 November 2017 at 22:15, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Nov 13, 2017 at 2:03 PM, Luke Diamand <luke@diamand.org> wrote:
>> On 13 November 2017 at 19:51, Luke Diamand <luke@diamand.org> wrote:
>>> Hi!
>>>
>>> I think there may be a regression caused by this change which means
>>> that "git fetch origin" doesn't work:
>>>
>>> commit d0c39a49ccb5dfe7feba4325c3374d99ab123c59 (refs/bisect/bad)
>>> Author: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> Date:   Wed Aug 23 19:36:59 2017 +0700
>>>
>>>     revision.c: --all adds HEAD from all worktrees
>>>
>>> $ git fetch origin
>>> fatal: bad object HEAD
>>> error: ssh://my_remote_host/reponame did not send all necessary objects
>>>
>>> I used git bisect to find the problem, and it seems pretty consistent.
>>> "git fetch" with the previous revision works fine.
>>>
>>> FWIW, I've got a lot of git worktrees associated with this repo, so
>>> that may be why it's failing. The remote repo is actually a git-p4
>>> clone, so HEAD there actually ends up pointing at
>>> refs/remote/p4/master.
>>>
>>> Thanks,
>>> Luke
>>
>> Quite a few of the worktrees have expired - their head revision has
>> been GC'd and no longer points to anything sensible
>> (gc.worktreePruneExpire). The function other_head_refs() in worktree.c
>> bails out if there's an error, which I think is the problem. I wonder
>> if it should instead just report something and then keep going.
>
> Also see
> https://public-inbox.org/git/CAGZ79kYP0z1G_H3nwfmSHraWHMBOcik5LepUXKj0nveeBrihiw@mail.gmail.com/

So is this a bug or user error on my part?

Surely at the very least "git fetch" shouldn't give a cryptic error
message just because one of my git worktrees has expired!

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Junio C Hamano]

Luke Diamand <luke@diamand.org> writes:

> Quite a few of the worktrees have expired - their head revision has
> been GC'd and no longer points to anything sensible
> (gc.worktreePruneExpire). The function other_head_refs() in worktree.c
> bails out if there's an error, which I think is the problem. I wonder
> if it should instead just report something and then keep going.

Am I correct to understand that your "git fsck" would fail because
these HEAD refs used by other stale worktrees are pointing at
missing objects?

What do you mean by "expired"?  "Even though I want to keep using
them, Git for some reason decided to destroy them." or "I no longer
use them but kept them lying around."?

If the latter, I wonder "worktree prune" to remove the
admininstrative information for them would unblock you?

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Luke Diamand]

On 15 November 2017 at 22:08, Junio C Hamano <gitster@pobox.com> wrote:
> Luke Diamand <luke@diamand.org> writes:
>
>> Quite a few of the worktrees have expired - their head revision has
>> been GC'd and no longer points to anything sensible
>> (gc.worktreePruneExpire). The function other_head_refs() in worktree.c
>> bails out if there's an error, which I think is the problem. I wonder
>> if it should instead just report something and then keep going.
>
> Am I correct to understand that your "git fsck" would fail because
> these HEAD refs used by other stale worktrees are pointing at
> missing objects?

git fsck says:

Checking object directories: 100% (256/256), done.
Checking objects: 100% (1434634/1434634), done.
error: HEAD: invalid reflog entry
7fa2b7ee4bc0d11529f659db8b13ed1f537d2a98
error: HEAD: invalid reflog entry
7fa2b7ee4bc0d11529f659db8b13ed1f537d2a98
error: HEAD: invalid reflog entry
7e79e09e8a7382f91610f7255a1b99ea59f68c0b
error: refs/stash: invalid reflog entry
feeb35e7b045d28943c706e761d0a2ac8206af2f
error: refs/remotes/origin/master: invalid reflog entry
7fa2b7ee4bc0d11529f659db8b13ed1f537d2a98
Checking connectivity: 1419477, done.
missing tree 1480c0a7ed2ad59ae701667292399c38d294658e
missing tree ca2a01116bfbbd1fcbcf9812b95d8dc6c39e69d5
missing tree 5b7c41e547fc5c4c840e5b496da13d3daebc5fbe
...
...

>
> What do you mean by "expired"?  "Even though I want to keep using
> them, Git for some reason decided to destroy them." or "I no longer
> use them but kept them lying around."?

git worktree automatically prunes work trees:

"The working tree’s administrative files in the repository (see
"DETAILS" below) will eventually be removed automatically (see
gc.worktreePruneExpire in git-config(1)),"

In my case I didn't actually want them removed, but fortunately
there's nothing important in them (there certainly isn't anymore...).

>
> If the latter, I wonder "worktree prune" to remove the
> admininstrative information for them would unblock you?

It doesn't seem to help.

$ git worktree prune -n
<lists lots of unhappy trees>
$ git worktree prune
$ git fetch
remote: Counting objects: 35, done.
remote: Compressing objects: 100% (20/20), done.
remote: Total 21 (delta 17), reused 5 (delta 1)
Unpacking objects: 100% (21/21), done.
fatal: bad object HEAD
error: ssh://whatever/myrepol did not send all necessary objects
$ /usr/bin/git-2.7.3 fetch
<works fine>

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Jeff King]

On Wed, Nov 15, 2017 at 09:38:26PM +0000, Luke Diamand wrote:

> >> Quite a few of the worktrees have expired - their head revision has
> >> been GC'd and no longer points to anything sensible
> >> (gc.worktreePruneExpire). The function other_head_refs() in worktree.c
> >> bails out if there's an error, which I think is the problem. I wonder
> >> if it should instead just report something and then keep going.
> >
> > Also see
> > https://public-inbox.org/git/CAGZ79kYP0z1G_H3nwfmSHraWHMBOcik5LepUXKj0nveeBrihiw@mail.gmail.com/
> 
> So is this a bug or user error on my part?

It's a bug. You didn't do anything wrong, but the worktree feature
corrupted your repository.

-Peff

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Ævar Arnfjörð Bjarmason]

On Thu, Nov 16 2017, Luke Diamand jotted:

> On 15 November 2017 at 22:08, Junio C Hamano <gitster@pobox.com> wrote:
>> Luke Diamand <luke@diamand.org> writes:
>>
>>> Quite a few of the worktrees have expired - their head revision has
>>> been GC'd and no longer points to anything sensible
>>> (gc.worktreePruneExpire). The function other_head_refs() in worktree.c
>>> bails out if there's an error, which I think is the problem. I wonder
>>> if it should instead just report something and then keep going.
>>
>> Am I correct to understand that your "git fsck" would fail because
>> these HEAD refs used by other stale worktrees are pointing at
>> missing objects?
>
> git fsck says:
>
> Checking object directories: 100% (256/256), done.
> Checking objects: 100% (1434634/1434634), done.
> error: HEAD: invalid reflog entry
> 7fa2b7ee4bc0d11529f659db8b13ed1f537d2a98
> error: HEAD: invalid reflog entry
> 7fa2b7ee4bc0d11529f659db8b13ed1f537d2a98
> error: HEAD: invalid reflog entry
> 7e79e09e8a7382f91610f7255a1b99ea59f68c0b
> error: refs/stash: invalid reflog entry
> feeb35e7b045d28943c706e761d0a2ac8206af2f
> error: refs/remotes/origin/master: invalid reflog entry
> 7fa2b7ee4bc0d11529f659db8b13ed1f537d2a98
> Checking connectivity: 1419477, done.
> missing tree 1480c0a7ed2ad59ae701667292399c38d294658e
> missing tree ca2a01116bfbbd1fcbcf9812b95d8dc6c39e69d5
> missing tree 5b7c41e547fc5c4c840e5b496da13d3daebc5fbe
> ...
> ...
>
>>
>> What do you mean by "expired"?  "Even though I want to keep using
>> them, Git for some reason decided to destroy them." or "I no longer
>> use them but kept them lying around."?
>
> git worktree automatically prunes work trees:
>
> "The working tree’s administrative files in the repository (see
> "DETAILS" below) will eventually be removed automatically (see
> gc.worktreePruneExpire in git-config(1)),"
>
> In my case I didn't actually want them removed, but fortunately
> there's nothing important in them (there certainly isn't anymore...).
>
>>
>> If the latter, I wonder "worktree prune" to remove the
>> admininstrative information for them would unblock you?
>
> It doesn't seem to help.
>
> $ git worktree prune -n
> <lists lots of unhappy trees>
> $ git worktree prune
> $ git fetch
> remote: Counting objects: 35, done.
> remote: Compressing objects: 100% (20/20), done.
> remote: Total 21 (delta 17), reused 5 (delta 1)
> Unpacking objects: 100% (21/21), done.
> fatal: bad object HEAD
> error: ssh://whatever/myrepol did not send all necessary objects
> $ /usr/bin/git-2.7.3 fetch
> <works fine>

Digging up this old thread. I've also noticed this bug because I tried
to "git repack -A -d" a repo on a GitLab server and just got "fatal: bad
object HEAD".

Bisect revealed that the reason was that GitLab itself runs 2.14.3,
which is the last release version that doesn't have Duy's d0c39a49cc
("revision.c: --all adds HEAD from all worktrees", 2017-08-23), and that
the HEAD revision of some worktrees was corrupt (GitLab creates squash-*
worktrees for some reason).

Doing a "git worktree prune" beforehand makes it work.

This can be reproduced with:

    (
        rm -rf /tmp/git &&
        git clone --bare https://github.com/git/git.git /tmp/git &&
        cd /tmp/git &&
        git worktree add blah HEAD &&
        echo 1111111111111111111111111111111111111111 > worktrees/blah/HEAD
    )

Now, obviously the root cause is that the repo is corrupt through some
other bug (since fixed?), but the error message here is really bad, and
should at least say "fatal: bad object HEAD in worktree blah" or
something like that.

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Duy Nguyen]

On Mon, Mar 12, 2018 at 10:39 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Now, obviously the root cause is that the repo is corrupt through some
> other bug (since fixed?), but the error message here is really bad, and
> should at least say "fatal: bad object HEAD in worktree blah" or
> something like that.

I know. I know. I'm working on it. I hit a wall and had to re-think my
approach, but I think it should be ok this time. Will be posting a
series soonish, which should also fix fsck ignoring HEADs from other
worktrees.
-- 
Duy

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Stan Hu]

To be clear, this is how I think we got into this state:

We have worktrees that are created to squash and rebase a branch. They were left around inadvertently for one reason or another.

Since we were using git v2.14.3, a git gc would prune dangling objects because it never saw that a worktree still had a reference to it. Now, in git v2.15+, worktrees are examined, and a git gc won't garbage collect any objects referenced by a worktree.

Another words, the failure of git v2.14 to preserve these objects caused problems with these stale worktrees with the upgrade to v2.15+.

Re: Bug in "revision.c: --all adds HEAD from all worktrees" ?

[Ævar Arnfjörð Bjarmason]

On Tue, Mar 13 2018, Stan Hu jotted:

> To be clear, this is how I think we got into this state:
>
> We have worktrees that are created to squash and rebase a branch. They were left around inadvertently for one reason or another.
>
> Since we were using git v2.14.3, a git gc would prune dangling objects because it never saw that a worktree still had a reference to it. Now, in git v2.15+, worktrees are examined, and a git gc won't garbage collect any objects referenced by a worktree.
>
> Another words, the failure of git v2.14 to preserve these objects caused problems with these stale worktrees with the upgrade to v2.15+.

Thanks, I hadn't dug into the root cause. Good to know it's not some
failure mode we're still producing, just the result of past bugs.