RFC: How should we handle un-deleted remote branches?

RFC: How should we handle un-deleted remote branches?

[Ævar Arnfjörð Bjarmason]

I removed a remote and its refs persisted. At first I thought this was a
bug but looking at add_branch_for_removal()'s "don't delete a branch if
another remote also uses it" logic it's intentional.

This goes very wrong if you do e.g.:

    (
        rm -rf /tmp/git &&
        git clone --bare --mirror git@github.com:git/git.git /tmp/git &&
        cd /tmp/git &&
        git remote add avar  git@github.com:avar/git.git &&
        git fetch avar &&
        git remote remove avar &&
        git for-each-ref|grep -c refs/remotes/avar
    )

At this point you can't prune it since no remote config is left:

    $ git remote prune avar
    fatal: 'avar' does not appear to be a git repository
    fatal: Could not read from remote repository.

But this is a possible work-around:

    git init /tmp/empty.git
    git remote add avar file:///tmp/empty.git
    git remote prune avar
    git remote remove avar

Or use update-ref -d to remove them:

    for rr in $(git for-each-ref --format="%(refname)"|grep ^refs/remotes/avar/)
    do
        git update-ref -d $rr
    done

I started to patch this, but I'm not sure what to do here. we could do
some combination of:

 0. Just document the current behavior and leave it.

 1. Dig further down to see what other remotes reference these refs, and
    just ignore any refspecs that don't explicitly reference
    refs/remotes/<our_deleted_remote>/*.

    I.e. isn't the intention here to preserve a case where you have two
    URLs for the same effective remote, not whene you have something
    like a --mirror refspec? Unfortunately I can't ask the original
    author :(

 2. Warn about each ref we didn't delete, or at least warn saying
    there's undeleted refs under refs/remotes/<name>/*.

 3. Make 'git remote remove --force-deletion <name>' (or whatever the
    flag is called) be a thing. But unless we do the next item this
    won't be useful.

 4. Make 'git remote prune <name>' work in cases where we don't have a
    remote called <name> anymore, just falling back to deleting
    refs/remotes/<name>. In this case 'git remote remove
    --force-deletion <name>' would also do the same thing.

In any case, the current behavior where we just leave this crap behind
without any explanation or an obvious way to delete them (can't use
git-branch -d, need update-ref -d) isn't nice.

I encountered this in the wild because GitLab will add an "upstream" ref
if you clone a repo, but if you stop using the remote to update it in
combination with their "geo" remote (which has --mirror refspecs) it'll
leave behind all these dead refs.

Re: RFC: How should we handle un-deleted remote branches?

[Andreas Heiduk]

Am 20.04.2018 um 14:14 schrieb Ævar Arnfjörð Bjarmason:
> But this is a possible work-around:
> 
>     git init /tmp/empty.git
>     git remote add avar file:///tmp/empty.git
>     git remote prune avar
>     git remote remove avar

This won't do it also?

	git remote prune origin


> I started to patch this, but I'm not sure what to do here. we could do
> some combination of:
> 
>  0. Just document the current behavior and leave it.
> 
>  1. Dig further down to see what other remotes reference these refs, and
>     just ignore any refspecs that don't explicitly reference
>     refs/remotes/<our_deleted_remote>/*.
> 
>     I.e. isn't the intention here to preserve a case where you have two
>     URLs for the same effective remote, not whene you have something
>     like a --mirror refspec? Unfortunately I can't ask the original
>     author :(
> 
>  2. Warn about each ref we didn't delete, or at least warn saying
>     there's undeleted refs under refs/remotes/<name>/*.
> 
>  3. Make 'git remote remove --force-deletion <name>' (or whatever the
>     flag is called) be a thing. But unless we do the next item this
>     won't be useful.
> 
>  4. Make 'git remote prune <name>' work in cases where we don't have a
>     remote called <name> anymore, just falling back to deleting
>     refs/remotes/<name>. In this case 'git remote remove
>     --force-deletion <name>' would also do the same thing.

Possible 5):

	Don't fix "git remote remove" but "git remote add" to complain that its
ref-namespace is already occupied by some other remote. Add "--force"
for the experts.

Re: RFC: How should we handle un-deleted remote branches?

[Ævar Arnfjörð Bjarmason]

On Sun, Apr 22 2018, Andreas Heiduk wrote:

> Am 20.04.2018 um 14:14 schrieb Ævar Arnfjörð Bjarmason:
>> But this is a possible work-around:
>>
>>     git init /tmp/empty.git
>>     git remote add avar file:///tmp/empty.git
>>     git remote prune avar
>>     git remote remove avar
>
> This won't do it also?
>
> 	git remote prune origin

Yes, in this particular case, but that's just emergent behavior in how
we handle refspec prunign, and the fact that it "works" is arguably a
bug in "prune". i.e. this:

    (
        rm -rf /tmp/git &&
        git clone --bare --mirror git@github.com:git/git.git /tmp/git &&
        cd /tmp/git &&
        git remote add avar git@github.com:avar/git.git &&
        git remote add peff git@github.com:peff/git.git &&
        git fetch --all &&
        git remote remove avar &&
        git remote prune origin
    )

Will delete all the avar/* and peff/* branches, even though I still have
a "peff" remote.

IOW the guarding logic we have in add_branch_for_removal() for not
deleting the branches of other remotes isn't in the corresponding
"prune" function, and that's a bug.

In the specific example I picked "git remote prune origin" just so
happens to do the right thing since I have no other active remote, and
there *is* an alive remote so I can "prune" against it, but it doesn't
help in the general case. In my case I have a remote URL for a git
server called "upstream" that doesn't exist anymore (but as noted, I can
fake it with an empty repo...)>


>> I started to patch this, but I'm not sure what to do here. we could do
>> some combination of:
>>
>>  0. Just document the current behavior and leave it.
>>
>>  1. Dig further down to see what other remotes reference these refs, and
>>     just ignore any refspecs that don't explicitly reference
>>     refs/remotes/<our_deleted_remote>/*.
>>
>>     I.e. isn't the intention here to preserve a case where you have two
>>     URLs for the same effective remote, not whene you have something
>>     like a --mirror refspec? Unfortunately I can't ask the original
>>     author :(
>>
>>  2. Warn about each ref we didn't delete, or at least warn saying
>>     there's undeleted refs under refs/remotes/<name>/*.
>>
>>  3. Make 'git remote remove --force-deletion <name>' (or whatever the
>>     flag is called) be a thing. But unless we do the next item this
>>     won't be useful.
>>
>>  4. Make 'git remote prune <name>' work in cases where we don't have a
>>     remote called <name> anymore, just falling back to deleting
>>     refs/remotes/<name>. In this case 'git remote remove
>>     --force-deletion <name>' would also do the same thing.
>
> Possible 5):
>
> 	Don't fix "git remote remove" but "git remote add" to complain that its
> ref-namespace is already occupied by some other remote. Add "--force"
> for the experts.

Indeed, that's another bug here, i.e. in the above example:

    git remote remove peff && # won't delete peff/ branches
    git remote add peff git@github.com:peff/git.git

Will happily add the "peff" remote again, even though as you point out
it could be an entirely different remote.

Re: RFC: How should we handle un-deleted remote branches?

[Andreas Heiduk]

Am 22.04.2018 um 13:17 schrieb Ævar Arnfjörð Bjarmason:
> 
> On Sun, Apr 22 2018, Andreas Heiduk wrote:
> 
>> Am 20.04.2018 um 14:14 schrieb Ævar Arnfjörð Bjarmason:
>>> But this is a possible work-around:
>>>
>>>     git init /tmp/empty.git
>>>     git remote add avar file:///tmp/empty.git
>>>     git remote prune avar
>>>     git remote remove avar
>>
>> This won't do it also?
>>
>> 	git remote prune origin
> 
> Yes, in this particular case, but that's just emergent behavior in how
> we handle refspec prunign, and the fact that it "works" is arguably a
> bug in "prune". i.e. this:

Its not emergent because "origin" is the other remote responsible for
that ref and cleaning stuff "belonging" to the remote is the job
description (I'm arguing from a user's perspective, not as a git-developer).

> 
>     (
>         rm -rf /tmp/git &&
>         git clone --bare --mirror git@github.com:git/git.git /tmp/git &&
>         cd /tmp/git &&
>         git remote add avar git@github.com:avar/git.git &&
>         git remote add peff git@github.com:peff/git.git &&
>         git fetch --all &&
>         git remote remove avar &&
>         git remote prune origin
>     )
> 
> Will delete all the avar/* and peff/* branches, even though I still have
> a "peff" remote.

Exactly my point: When you are in a the bad situation of "shared
responsibility", then there is no easy and always correct way out,
because there are uncountable possible situations.

To give another, slightly modified example expanding the problem space:

(
    rm -rf /tmp/git &&
    git clone --bare --mirror https://github.com/git/git.git /tmp/git &&
    cd /tmp/git &&
    git remote add github https://github.com/avar/git.git &&
    git fetch github &&
    git fetch origin &&
    # note new fetches for "github/master" using with "(forced update)"
)

For ... reasons the first repo publishes some references like

	github/maint
	github/master
	github/pu

So when this repo is mirrored AND another, suitably named remote is
added then there will be also namespace conflicts. You can call

    git fetch github
    git fetch origin

in a loop and most likely each fetch will update the same refs, always
toggling between two states.

So: not only "remote remove" and "remote prune" are at stake but every
command handling remote references.

How should "git remote remove github" work in both situations? Remove
the refs/remotes/github/master & co? remove them only if the last fetch
was for "github" but not when the last update was for "origin"? Should
"git fetch" use the same logic?

So it seems better to me to avoid that bad situation altogether. Don't
allow overlapping/conflicting refspecs when adding a new remote. Using
*your* last examples both

>         git remote add avar git@github.com:avar/git.git &&
>         git remote add peff git@github.com:peff/git.git &&

should have failed and hence the "prune" problems won't exist. Same for
my example.

>> Possible 5):
>>
>> 	Don't fix "git remote remove" but "git remote add" to complain that its
>> ref-namespace is already occupied by some other remote. Add "--force"
>> for the experts.
> 
> Indeed, that's another bug here, i.e. in the above example:
> 
>     git remote remove peff && # won't delete peff/ branches
>     git remote add peff git@github.com:peff/git.git
> 
> Will happily add the "peff" remote again, even though as you point out
> it could be an entirely different remote.

Ummm. That was not my point. My is: "git clone --mirror" uses a refspec

	fetch = +refs/*:refs/*

and hence "occupies" the complete "refs/*" namespace. So adding another
remote (for the first time or for the second time is as irrelevant as
the url) will use

	fetch = +refs/heads/*:refs/remotes/peff/*

and now the "refs/remotes/peff/*" part is in conflict with "refs/*" from
above. The conflict exists not only for "prune" or "remove" but also for
"fetch", "rename" (didn't check) .

This kind of conflict should not be allowed right from the start - when
the first "git remote add peff..." is executed. Then prune, remove AND
fetch would be OK.

Re: RFC: How should we handle un-deleted remote branches?

[Ævar Arnfjörð Bjarmason]

On Sun, Apr 22 2018, Andreas Heiduk wrote:

> Am 22.04.2018 um 13:17 schrieb Ævar Arnfjörð Bjarmason:
>>
>> On Sun, Apr 22 2018, Andreas Heiduk wrote:
>>
>>> Am 20.04.2018 um 14:14 schrieb Ævar Arnfjörð Bjarmason:
>>>> But this is a possible work-around:
>>>>
>>>>     git init /tmp/empty.git
>>>>     git remote add avar file:///tmp/empty.git
>>>>     git remote prune avar
>>>>     git remote remove avar
>>>
>>> This won't do it also?
>>>
>>> 	git remote prune origin
>>
>> Yes, in this particular case, but that's just emergent behavior in how
>> we handle refspec prunign, and the fact that it "works" is arguably a
>> bug in "prune". i.e. this:
>
> Its not emergent because "origin" is the other remote responsible for
> that ref and cleaning stuff "belonging" to the remote is the job
> description (I'm arguing from a user's perspective, not as a git-developer).

Right, I should have said something closer to "inconsistent", at least
from the user's perspective. I.e. "remove" doesn't delete your branches
because another ref "owns" them, but "prune" will happily clobber
another remote's refs without warning.

Maybe we're happy to keep that, pruning is a bit of an oddity already,
see the "Git has a default disposition of keeping[...]" docs and the
rest of the "PRUNING" section I added to git-fetch in 2.17.0, but this
is osmething worth keeping in mind.

>>
>>     (
>>         rm -rf /tmp/git &&
>>         git clone --bare --mirror git@github.com:git/git.git /tmp/git &&
>>         cd /tmp/git &&
>>         git remote add avar git@github.com:avar/git.git &&
>>         git remote add peff git@github.com:peff/git.git &&
>>         git fetch --all &&
>>         git remote remove avar &&
>>         git remote prune origin
>>     )
>>
>> Will delete all the avar/* and peff/* branches, even though I still have
>> a "peff" remote.
>
> Exactly my point: When you are in a the bad situation of "shared
> responsibility", then there is no easy and always correct way out,
> because there are uncountable possible situations.
>
> To give another, slightly modified example expanding the problem space:
>
> (
>     rm -rf /tmp/git &&
>     git clone --bare --mirror https://github.com/git/git.git /tmp/git &&
>     cd /tmp/git &&
>     git remote add github https://github.com/avar/git.git &&
>     git fetch github &&
>     git fetch origin &&
>     # note new fetches for "github/master" using with "(forced update)"
> )
>
> For ... reasons the first repo publishes some references like
>
> 	github/maint
> 	github/master
> 	github/pu
>
> So when this repo is mirrored AND another, suitably named remote is
> added then there will be also namespace conflicts. You can call
>
>     git fetch github
>     git fetch origin
>
> in a loop and most likely each fetch will update the same refs, always
> toggling between two states.
>
> So: not only "remote remove" and "remote prune" are at stake but every
> command handling remote references.

Right, there's certainly a lot of insanity you can create with
overlapping refs, but to bring this thread around a bit, the edge cases
I'm interested in addressing are those where "git remote <whatever>"
silently doesn't do its job, or does it too eagerly, resulting in a
hard-to-repair repo state.

> How should "git remote remove github" work in both situations? Remove
> the refs/remotes/github/master & co? remove them only if the last fetch
> was for "github" but not when the last update was for "origin"? Should
> "git fetch" use the same logic?

Until we move to some other ref store implementation that namespaces
things properly, the best we can do is simply to assume that
refs/remotes/<name>/ is owned by the <name> remote for the purposes of
remove/prune etc.

That'll of course leave open this edge case you're pointing out where
you're mirroring another remote into refs/* and it creates a
remote/<name>/ branch, but I think it's sufficient for the purposes of
sane UI to just document that fetching into refs/* creates such caveats.

> So it seems better to me to avoid that bad situation altogether. Don't
> allow overlapping/conflicting refspecs when adding a new remote. Using
> *your* last examples both
>
>>         git remote add avar git@github.com:avar/git.git &&
>>         git remote add peff git@github.com:peff/git.git &&
>
> should have failed and hence the "prune" problems won't exist. Same for
> my example.

I think this is a non-started. There's plenty of legitimate reasons to
have overlapping refspecs, e.g. the GitLab case where they're creating a
"geo" remote which is a mirror of other refs they push to.

Even if that wasn't the case, it would be very fragile to solve these
cases by disallowing adding such remotes, since users can edit the
config file, we'd need to detect it on the fly.

>>> Possible 5):
>>>
>>> 	Don't fix "git remote remove" but "git remote add" to complain that its
>>> ref-namespace is already occupied by some other remote. Add "--force"
>>> for the experts.
>>
>> Indeed, that's another bug here, i.e. in the above example:
>>
>>     git remote remove peff && # won't delete peff/ branches
>>     git remote add peff git@github.com:peff/git.git
>>
>> Will happily add the "peff" remote again, even though as you point out
>> it could be an entirely different remote.
>
> Ummm. That was not my point. My is: "git clone --mirror" uses a refspec
>
> 	fetch = +refs/*:refs/*
>
> and hence "occupies" the complete "refs/*" namespace. So adding another
> remote (for the first time or for the second time is as irrelevant as
> the url) will use
>
> 	fetch = +refs/heads/*:refs/remotes/peff/*
>
> and now the "refs/remotes/peff/*" part is in conflict with "refs/*" from
> above. The conflict exists not only for "prune" or "remove" but also for
> "fetch", "rename" (didn't check) .
>
> This kind of conflict should not be allowed right from the start - when
> the first "git remote add peff..." is executed. Then prune, remove AND
> fetch would be OK.

As noted above we need to deal with this overlap, and it has to be
allowed, but we can still do better than we do now with "remove" /
"prune" et al, i.e. simply inform & ask the user what he'd like to do.