{"thread":{"id":"28502","subject":"[BUG?] git fetch -p -t prunes all non-tag refs","startedAt":"2011-09-26T18:47:39Z","lastAt":"2011-10-07T23:00:07Z","messageCount":14,"participants":["Ben Boeckel","Junio C Hamano","Carlos Martín Nieto","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"176243","messageId":"20110926184739.GA11745@erythro.kitwarein.com","threadId":"28502","inReplyTo":null,"subject":"[BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2011-09-26T18:47:39Z","receivedAt":"2011-09-26T18:47:39Z","isPatch":false,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"Hi,\n\nWhen the --prune and --tags options are given to git fetch together, all\nnon-tag refs are pruned because only tags are looked at and when pruning\nit appears as if the branches have disappeared and are therefore deleted\nlocally. Maybe this is an unintentional wanted behavior, but it should\nat least be documented that the combination causes this to happen.\n\n    git-1.7.6.2-1.fc15.x86_64\n\nAppears to still be there in master (85e9c7e) from looking at the logs.\n\n--Ben\n"},{"id":"176272","messageId":"7vehz30wdy.fsf@alter.siamese.dyndns.org","threadId":"28502","inReplyTo":"20110926184739.GA11745@erythro.kitwarein.com","subject":"Re: [BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-26T22:30:33Z","receivedAt":"2011-09-26T22:30:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Boeckel <mathstuf@gmail.com> writes:\n\n> When the --prune and --tags options are given to git fetch together, all\n> non-tag refs are pruned because only tags are looked at and when pruning\n> it appears as if the branches have disappeared and are therefore deleted\n> locally.\n\nI would call that a bug, and it is not limited to the use of \"--tags\". For\nexample, I suspect that\n\n    $ git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n\nwould prune remote tracking branches for \"origin\" other than \"master\".\n"},{"id":"176274","messageId":"20110926225131.GA3421@erythro.kitwarein.com","threadId":"28502","inReplyTo":"7vehz30wdy.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2011-09-26T22:51:31Z","receivedAt":"2011-09-26T22:51:31Z","isPatch":false,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"On Mon, Sep 26, 2011 at 15:30:33 -0700, Junio C Hamano wrote:\n> I would call that a bug, and it is not limited to the use of \"--tags\". For\n> example, I suspect that\n> \n>     $ git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n> \n> would prune remote tracking branches for \"origin\" other than \"master\".\n\nIndeed it does. Given that, should --prune be an error when a refspec is\ngiven or when in combination with --tags?\n\n--Ben\n"},{"id":"176278","messageId":"1317078667.5579.13.camel@centaur.lab.cmartin.tk","threadId":"28502","inReplyTo":"7vehz30wdy.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-09-26T23:11:03Z","receivedAt":"2011-09-26T23:11:03Z","isPatch":false,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Mon, 2011-09-26 at 15:30 -0700, Junio C Hamano wrote:\n> Ben Boeckel <mathstuf@gmail.com> writes:\n> \n> > When the --prune and --tags options are given to git fetch together, all\n> > non-tag refs are pruned because only tags are looked at and when pruning\n> > it appears as if the branches have disappeared and are therefore deleted\n> > locally.\n> \n> I would call that a bug, and it is not limited to the use of \"--tags\". For\n> example, I suspect that\n> \n>     $ git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n> \n> would prune remote tracking branches for \"origin\" other than \"master\".\n\nThis should fix it (in a way). Let's agree that it's a bad idea and\ncomplain to the user. Looking at the surrounding code, I can't find\nanything obvious that would break, and `git fetch --prune --multiple a\nb` is allowed.\n\nPatch on top of maint.\n\n--- 8< ---\nSubject: [PATCH] fetch: disallow --prune in combination with tags or refspecs\n\nPruning shouldn't be used when other options limit the references that\nshould be taken into account. For example\n\n    git fetch --prune --tags origin\n    git fetch --prune origin refs/heads/*:refs/remotes/*\n\nBoth these commands would remove references which do still exist in\nthe remote.\n\nPrint an error and exit if prune is selected at the same time as\neither tags are selected or a refspec is given.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n builtin/fetch.c |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex e422ced..158b20a 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -967,6 +967,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\t\tif (!add_remote_or_group(argv[i], &list))\n \t\t\t\tdie(_(\"No such remote or remote group: %s\"), argv[i]);\n \t\tresult = fetch_multiple(&list);\n+\t} else if (prune && (argc == 2 || tags != TAGS_DEFAULT)) {\n+\t\t/* The if (multiple) is above so we don't report multiple remotes as an error */\n+\t\tdie(_(\"Pruning and limiting refs is not compatible\"));\n \t} else {\n \t\t/* Single remote or group */\n \t\t(void) add_remote_or_group(argv[0], &list);\n-- \n1.7.5.2.354.g349bf\n\n\n\n"},{"id":"176279","messageId":"7v1uv228t4.fsf@alter.siamese.dyndns.org","threadId":"28502","inReplyTo":"1317078667.5579.13.camel@centaur.lab.cmartin.tk","subject":"Re: [BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-26T23:16:55Z","receivedAt":"2011-09-26T23:16:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto <cmn@elego.de> writes:\n\n> On Mon, 2011-09-26 at 15:30 -0700, Junio C Hamano wrote:\n>> Ben Boeckel <mathstuf@gmail.com> writes:\n>> \n>> > When the --prune and --tags options are given to git fetch together, all\n>> > non-tag refs are pruned because only tags are looked at and when pruning\n>> > it appears as if the branches have disappeared and are therefore deleted\n>> > locally.\n>> \n>> I would call that a bug, and it is not limited to the use of \"--tags\". For\n>> example, I suspect that\n>> \n>>     $ git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n>> \n>> would prune remote tracking branches for \"origin\" other than \"master\".\n>\n> This should fix it (in a way). Let's agree that it's a bad idea and\n> complain to the user.\n\nThat might be a reasonable short-term safety measure, but in the longer\nterm I think we should fix it properly. We are already learning \"what are\nthe refs the remote side currently has\" from the transport and the right\nfix ought to be to use that original information, not the version filtered\nfor the use of the primary objective of fetch, which is to only fetch what\nthe user asked for.\n"},{"id":"176285","messageId":"1317079692.5579.19.camel@centaur.lab.cmartin.tk","threadId":"28502","inReplyTo":"7v1uv228t4.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-09-26T23:28:09Z","receivedAt":"2011-09-26T23:28:09Z","isPatch":false,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Mon, 2011-09-26 at 16:16 -0700, Junio C Hamano wrote:\n> Carlos Martín Nieto <cmn@elego.de> writes:\n> \n> > On Mon, 2011-09-26 at 15:30 -0700, Junio C Hamano wrote:\n> >> Ben Boeckel <mathstuf@gmail.com> writes:\n> >> \n> >> > When the --prune and --tags options are given to git fetch together, all\n> >> > non-tag refs are pruned because only tags are looked at and when pruning\n> >> > it appears as if the branches have disappeared and are therefore deleted\n> >> > locally.\n> >> \n> >> I would call that a bug, and it is not limited to the use of \"--tags\". For\n> >> example, I suspect that\n> >> \n> >>     $ git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n> >> \n> >> would prune remote tracking branches for \"origin\" other than \"master\".\n> >\n> > This should fix it (in a way). Let's agree that it's a bad idea and\n> > complain to the user.\n> \n> That might be a reasonable short-term safety measure, but in the longer\n\nSure.\n\n> term I think we should fix it properly. We are already learning \"what are\n> the refs the remote side currently has\" from the transport and the right\n> fix ought to be to use that original information, not the version filtered\n> for the use of the primary objective of fetch, which is to only fetch what\n> the user asked for.\n\nDo you mean that we should ignore the refspec? Or do you mean that we\nshould look at the refspec if it exists, and only consider deleting\nthose that meet the refspec, so that `--prune --tags` would only delete\ntags that don't exist in the remote?\n\nEither way, it's a bit more complicated than a two-liner and it's too\nlate in my timezone for that. I'll try to see if I can do it in the next\nfew days.\n\n   cmn\n"},{"id":"176303","messageId":"20110927033134.GB8534@sigill.intra.peff.net","threadId":"28502","inReplyTo":"1317079692.5579.19.camel@centaur.lab.cmartin.tk","subject":"Re: [BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-27T03:31:34Z","receivedAt":"2011-09-27T03:31:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 27, 2011 at 01:28:09AM +0200, Carlos Martín Nieto wrote:\n\n> > term I think we should fix it properly. We are already learning \"what are\n> > the refs the remote side currently has\" from the transport and the right\n> > fix ought to be to use that original information, not the version filtered\n> > for the use of the primary objective of fetch, which is to only fetch what\n> > the user asked for.\n> \n> Do you mean that we should ignore the refspec? Or do you mean that we\n> should look at the refspec if it exists, and only consider deleting\n> those that meet the refspec, so that `--prune --tags` would only delete\n> tags that don't exist in the remote?\n\nThe latter. If I say:\n\n  git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n\nand refs/heads/master doesn't exist on the remote, I would expect\nrefs/remotes/origin/master to be deleted locally. And that naturally\nextends to:\n\n  git fetch --prune origin refs/heads/*:refs/remotes/origin/*\n\nWe do something similar with \"git push --mirror\", which does pruning\nlike this[1].\n\n-Peff\n\n[1] Actually, I'm not sure how correct \"push --mirror\" is. It would be\n    nice if the prune operation could be split from the mirror, too. In\n    the past, I have wanted to do both:\n\n      # backup to a repository where our objects will be shared\n      # with other related backups. So we must only use our slice of the\n      # ref namespace.\n      git push --mirror backup-repo +refs/*:refs/`hostname`/*\n\n    and:\n\n      # update topic branches we have already published (using the\n      # \"matching\" refspec), but remove any that we have deleted\n      # locally.\n      git push --mirror publish-point +:\n\n    and I don't think either works.\n"},{"id":"176832","messageId":"1317724404.2577.6.camel@bee.lab.cmartin.tk","threadId":"28502","inReplyTo":"20110927033134.GB8534@sigill.intra.peff.net","subject":"Re: [BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-04T10:33:22Z","receivedAt":"2011-10-04T10:33:22Z","isPatch":false,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Mon, 2011-09-26 at 23:31 -0400, Jeff King wrote:\n> On Tue, Sep 27, 2011 at 01:28:09AM +0200, Carlos Martín Nieto wrote:\n> \n> > > term I think we should fix it properly. We are already learning \"what are\n> > > the refs the remote side currently has\" from the transport and the right\n> > > fix ought to be to use that original information, not the version filtered\n> > > for the use of the primary objective of fetch, which is to only fetch what\n> > > the user asked for.\n> > \n> > Do you mean that we should ignore the refspec? Or do you mean that we\n> > should look at the refspec if it exists, and only consider deleting\n> > those that meet the refspec, so that `--prune --tags` would only delete\n> > tags that don't exist in the remote?\n> \n> The latter. If I say:\n> \n>   git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n> \n> and refs/heads/master doesn't exist on the remote, I would expect\n> refs/remotes/origin/master to be deleted locally. And that naturally\n> extends to:\n> \n>   git fetch --prune origin refs/heads/*:refs/remotes/origin/*\n\nI have some code locally that solves this second part. If we are given\nrefspecs on the command-line, it will try to match against that instead\nof blindly trusting what get_stale_heads tells us. I'm looking into\nputting the logic into get_stale_heads so that we can trust it.\n\nThe first part might be more complicated. If the remote head doesn't\nexist, get_fetch_map dies. It does take a missing_ok flag, so it might\nbe as easy as passing 1 there; but I'm not sure what that would do for a\nnon-prune fetch.\n\n> \n> We do something similar with \"git push --mirror\", which does pruning\n> like this[1].\n> \n> -Peff\n> \n> [1] Actually, I'm not sure how correct \"push --mirror\" is. It would be\n>     nice if the prune operation could be split from the mirror, too. In\n>     the past, I have wanted to do both:\n> \n>       # backup to a repository where our objects will be shared\n>       # with other related backups. So we must only use our slice of the\n>       # ref namespace.\n>       git push --mirror backup-repo +refs/*:refs/`hostname`/*\n\nIs --mirror needed there? I would have thought that\nrefs/*:refs/`hostname`/* would do the same by itself.\n\n> \n>     and:\n> \n>       # update topic branches we have already published (using the\n>       # \"matching\" refspec), but remove any that we have deleted\n>       # locally.\n>       git push --mirror publish-point +:\n> \n>     and I don't think either works.\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n\n\n"},{"id":"176833","messageId":"20111004103624.GA11863@sigill.intra.peff.net","threadId":"28502","inReplyTo":"1317724404.2577.6.camel@bee.lab.cmartin.tk","subject":"Re: [BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-04T10:36:24Z","receivedAt":"2011-10-04T10:36:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2011 at 12:33:22PM +0200, Carlos Martín Nieto wrote:\n\n> > The latter. If I say:\n> > \n> >   git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n> > \n> > and refs/heads/master doesn't exist on the remote, I would expect\n> > refs/remotes/origin/master to be deleted locally. And that naturally\n> > extends to:\n> > \n> >   git fetch --prune origin refs/heads/*:refs/remotes/origin/*\n> \n> I have some code locally that solves this second part. If we are given\n> refspecs on the command-line, it will try to match against that instead\n> of blindly trusting what get_stale_heads tells us. I'm looking into\n> putting the logic into get_stale_heads so that we can trust it.\n> \n> The first part might be more complicated. If the remote head doesn't\n> exist, get_fetch_map dies. It does take a missing_ok flag, so it might\n> be as easy as passing 1 there; but I'm not sure what that would do for a\n> non-prune fetch.\n\nLet's not worry about the first one for now, then. It is a natural\nextension of the other, but in practice, I don't expect people to really\ncare that much about auto-pruning one specific ref. Instead, they want\nto prune wildcards. So as long as it works in the wildcard case, that is\na good start.\n\n> >       # backup to a repository where our objects will be shared\n> >       # with other related backups. So we must only use our slice of the\n> >       # ref namespace.\n> >       git push --mirror backup-repo +refs/*:refs/`hostname`/*\n> \n> Is --mirror needed there? I would have thought that\n> refs/*:refs/`hostname`/* would do the same by itself.\n\nI wanted it to auto-prune the remote branches. So if I delete\n\"refs/heads/foo\" locally, then it will be deleted from the backup on the\nnext push. Regular \"push\" will not do that, but --mirror will.\n\n-Peff\n"},{"id":"176836","messageId":"1317726379.2577.19.camel@bee.lab.cmartin.tk","threadId":"28502","inReplyTo":"20111004103624.GA11863@sigill.intra.peff.net","subject":"Re: [BUG?] git fetch -p -t prunes all non-tag refs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-04T11:06:18Z","receivedAt":"2011-10-04T11:06:18Z","isPatch":false,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Tue, 2011-10-04 at 06:36 -0400, Jeff King wrote:\n> On Tue, Oct 04, 2011 at 12:33:22PM +0200, Carlos Martín Nieto wrote:\n> \n> > > The latter. If I say:\n> > > \n> > >   git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n> > > \n> > > and refs/heads/master doesn't exist on the remote, I would expect\n> > > refs/remotes/origin/master to be deleted locally. And that naturally\n> > > extends to:\n> > > \n> > >   git fetch --prune origin refs/heads/*:refs/remotes/origin/*\n> > \n> > I have some code locally that solves this second part. If we are given\n> > refspecs on the command-line, it will try to match against that instead\n> > of blindly trusting what get_stale_heads tells us. I'm looking into\n> > putting the logic into get_stale_heads so that we can trust it.\n> > \n> > The first part might be more complicated. If the remote head doesn't\n> > exist, get_fetch_map dies. It does take a missing_ok flag, so it might\n> > be as easy as passing 1 there; but I'm not sure what that would do for a\n> > non-prune fetch.\n> \n> Let's not worry about the first one for now, then. It is a natural\n> extension of the other, but in practice, I don't expect people to really\n> care that much about auto-pruning one specific ref. Instead, they want\n> to prune wildcards. So as long as it works in the wildcard case, that is\n> a good start.\n\nI'm going to add the logic to do specific-ref-prunning because it's just\nadding a strcmp to an if (I may yet be proven wrong, mind) and if that\nworks, we can use it later. get_stale_heads is going to gain a couple of\narguments so it can inspect the user-given refspecs. It should now be a\nsimple matter of adding a check to see if the refspec's dst matches the\nrefname (it's a simple matter once you've spent five hours trying to\nunderstand what the fetch code does).\n\n> \n> > >       # backup to a repository where our objects will be shared\n> > >       # with other related backups. So we must only use our slice of the\n> > >       # ref namespace.\n> > >       git push --mirror backup-repo +refs/*:refs/`hostname`/*\n> > \n> > Is --mirror needed there? I would have thought that\n> > refs/*:refs/`hostname`/* would do the same by itself.\n> \n> I wanted it to auto-prune the remote branches. So if I delete\n> \"refs/heads/foo\" locally, then it will be deleted from the backup on the\n> next push. Regular \"push\" will not do that, but --mirror will.\n\nOK, gotcha.\n\n   cmn\n\n"},{"id":"177055","messageId":"1317920187-17389-1-git-send-email-cmn@elego.de","threadId":"28502","inReplyTo":"20111004103624.GA11863@sigill.intra.peff.net","subject":"[WIP PATCH 0/2] Be more careful when prunning","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T16:56:25Z","receivedAt":"2011-10-06T16:56:25Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"[I turns out that my system was slightly misconfigured, so vger didn't\nwant to accept my mail, this a re-send to the list only]\n\nHello,\n\nI consider this WIP because I haven't addressed the issue when --tags\nis given. It's likely the most common issue (and what sparked this),\nbut the second patch sets it up so it becomes easier.\n\nThe first patch is not that big a deal, but it's better if we're\nfreeing the refspecs, we might as well free all of them.\n\nThe second patch teaches get_stale_heads to use the user-provided\nrefspecs instead of the ones in the config. For example, running\n\n    git fetch --prune origin refs/heads/master:refs/heads/master\n\ndoesn't remove the other branches anymore. For a more interesting (and\nbelievable) example, let's take\n\n    git fetch --prune origin refs/heads/b/*:refs/heads/b/*\n\nbecause you want to prune the refs inside the b/ namespace\nonly. Currently git will delete all the refs that aren't under that\nnamespace. With the second patch applied, git won't remove any refs\noutside the b/ namespace.\n\nNow comes the interesting part: when --tags is given, there is no\nrefspec set up, fetch just sets up a global variable. What I'm\nthinking (and going to implement after dinner, unless people cry out\nagainst it) is this: just before calling prune_refs, add a refspec to\nthe user-provided list with the refspec refs/tags/*:refs/tags/*\nsimilar to what fetch_one does if you gave it \"tag v1.5.6\". This would\ncause us to ignore the configured refspec and keep the branches. The\nlack of '+' is most certainly on purpose. Is there anything\nfundamentally wrong with that idea?\n\nCheers,\n   cmn\n\nCarlos Martín Nieto (2):\n  fetch: free all the additional refspecs\n  fetch: honor the user-provided refspecs when pruning refs\n\n builtin/fetch.c  |    8 +++---\n builtin/remote.c |    2 +-\n remote.c         |   74 ++++++++++++++++++++++++++++++++++++++++++++++++-----\n remote.h         |    3 +-\n 4 files changed, 74 insertions(+), 13 deletions(-)\n\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177057","messageId":"1317920187-17389-2-git-send-email-cmn@elego.de","threadId":"28502","inReplyTo":"1317920187-17389-1-git-send-email-cmn@elego.de","subject":"[PATCH 1/2] fetch: free all the additional refspecs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T16:56:26Z","receivedAt":"2011-10-06T16:56:26Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Signed-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n builtin/fetch.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 7a4e41c..30b485e 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -883,7 +883,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)\n \tatexit(unlock_pack);\n \trefspec = parse_fetch_refspec(ref_nr, refs);\n \texit_code = do_fetch(transport, refspec, ref_nr);\n-\tfree(refspec);\n+\tfree_refspec(ref_nr, refspec);\n \ttransport_disconnect(transport);\n \ttransport = NULL;\n \treturn exit_code;\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177056","messageId":"1317920187-17389-3-git-send-email-cmn@elego.de","threadId":"28502","inReplyTo":"1317920187-17389-1-git-send-email-cmn@elego.de","subject":"[PATCH 2/2] fetch: honor the user-provided refspecs when pruning refs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T16:56:27Z","receivedAt":"2011-10-06T16:56:27Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"If the user gave us refspecs on the command line, we should use those\nwhen deciding whether to prune a ref instead of relying on the\nrefspecs in the config.\n\nPreviously, running\n\n    git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n\nwould delete every other tag under the origin namespace because we\nwere using the refspec to filter the available refs but using the\nconfigured refspec to figure out if a ref had been deleted on the\nremote. This is clearly the wrong thing to do.\n\nTeach get_stale_heads about user-provided refspecs and use them if\nthey're available.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n builtin/fetch.c  |    6 ++--\n builtin/remote.c |    2 +-\n remote.c         |   74 ++++++++++++++++++++++++++++++++++++++++++++++++-----\n remote.h         |    3 +-\n 4 files changed, 73 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 30b485e..b937d71 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -505,10 +505,10 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n \treturn ret;\n }\n \n-static int prune_refs(struct transport *transport, struct ref *ref_map)\n+static int prune_refs(struct transport *transport, struct refspec *refs, int n, struct ref *ref_map)\n {\n \tint result = 0;\n-\tstruct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map);\n+\tstruct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map, refs, n);\n \tconst char *dangling_msg = dry_run\n \t\t? _(\"   (%s will become dangling)\\n\")\n \t\t: _(\"   (%s has become dangling)\\n\");\n@@ -700,7 +700,7 @@ static int do_fetch(struct transport *transport,\n \t\treturn 1;\n \t}\n \tif (prune)\n-\t\tprune_refs(transport, ref_map);\n+\t\tprune_refs(transport, refs, ref_count, ref_map);\n \tfree_refs(ref_map);\n \n \t/* if neither --no-tags nor --tags was specified, do automated tag\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex b25dfb4..91a2148 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -349,7 +349,7 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat\n \t\telse\n \t\t\tstring_list_append(&states->tracked, abbrev_branch(ref->name));\n \t}\n-\tstale_refs = get_stale_heads(states->remote, fetch_map);\n+\tstale_refs = get_stale_heads(states->remote, fetch_map, NULL, 0);\n \tfor (ref = stale_refs; ref; ref = ref->next) {\n \t\tstruct string_list_item *item =\n \t\t\tstring_list_append(&states->stale, abbrev_branch(ref->name));\ndiff --git a/remote.c b/remote.c\nindex 7840d2f..72a26d3 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1684,26 +1684,84 @@ struct stale_heads_info {\n \tstruct remote *remote;\n \tstruct string_list *ref_names;\n \tstruct ref **stale_refs_tail;\n+\tstruct refspec *refs;\n+\tint ref_count;\n };\n \n+/*\n+ * Find a refspec to a remote's\n+ * Returns 0 on success, -1 if it couldn't find a the refspec\n+ */\n+static int find_in_refs(struct refspec *refs, int ref_count, struct refspec *query)\n+{\n+\tint i;\n+\tstruct refspec *refspec;\n+\n+\tfor (i = 0; i < ref_count; ++i) {\n+\t\trefspec = &refs[i];\n+\n+\t\t/*\n+\t\t * No '*' means that it must match exactly. If it does\n+\t\t * have it, try to match it against the pattern. If\n+\t\t * the refspec matches, store the ref name as it would\n+\t\t * appear in the server in query->src.\n+\t\t */\n+\t\tif (!strchr(refspec->dst, '*')) {\n+\t\t\tif (!strcmp(query->dst, refspec->dst)) {\n+\t\t\t\tquery->src = xstrdup(refspec->src);\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tif (match_name_with_pattern(refspec->dst, query->dst,\n+\t\t\t\t\t\t    refspec->src, &query->src)) {\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\treturn -1;\n+}\n+\n static int get_stale_heads_cb(const char *refname,\n \tconst unsigned char *sha1, int flags, void *cb_data)\n {\n \tstruct stale_heads_info *info = cb_data;\n \tstruct refspec refspec;\n+\tint ret;\n \tmemset(&refspec, 0, sizeof(refspec));\n \trefspec.dst = (char *)refname;\n-\tif (!remote_find_tracking(info->remote, &refspec)) {\n-\t\tif (!((flags & REF_ISSYMREF) ||\n-\t\t    string_list_has_string(info->ref_names, refspec.src))) {\n-\t\t\tstruct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);\n-\t\t\thashcpy(ref->new_sha1, sha1);\n-\t\t}\n+\n+\t/*\n+\t * If the user speicified refspecs on the command line, we\n+\t * should only use those to check. Otherwise, look in the\n+\t * remote's configuration for the branch.\n+\t */\n+\tif (info->ref_count)\n+\t\tret = find_in_refs(info->refs, info->ref_count, &refspec);\n+\telse\n+\t\tret = remote_find_tracking(info->remote, &refspec);\n+\n+\t/* No matches */\n+\tif (ret)\n+\t\treturn 0;\n+\n+\t/*\n+\t * If we did find a suitable refspec and it's not a symref and\n+\t * it's not in the list of refs that currently exist in that\n+\t * remote we consider it to be stale.\n+\t */\n+\tif (!((flags & REF_ISSYMREF) ||\n+\t      string_list_has_string(info->ref_names, refspec.src))) {\n+\t\tstruct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);\n+\t\thashcpy(ref->new_sha1, sha1);\n \t}\n+\n+\tfree(refspec.src);\n \treturn 0;\n }\n \n-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)\n+struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map,\n+\t\t\t    struct refspec *refs, int ref_count)\n {\n \tstruct ref *ref, *stale_refs = NULL;\n \tstruct string_list ref_names = STRING_LIST_INIT_NODUP;\n@@ -1711,6 +1769,8 @@ struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)\n \tinfo.remote = remote;\n \tinfo.ref_names = &ref_names;\n \tinfo.stale_refs_tail = &stale_refs;\n+\tinfo.refs = refs;\n+\tinfo.ref_count = ref_count;\n \tfor (ref = fetch_map; ref; ref = ref->next)\n \t\tstring_list_append(&ref_names, ref->name);\n \tsort_string_list(&ref_names);\ndiff --git a/remote.h b/remote.h\nindex 9a30a9d..2f753a0 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -164,6 +164,7 @@ struct ref *guess_remote_head(const struct ref *head,\n \t\t\t      int all);\n \n /* Return refs which no longer exist on remote */\n-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map);\n+struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map,\n+\t\t\t    struct refspec *refs, int ref_count);\n \n #endif\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177201","messageId":"7v39f4tnk8.fsf@alter.siamese.dyndns.org","threadId":"28502","inReplyTo":"1317920187-17389-1-git-send-email-cmn@elego.de","subject":"Re: [WIP PATCH 0/2] Be more careful when prunning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-07T23:00:07Z","receivedAt":"2011-10-07T23:00:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto <cmn@elego.de> writes:\n\n> Now comes the interesting part: when --tags is given, there is no\n> refspec set up, fetch just sets up a global variable. What I'm\n> thinking (and going to implement after dinner, unless people cry out\n> against it) is this: just before calling prune_refs, add a refspec to\n> the user-provided list with the refspec refs/tags/*:refs/tags/*\n> similar to what fetch_one does if you gave it \"tag v1.5.6\". This would\n> cause us to ignore the configured refspec and keep the branches. The\n> lack of '+' is most certainly on purpose. Is there anything\n> fundamentally wrong with that idea?\n\nIt sounds like that the approach should work and preserve the current\n\"fetch --tags\" semantics, but with one small caveat (which is not a\ndownside).\n\nAs was discussed in a few separate threads last month, in the longer term\nI think we should fix the semantics of \"fetch --tags\" to mean \"in addition\nto what you usually fetch with the configured refspecs, add that all\nmatching refs/tags/*:refs/tags/* to the refspec\" to reduce confusion.\n\"Only fetch tags\" may make sense if you have everything else, but by\nitself it is somewhat a senseless thing to do.\n\nThe semantics has been kept this way only from fear of breaking backward\ncompatibility, but because nobody wants to only fetch tags without\nbranches, this forces people to say \"git fetch && git fetch --tags\".\n\nWe should re-evaluate the design and change it at a major version\nboundary, I would think. And when that happens, we may need to rip out the\nspecial case you discussed above.\n\nThanks.\n"}]}