{"thread":{"id":"48333","subject":"RFC: How should we handle un-deleted remote branches?","startedAt":"2018-04-20T12:14:30Z","lastAt":"2018-04-22T18:37:42Z","messageCount":5,"participants":["Ævar Arnfjörð Bjarmason","Andreas Heiduk"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"345214","messageId":"87in8mdqhr.fsf@evledraar.gmail.com","threadId":"48333","inReplyTo":null,"subject":"RFC: How should we handle un-deleted remote branches?","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-04-20T12:14:24Z","receivedAt":"2018-04-20T12:14:30Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"I removed a remote and its refs persisted. At first I thought this was a\nbug but looking at add_branch_for_removal()'s \"don't delete a branch if\nanother remote also uses it\" logic it's intentional.\n\nThis goes very wrong if you do e.g.:\n\n    (\n        rm -rf /tmp/git &&\n        git clone --bare --mirror git@github.com:git/git.git /tmp/git &&\n        cd /tmp/git &&\n        git remote add avar  git@github.com:avar/git.git &&\n        git fetch avar &&\n        git remote remove avar &&\n        git for-each-ref|grep -c refs/remotes/avar\n    )\n\nAt this point you can't prune it since no remote config is left:\n\n    $ git remote prune avar\n    fatal: 'avar' does not appear to be a git repository\n    fatal: Could not read from remote repository.\n\nBut this is a possible work-around:\n\n    git init /tmp/empty.git\n    git remote add avar file:///tmp/empty.git\n    git remote prune avar\n    git remote remove avar\n\nOr use update-ref -d to remove them:\n\n    for rr in $(git for-each-ref --format=\"%(refname)\"|grep ^refs/remotes/avar/)\n    do\n        git update-ref -d $rr\n    done\n\nI started to patch this, but I'm not sure what to do here. we could do\nsome combination of:\n\n 0. Just document the current behavior and leave it.\n\n 1. Dig further down to see what other remotes reference these refs, and\n    just ignore any refspecs that don't explicitly reference\n    refs/remotes/<our_deleted_remote>/*.\n\n    I.e. isn't the intention here to preserve a case where you have two\n    URLs for the same effective remote, not whene you have something\n    like a --mirror refspec? Unfortunately I can't ask the original\n    author :(\n\n 2. Warn about each ref we didn't delete, or at least warn saying\n    there's undeleted refs under refs/remotes/<name>/*.\n\n 3. Make 'git remote remove --force-deletion <name>' (or whatever the\n    flag is called) be a thing. But unless we do the next item this\n    won't be useful.\n\n 4. Make 'git remote prune <name>' work in cases where we don't have a\n    remote called <name> anymore, just falling back to deleting\n    refs/remotes/<name>. In this case 'git remote remove\n    --force-deletion <name>' would also do the same thing.\n\nIn any case, the current behavior where we just leave this crap behind\nwithout any explanation or an obvious way to delete them (can't use\ngit-branch -d, need update-ref -d) isn't nice.\n\nI encountered this in the wild because GitLab will add an \"upstream\" ref\nif you clone a repo, but if you stop using the remote to update it in\ncombination with their \"geo\" remote (which has --mirror refspecs) it'll\nleave behind all these dead refs.\n"},{"id":"345384","messageId":"3fcd1b50-2aeb-0ea4-fea7-b5705e76c027@gmail.com","threadId":"48333","inReplyTo":"87in8mdqhr.fsf@evledraar.gmail.com","subject":"Re: RFC: How should we handle un-deleted remote branches?","fromName":"Andreas Heiduk","fromEmail":"asheiduk@gmail.com","sentAt":"2018-04-22T07:41:42Z","receivedAt":"2018-04-22T07:41:50Z","isPatch":false,"sender":{"key":"asheiduk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9371344?v=4"},"body":"Am 20.04.2018 um 14:14 schrieb Ævar Arnfjörð Bjarmason:\n> But this is a possible work-around:\n> \n>     git init /tmp/empty.git\n>     git remote add avar file:///tmp/empty.git\n>     git remote prune avar\n>     git remote remove avar\n\nThis won't do it also?\n\n\tgit remote prune origin\n\n\n> I started to patch this, but I'm not sure what to do here. we could do\n> some combination of:\n> \n>  0. Just document the current behavior and leave it.\n> \n>  1. Dig further down to see what other remotes reference these refs, and\n>     just ignore any refspecs that don't explicitly reference\n>     refs/remotes/<our_deleted_remote>/*.\n> \n>     I.e. isn't the intention here to preserve a case where you have two\n>     URLs for the same effective remote, not whene you have something\n>     like a --mirror refspec? Unfortunately I can't ask the original\n>     author :(\n> \n>  2. Warn about each ref we didn't delete, or at least warn saying\n>     there's undeleted refs under refs/remotes/<name>/*.\n> \n>  3. Make 'git remote remove --force-deletion <name>' (or whatever the\n>     flag is called) be a thing. But unless we do the next item this\n>     won't be useful.\n> \n>  4. Make 'git remote prune <name>' work in cases where we don't have a\n>     remote called <name> anymore, just falling back to deleting\n>     refs/remotes/<name>. In this case 'git remote remove\n>     --force-deletion <name>' would also do the same thing.\n\nPossible 5):\n\n\tDon't fix \"git remote remove\" but \"git remote add\" to complain that its\nref-namespace is already occupied by some other remote. Add \"--force\"\nfor the experts.\n\n"},{"id":"345385","messageId":"87d0yrebhw.fsf@evledraar.gmail.com","threadId":"48333","inReplyTo":"3fcd1b50-2aeb-0ea4-fea7-b5705e76c027@gmail.com","subject":"Re: RFC: How should we handle un-deleted remote branches?","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-04-22T11:17:31Z","receivedAt":"2018-04-22T11:17:37Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Apr 22 2018, Andreas Heiduk wrote:\n\n> Am 20.04.2018 um 14:14 schrieb Ævar Arnfjörð Bjarmason:\n>> But this is a possible work-around:\n>>\n>>     git init /tmp/empty.git\n>>     git remote add avar file:///tmp/empty.git\n>>     git remote prune avar\n>>     git remote remove avar\n>\n> This won't do it also?\n>\n> \tgit remote prune origin\n\nYes, in this particular case, but that's just emergent behavior in how\nwe handle refspec prunign, and the fact that it \"works\" is arguably a\nbug in \"prune\". i.e. this:\n\n    (\n        rm -rf /tmp/git &&\n        git clone --bare --mirror git@github.com:git/git.git /tmp/git &&\n        cd /tmp/git &&\n        git remote add avar git@github.com:avar/git.git &&\n        git remote add peff git@github.com:peff/git.git &&\n        git fetch --all &&\n        git remote remove avar &&\n        git remote prune origin\n    )\n\nWill delete all the avar/* and peff/* branches, even though I still have\na \"peff\" remote.\n\nIOW the guarding logic we have in add_branch_for_removal() for not\ndeleting the branches of other remotes isn't in the corresponding\n\"prune\" function, and that's a bug.\n\nIn the specific example I picked \"git remote prune origin\" just so\nhappens to do the right thing since I have no other active remote, and\nthere *is* an alive remote so I can \"prune\" against it, but it doesn't\nhelp in the general case. In my case I have a remote URL for a git\nserver called \"upstream\" that doesn't exist anymore (but as noted, I can\nfake it with an empty repo...)>\n\n\n>> I started to patch this, but I'm not sure what to do here. we could do\n>> some combination of:\n>>\n>>  0. Just document the current behavior and leave it.\n>>\n>>  1. Dig further down to see what other remotes reference these refs, and\n>>     just ignore any refspecs that don't explicitly reference\n>>     refs/remotes/<our_deleted_remote>/*.\n>>\n>>     I.e. isn't the intention here to preserve a case where you have two\n>>     URLs for the same effective remote, not whene you have something\n>>     like a --mirror refspec? Unfortunately I can't ask the original\n>>     author :(\n>>\n>>  2. Warn about each ref we didn't delete, or at least warn saying\n>>     there's undeleted refs under refs/remotes/<name>/*.\n>>\n>>  3. Make 'git remote remove --force-deletion <name>' (or whatever the\n>>     flag is called) be a thing. But unless we do the next item this\n>>     won't be useful.\n>>\n>>  4. Make 'git remote prune <name>' work in cases where we don't have a\n>>     remote called <name> anymore, just falling back to deleting\n>>     refs/remotes/<name>. In this case 'git remote remove\n>>     --force-deletion <name>' would also do the same thing.\n>\n> Possible 5):\n>\n> \tDon't fix \"git remote remove\" but \"git remote add\" to complain that its\n> ref-namespace is already occupied by some other remote. Add \"--force\"\n> for the experts.\n\nIndeed, that's another bug here, i.e. in the above example:\n\n    git remote remove peff && # won't delete peff/ branches\n    git remote add peff git@github.com:peff/git.git\n\nWill happily add the \"peff\" remote again, even though as you point out\nit could be an entirely different remote.\n"},{"id":"345390","messageId":"f2fc89c6-14a9-b700-b7c9-a646148e9206@gmail.com","threadId":"48333","inReplyTo":"87d0yrebhw.fsf@evledraar.gmail.com","subject":"Re: RFC: How should we handle un-deleted remote branches?","fromName":"Andreas Heiduk","fromEmail":"asheiduk@gmail.com","sentAt":"2018-04-22T14:30:28Z","receivedAt":"2018-04-22T14:30:40Z","isPatch":false,"sender":{"key":"asheiduk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9371344?v=4"},"body":"Am 22.04.2018 um 13:17 schrieb Ævar Arnfjörð Bjarmason:\n> \n> On Sun, Apr 22 2018, Andreas Heiduk wrote:\n> \n>> Am 20.04.2018 um 14:14 schrieb Ævar Arnfjörð Bjarmason:\n>>> But this is a possible work-around:\n>>>\n>>>     git init /tmp/empty.git\n>>>     git remote add avar file:///tmp/empty.git\n>>>     git remote prune avar\n>>>     git remote remove avar\n>>\n>> This won't do it also?\n>>\n>> \tgit remote prune origin\n> \n> Yes, in this particular case, but that's just emergent behavior in how\n> we handle refspec prunign, and the fact that it \"works\" is arguably a\n> bug in \"prune\". i.e. this:\n\nIts not emergent because \"origin\" is the other remote responsible for\nthat ref and cleaning stuff \"belonging\" to the remote is the job\ndescription (I'm arguing from a user's perspective, not as a git-developer).\n\n> \n>     (\n>         rm -rf /tmp/git &&\n>         git clone --bare --mirror git@github.com:git/git.git /tmp/git &&\n>         cd /tmp/git &&\n>         git remote add avar git@github.com:avar/git.git &&\n>         git remote add peff git@github.com:peff/git.git &&\n>         git fetch --all &&\n>         git remote remove avar &&\n>         git remote prune origin\n>     )\n> \n> Will delete all the avar/* and peff/* branches, even though I still have\n> a \"peff\" remote.\n\nExactly my point: When you are in a the bad situation of \"shared\nresponsibility\", then there is no easy and always correct way out,\nbecause there are uncountable possible situations.\n\nTo give another, slightly modified example expanding the problem space:\n\n(\n    rm -rf /tmp/git &&\n    git clone --bare --mirror https://github.com/git/git.git /tmp/git &&\n    cd /tmp/git &&\n    git remote add github https://github.com/avar/git.git &&\n    git fetch github &&\n    git fetch origin &&\n    # note new fetches for \"github/master\" using with \"(forced update)\"\n)\n\nFor ... reasons the first repo publishes some references like\n\n\tgithub/maint\n\tgithub/master\n\tgithub/pu\n\nSo when this repo is mirrored AND another, suitably named remote is\nadded then there will be also namespace conflicts. You can call\n\n    git fetch github\n    git fetch origin\n\nin a loop and most likely each fetch will update the same refs, always\ntoggling between two states.\n\nSo: not only \"remote remove\" and \"remote prune\" are at stake but every\ncommand handling remote references.\n\nHow should \"git remote remove github\" work in both situations? Remove\nthe refs/remotes/github/master & co? remove them only if the last fetch\nwas for \"github\" but not when the last update was for \"origin\"? Should\n\"git fetch\" use the same logic?\n\nSo it seems better to me to avoid that bad situation altogether. Don't\nallow overlapping/conflicting refspecs when adding a new remote. Using\n*your* last examples both\n\n>         git remote add avar git@github.com:avar/git.git &&\n>         git remote add peff git@github.com:peff/git.git &&\n\nshould have failed and hence the \"prune\" problems won't exist. Same for\nmy example.\n\n>> Possible 5):\n>>\n>> \tDon't fix \"git remote remove\" but \"git remote add\" to complain that its\n>> ref-namespace is already occupied by some other remote. Add \"--force\"\n>> for the experts.\n> \n> Indeed, that's another bug here, i.e. in the above example:\n> \n>     git remote remove peff && # won't delete peff/ branches\n>     git remote add peff git@github.com:peff/git.git\n> \n> Will happily add the \"peff\" remote again, even though as you point out\n> it could be an entirely different remote.\n\nUmmm. That was not my point. My is: \"git clone --mirror\" uses a refspec\n\n\tfetch = +refs/*:refs/*\n\nand hence \"occupies\" the complete \"refs/*\" namespace. So adding another\nremote (for the first time or for the second time is as irrelevant as\nthe url) will use\n\n\tfetch = +refs/heads/*:refs/remotes/peff/*\n\nand now the \"refs/remotes/peff/*\" part is in conflict with \"refs/*\" from\nabove. The conflict exists not only for \"prune\" or \"remove\" but also for\n\"fetch\", \"rename\" (didn't check) .\n\nThis kind of conflict should not be allowed right from the start - when\nthe first \"git remote add peff...\" is executed. Then prune, remove AND\nfetch would be OK.\n\n\n"},{"id":"345407","messageId":"87bmebdr4f.fsf@evledraar.gmail.com","threadId":"48333","inReplyTo":"f2fc89c6-14a9-b700-b7c9-a646148e9206@gmail.com","subject":"Re: RFC: How should we handle un-deleted remote branches?","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-04-22T18:37:36Z","receivedAt":"2018-04-22T18:37:42Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Apr 22 2018, Andreas Heiduk wrote:\n\n> Am 22.04.2018 um 13:17 schrieb Ævar Arnfjörð Bjarmason:\n>>\n>> On Sun, Apr 22 2018, Andreas Heiduk wrote:\n>>\n>>> Am 20.04.2018 um 14:14 schrieb Ævar Arnfjörð Bjarmason:\n>>>> But this is a possible work-around:\n>>>>\n>>>>     git init /tmp/empty.git\n>>>>     git remote add avar file:///tmp/empty.git\n>>>>     git remote prune avar\n>>>>     git remote remove avar\n>>>\n>>> This won't do it also?\n>>>\n>>> \tgit remote prune origin\n>>\n>> Yes, in this particular case, but that's just emergent behavior in how\n>> we handle refspec prunign, and the fact that it \"works\" is arguably a\n>> bug in \"prune\". i.e. this:\n>\n> Its not emergent because \"origin\" is the other remote responsible for\n> that ref and cleaning stuff \"belonging\" to the remote is the job\n> description (I'm arguing from a user's perspective, not as a git-developer).\n\nRight, I should have said something closer to \"inconsistent\", at least\nfrom the user's perspective. I.e. \"remove\" doesn't delete your branches\nbecause another ref \"owns\" them, but \"prune\" will happily clobber\nanother remote's refs without warning.\n\nMaybe we're happy to keep that, pruning is a bit of an oddity already,\nsee the \"Git has a default disposition of keeping[...]\" docs and the\nrest of the \"PRUNING\" section I added to git-fetch in 2.17.0, but this\nis osmething worth keeping in mind.\n\n>>\n>>     (\n>>         rm -rf /tmp/git &&\n>>         git clone --bare --mirror git@github.com:git/git.git /tmp/git &&\n>>         cd /tmp/git &&\n>>         git remote add avar git@github.com:avar/git.git &&\n>>         git remote add peff git@github.com:peff/git.git &&\n>>         git fetch --all &&\n>>         git remote remove avar &&\n>>         git remote prune origin\n>>     )\n>>\n>> Will delete all the avar/* and peff/* branches, even though I still have\n>> a \"peff\" remote.\n>\n> Exactly my point: When you are in a the bad situation of \"shared\n> responsibility\", then there is no easy and always correct way out,\n> because there are uncountable possible situations.\n>\n> To give another, slightly modified example expanding the problem space:\n>\n> (\n>     rm -rf /tmp/git &&\n>     git clone --bare --mirror https://github.com/git/git.git /tmp/git &&\n>     cd /tmp/git &&\n>     git remote add github https://github.com/avar/git.git &&\n>     git fetch github &&\n>     git fetch origin &&\n>     # note new fetches for \"github/master\" using with \"(forced update)\"\n> )\n>\n> For ... reasons the first repo publishes some references like\n>\n> \tgithub/maint\n> \tgithub/master\n> \tgithub/pu\n>\n> So when this repo is mirrored AND another, suitably named remote is\n> added then there will be also namespace conflicts. You can call\n>\n>     git fetch github\n>     git fetch origin\n>\n> in a loop and most likely each fetch will update the same refs, always\n> toggling between two states.\n>\n> So: not only \"remote remove\" and \"remote prune\" are at stake but every\n> command handling remote references.\n\nRight, there's certainly a lot of insanity you can create with\noverlapping refs, but to bring this thread around a bit, the edge cases\nI'm interested in addressing are those where \"git remote <whatever>\"\nsilently doesn't do its job, or does it too eagerly, resulting in a\nhard-to-repair repo state.\n\n> How should \"git remote remove github\" work in both situations? Remove\n> the refs/remotes/github/master & co? remove them only if the last fetch\n> was for \"github\" but not when the last update was for \"origin\"? Should\n> \"git fetch\" use the same logic?\n\nUntil we move to some other ref store implementation that namespaces\nthings properly, the best we can do is simply to assume that\nrefs/remotes/<name>/ is owned by the <name> remote for the purposes of\nremove/prune etc.\n\nThat'll of course leave open this edge case you're pointing out where\nyou're mirroring another remote into refs/* and it creates a\nremote/<name>/ branch, but I think it's sufficient for the purposes of\nsane UI to just document that fetching into refs/* creates such caveats.\n\n> So it seems better to me to avoid that bad situation altogether. Don't\n> allow overlapping/conflicting refspecs when adding a new remote. Using\n> *your* last examples both\n>\n>>         git remote add avar git@github.com:avar/git.git &&\n>>         git remote add peff git@github.com:peff/git.git &&\n>\n> should have failed and hence the \"prune\" problems won't exist. Same for\n> my example.\n\nI think this is a non-started. There's plenty of legitimate reasons to\nhave overlapping refspecs, e.g. the GitLab case where they're creating a\n\"geo\" remote which is a mirror of other refs they push to.\n\nEven if that wasn't the case, it would be very fragile to solve these\ncases by disallowing adding such remotes, since users can edit the\nconfig file, we'd need to detect it on the fly.\n\n>>> Possible 5):\n>>>\n>>> \tDon't fix \"git remote remove\" but \"git remote add\" to complain that its\n>>> ref-namespace is already occupied by some other remote. Add \"--force\"\n>>> for the experts.\n>>\n>> Indeed, that's another bug here, i.e. in the above example:\n>>\n>>     git remote remove peff && # won't delete peff/ branches\n>>     git remote add peff git@github.com:peff/git.git\n>>\n>> Will happily add the \"peff\" remote again, even though as you point out\n>> it could be an entirely different remote.\n>\n> Ummm. That was not my point. My is: \"git clone --mirror\" uses a refspec\n>\n> \tfetch = +refs/*:refs/*\n>\n> and hence \"occupies\" the complete \"refs/*\" namespace. So adding another\n> remote (for the first time or for the second time is as irrelevant as\n> the url) will use\n>\n> \tfetch = +refs/heads/*:refs/remotes/peff/*\n>\n> and now the \"refs/remotes/peff/*\" part is in conflict with \"refs/*\" from\n> above. The conflict exists not only for \"prune\" or \"remove\" but also for\n> \"fetch\", \"rename\" (didn't check) .\n>\n> This kind of conflict should not be allowed right from the start - when\n> the first \"git remote add peff...\" is executed. Then prune, remove AND\n> fetch would be OK.\n\nAs noted above we need to deal with this overlap, and it has to be\nallowed, but we can still do better than we do now with \"remove\" /\n\"prune\" et al, i.e. simply inform & ask the user what he'd like to do.\n"}]}