{"thread":{"id":"42598","subject":"[PATCH] fetch: document that pruning happens before fetching","startedAt":"2016-06-16T06:55:52Z","lastAt":"2016-06-16T06:55:53Z","messageCount":8,"participants":["Jeff King","Jacob Keller","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"289180","messageId":"20160613235850.GA8009@sigill.intra.peff.net","threadId":"42598","inReplyTo":null,"subject":"[PATCH] fetch: document that pruning happens before fetching","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-13T23:58:51Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This was changed in 10a6cc8 (fetch --prune: Run prune before\nfetching, 2014-01-02), but it seems that nobody in that\ndiscussion realized we were advertising the \"after\"\nexplicitly.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI include myself in that \"nobody\" of course. :)\n\n Documentation/fetch-options.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex 036edfb..b05a834 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -52,7 +52,7 @@ ifndef::git-pull[]\n \n -p::\n --prune::\n-\tAfter fetching, remove any remote-tracking references that no\n+\tBefore fetching, remove any remote-tracking references that no\n \tlonger exist on the remote.  Tags are not subject to pruning\n \tif they are fetched only because of the default tag\n \tauto-following or due to a --tags option.  However, if tags\n-- \n2.9.0.150.g8bd4cf6\n"},{"id":"289186","messageId":"CA+P7+xpk6HqeJfUcAkpTbW_hO6dyjanXoqaaKqdTU363n=-Stw@mail.gmail.com","threadId":"42598","inReplyTo":"20160613235850.GA8009@sigill.intra.peff.net","subject":"Re: [PATCH] fetch: document that pruning happens before fetching","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-06-14T06:14:36Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Jun 13, 2016 at 4:58 PM, Jeff King <peff@peff.net> wrote:\n> This was changed in 10a6cc8 (fetch --prune: Run prune before\n> fetching, 2014-01-02), but it seems that nobody in that\n> discussion realized we were advertising the \"after\"\n> explicitly.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> I include myself in that \"nobody\" of course. :)\n>\n>  Documentation/fetch-options.txt | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\n> index 036edfb..b05a834 100644\n> --- a/Documentation/fetch-options.txt\n> +++ b/Documentation/fetch-options.txt\n> @@ -52,7 +52,7 @@ ifndef::git-pull[]\n>\n>  -p::\n>  --prune::\n> -       After fetching, remove any remote-tracking references that no\n> +       Before fetching, remove any remote-tracking references that no\n>         longer exist on the remote.  Tags are not subject to pruning\n>         if they are fetched only because of the default tag\n>         auto-following or due to a --tags option.  However, if tags\n\nWhat's the difference in behavior due to pruning before instead of\nafter? Curious. It seems like pruning after would make more sense?\n"},{"id":"289188","messageId":"20160614061752.GA11935@sigill.intra.peff.net","threadId":"42598","inReplyTo":"CA+P7+xpk6HqeJfUcAkpTbW_hO6dyjanXoqaaKqdTU363n=-Stw@mail.gmail.com","subject":"Re: [PATCH] fetch: document that pruning happens before fetching","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-14T06:17:53Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 13, 2016 at 11:14:36PM -0700, Jacob Keller wrote:\n\n> On Mon, Jun 13, 2016 at 4:58 PM, Jeff King <peff@peff.net> wrote:\n> > This was changed in 10a6cc8 (fetch --prune: Run prune before\n> > fetching, 2014-01-02), but it seems that nobody in that\n> > discussion realized we were advertising the \"after\"\n> > explicitly.\n> >\n> > Signed-off-by: Jeff King <peff@peff.net>\n> > ---\n> > I include myself in that \"nobody\" of course. :)\n> >\n> >  Documentation/fetch-options.txt | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\n> > index 036edfb..b05a834 100644\n> > --- a/Documentation/fetch-options.txt\n> > +++ b/Documentation/fetch-options.txt\n> > @@ -52,7 +52,7 @@ ifndef::git-pull[]\n> >\n> >  -p::\n> >  --prune::\n> > -       After fetching, remove any remote-tracking references that no\n> > +       Before fetching, remove any remote-tracking references that no\n> >         longer exist on the remote.  Tags are not subject to pruning\n> >         if they are fetched only because of the default tag\n> >         auto-following or due to a --tags option.  However, if tags\n> \n> What's the difference in behavior due to pruning before instead of\n> after? Curious. It seems like pruning after would make more sense?\n\nSee 10a6cc8. :)\n\nBasically, you have to prune first to make way for new incoming refs\nwhen there is a D/F conflict.\n\nThe downside is that there is a moment where objects may be unreferenced\n(e.g., if upstream moved \"foo\" to \"bar\", we delete \"foo\" and _then_\ncreate \"bar\"). And due to the way refs are stored, we do not keep even a\nreflog for the deleted ref in the interim.\n\n-Peff\n"},{"id":"289189","messageId":"CA+P7+xq50QQEKxwdApPjd4-01wX7dsBwj5dsbLH+d7Ot05JjSQ@mail.gmail.com","threadId":"42598","inReplyTo":"20160614061752.GA11935@sigill.intra.peff.net","subject":"Re: [PATCH] fetch: document that pruning happens before fetching","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-06-14T06:18:51Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Jun 13, 2016 at 11:17 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Jun 13, 2016 at 11:14:36PM -0700, Jacob Keller wrote:\n>\n>> On Mon, Jun 13, 2016 at 4:58 PM, Jeff King <peff@peff.net> wrote:\n>> > This was changed in 10a6cc8 (fetch --prune: Run prune before\n>> > fetching, 2014-01-02), but it seems that nobody in that\n>> > discussion realized we were advertising the \"after\"\n>> > explicitly.\n>> >\n>> > Signed-off-by: Jeff King <peff@peff.net>\n>> > ---\n>> > I include myself in that \"nobody\" of course. :)\n>> >\n>> >  Documentation/fetch-options.txt | 2 +-\n>> >  1 file changed, 1 insertion(+), 1 deletion(-)\n>> >\n>> > diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\n>> > index 036edfb..b05a834 100644\n>> > --- a/Documentation/fetch-options.txt\n>> > +++ b/Documentation/fetch-options.txt\n>> > @@ -52,7 +52,7 @@ ifndef::git-pull[]\n>> >\n>> >  -p::\n>> >  --prune::\n>> > -       After fetching, remove any remote-tracking references that no\n>> > +       Before fetching, remove any remote-tracking references that no\n>> >         longer exist on the remote.  Tags are not subject to pruning\n>> >         if they are fetched only because of the default tag\n>> >         auto-following or due to a --tags option.  However, if tags\n>>\n>> What's the difference in behavior due to pruning before instead of\n>> after? Curious. It seems like pruning after would make more sense?\n>\n> See 10a6cc8. :)\n>\n> Basically, you have to prune first to make way for new incoming refs\n> when there is a D/F conflict.\n>\n> The downside is that there is a moment where objects may be unreferenced\n> (e.g., if upstream moved \"foo\" to \"bar\", we delete \"foo\" and _then_\n> create \"bar\"). And due to the way refs are stored, we do not keep even a\n> reflog for the deleted ref in the interim.\n>\n> -Peff\n\nAh that makes sense, thanks.\n\nRegards,\nJake\n"},{"id":"289198","messageId":"CACsJy8Ds_DnBLJ0jx7Pp9EH1coG-xAtri4w4hh__=nigCmBbdA@mail.gmail.com","threadId":"42598","inReplyTo":"20160613235850.GA8009@sigill.intra.peff.net","subject":"Re: [PATCH] fetch: document that pruning happens before fetching","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-14T09:36:59Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jun 14, 2016 at 6:58 AM, Jeff King <peff@peff.net> wrote:\n> This was changed in 10a6cc8 (fetch --prune: Run prune before\n> fetching, 2014-01-02), but it seems that nobody in that\n> discussion realized we were advertising the \"after\"\n> explicitly.\n\nAh... ok. Good to know it's moved up top on purpose because I almost\ntried to move it down :) It's irritating that current output looks\nlike this\n\n<delete ref>\n<delete ref>\n<delete ref>\nremote: <random progress lines>\n<update ref>\n<update ref>\n<update ref>\n\nIt probably looks better if we can move the <delete ref> part after\n\"remote: ...\" lines (iow still _after_ fetch, but _before_ ref\nupdates), e.g.\n\nremote: <random progress lines>\n<delete ref>\n<delete ref>\n<delete ref>\n<update ref>\n<update ref>\n<update ref>\n\nIf we do so, there's no need to update document. But I don't know,\nmaybe it's not worth doing.\n\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> I include myself in that \"nobody\" of course. :)\n>\n>  Documentation/fetch-options.txt | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\n> index 036edfb..b05a834 100644\n> --- a/Documentation/fetch-options.txt\n> +++ b/Documentation/fetch-options.txt\n> @@ -52,7 +52,7 @@ ifndef::git-pull[]\n>\n>  -p::\n>  --prune::\n> -       After fetching, remove any remote-tracking references that no\n> +       Before fetching, remove any remote-tracking references that no\n>         longer exist on the remote.  Tags are not subject to pruning\n>         if they are fetched only because of the default tag\n>         auto-following or due to a --tags option.  However, if tags\n> --\n> 2.9.0.150.g8bd4cf6\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-- \nDuy\n"},{"id":"289201","messageId":"20160614102210.GA14357@sigill.intra.peff.net","threadId":"42598","inReplyTo":"CACsJy8Ds_DnBLJ0jx7Pp9EH1coG-xAtri4w4hh__=nigCmBbdA@mail.gmail.com","subject":"Re: [PATCH] fetch: document that pruning happens before fetching","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-14T10:22:10Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 04:36:59PM +0700, Duy Nguyen wrote:\n\n> On Tue, Jun 14, 2016 at 6:58 AM, Jeff King <peff@peff.net> wrote:\n> > This was changed in 10a6cc8 (fetch --prune: Run prune before\n> > fetching, 2014-01-02), but it seems that nobody in that\n> > discussion realized we were advertising the \"after\"\n> > explicitly.\n> \n> Ah... ok. Good to know it's moved up top on purpose because I almost\n> tried to move it down :) It's irritating that current output looks\n> like this\n> \n> <delete ref>\n> <delete ref>\n> <delete ref>\n> remote: <random progress lines>\n> <update ref>\n> <update ref>\n> <update ref>\n> \n> It probably looks better if we can move the <delete ref> part after\n> \"remote: ...\" lines (iow still _after_ fetch, but _before_ ref\n> updates), e.g.\n> \n> remote: <random progress lines>\n> <delete ref>\n> <delete ref>\n> <delete ref>\n> <update ref>\n> <update ref>\n> <update ref>\n\nI don't think it would be hard to do the deletion separate from the\nstatus-printing (in the worst case, you could simply queue the list of\ndeleted refs and then print them later).\n\nThat might put the status lines for the deletions farther from any\nerrors or warnings we print when doing the actual deletions, but in\ntheory the error lines are self-contained and can stand on their own.\n\n> If we do so, there's no need to update document. But I don't know,\n> maybe it's not worth doing.\n\nI think the documentation should be updated either way. This is not\nabout the ordering in the status table, but rather about the order of\nthe real operations. The user may care about that ordering if they want\nto know what races are possible, or what would happen if the packfile\nfetch failed (we'd already have deleted the old refs, but wouldn't fetch\nthe new ones).\n\n-Peff\n"},{"id":"289202","messageId":"CACsJy8Bbd0=vnbaj4-2m7WKfvWG6PfyXLg1h+970HPsak1hnMQ@mail.gmail.com","threadId":"42598","inReplyTo":"20160614102210.GA14357@sigill.intra.peff.net","subject":"Re: [PATCH] fetch: document that pruning happens before fetching","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-14T10:36:35Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jun 14, 2016 at 5:22 PM, Jeff King <peff@peff.net> wrote:\n> I think the documentation should be updated either way. This is not\n> about the ordering in the status table, but rather about the order of\n> the real operations. The user may care about that ordering if they want\n> to know what races are possible\n\nGood point.\n\n> or what would happen if the packfile\n> fetch failed (we'd already have deleted the old refs, but wouldn't fetch\n> the new ones).\n\nOff topic, but this sounds like a bug to me. We could have kept ref\nupdate more consistent (maybe at some point we could even do atomic\ntransaction update for all refs if there's a need for it).\n-- \nDuy\n"},{"id":"289273","messageId":"20160615034629.GB3013@sigill.intra.peff.net","threadId":"42598","inReplyTo":"CACsJy8Bbd0=vnbaj4-2m7WKfvWG6PfyXLg1h+970HPsak1hnMQ@mail.gmail.com","subject":"Re: [PATCH] fetch: document that pruning happens before fetching","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-15T03:46:29Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 05:36:35PM +0700, Duy Nguyen wrote:\n\n> > or what would happen if the packfile\n> > fetch failed (we'd already have deleted the old refs, but wouldn't fetch\n> > the new ones).\n> \n> Off topic, but this sounds like a bug to me. We could have kept ref\n> update more consistent (maybe at some point we could even do atomic\n> transaction update for all refs if there's a need for it).\n\nI do think atomic ref transactions are a nice idea, and would probably\nnot be too hard to implement these days (the main thing is just\nrefactoring the deep call stack in git-fetch).\n\nIt's possible it could be annoying when you have a single broken ref\n(and would prefer to just ignore it), but that _should_ be the rare\ncase.\n\n-Peff\n"}]}