Re: [PATCH] fetch: document that pruning happens before fetching
On Tue, Jun 14, 2016 at 05:36:35PM +0700, Duy Nguyen wrote: > > or what would happen if the packfile > > fetch failed (we'd already have deleted the old refs, but wouldn't fetch > > the new ones). > > Off topic, but this sounds like a bug to me. We could have kept ref > update more consistent (maybe at some point we could even do atomic > transaction update for all refs if there's a need for it). I do think atomic ref transactions are a nice idea, and would probably not be too hard to implement these days (the main thing is just refactoring the deep call stack in git-fetch). It's possible it could be annoying when you have a single broken ref (and would prefer to just ignore it), but that _should_ be the rare case. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fetch: document that pruning happens before fetching
On Tue, Jun 14, 2016 at 5:22 PM, Jeff King wrote: > I think the documentation should be updated either way. This is not > about the ordering in the status table, but rather about the order of > the real operations. The user may care about that ordering if they want > to know what races are possible Good point. > or what would happen if the packfile > fetch failed (we'd already have deleted the old refs, but wouldn't fetch > the new ones). Off topic, but this sounds like a bug to me. We could have kept ref update more consistent (maybe at some point we could even do atomic transaction update for all refs if there's a need for it). -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fetch: document that pruning happens before fetching
On Tue, Jun 14, 2016 at 04:36:59PM +0700, Duy Nguyen wrote: > On Tue, Jun 14, 2016 at 6:58 AM, Jeff King wrote: > > This was changed in 10a6cc8 (fetch --prune: Run prune before > > fetching, 2014-01-02), but it seems that nobody in that > > discussion realized we were advertising the "after" > > explicitly. > > Ah... ok. Good to know it's moved up top on purpose because I almost > tried to move it down :) It's irritating that current output looks > like this > > > > > remote: > > > > > It probably looks better if we can move the part after > "remote: ..." lines (iow still _after_ fetch, but _before_ ref > updates), e.g. > > remote: > > > > > > I don't think it would be hard to do the deletion separate from the status-printing (in the worst case, you could simply queue the list of deleted refs and then print them later). That might put the status lines for the deletions farther from any errors or warnings we print when doing the actual deletions, but in theory the error lines are self-contained and can stand on their own. > If we do so, there's no need to update document. But I don't know, > maybe it's not worth doing. I think the documentation should be updated either way. This is not about the ordering in the status table, but rather about the order of the real operations. The user may care about that ordering if they want to know what races are possible, or what would happen if the packfile fetch failed (we'd already have deleted the old refs, but wouldn't fetch the new ones). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fetch: document that pruning happens before fetching
On Tue, Jun 14, 2016 at 6:58 AM, Jeff King wrote: > This was changed in 10a6cc8 (fetch --prune: Run prune before > fetching, 2014-01-02), but it seems that nobody in that > discussion realized we were advertising the "after" > explicitly. Ah... ok. Good to know it's moved up top on purpose because I almost tried to move it down :) It's irritating that current output looks like this remote: It probably looks better if we can move the part after "remote: ..." lines (iow still _after_ fetch, but _before_ ref updates), e.g. remote: If we do so, there's no need to update document. But I don't know, maybe it's not worth doing. > Signed-off-by: Jeff King > --- > I include myself in that "nobody" of course. :) > > Documentation/fetch-options.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt > index 036edfb..b05a834 100644 > --- a/Documentation/fetch-options.txt > +++ b/Documentation/fetch-options.txt > @@ -52,7 +52,7 @@ ifndef::git-pull[] > > -p:: > --prune:: > - After fetching, remove any remote-tracking references that no > + Before fetching, remove any remote-tracking references that no > longer exist on the remote. Tags are not subject to pruning > if they are fetched only because of the default tag > auto-following or due to a --tags option. However, if tags > -- > 2.9.0.150.g8bd4cf6 > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fetch: document that pruning happens before fetching
On Mon, Jun 13, 2016 at 11:17 PM, Jeff King wrote: > On Mon, Jun 13, 2016 at 11:14:36PM -0700, Jacob Keller wrote: > >> On Mon, Jun 13, 2016 at 4:58 PM, Jeff King wrote: >> > This was changed in 10a6cc8 (fetch --prune: Run prune before >> > fetching, 2014-01-02), but it seems that nobody in that >> > discussion realized we were advertising the "after" >> > explicitly. >> > >> > Signed-off-by: Jeff King >> > --- >> > I include myself in that "nobody" of course. :) >> > >> > Documentation/fetch-options.txt | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/Documentation/fetch-options.txt >> > b/Documentation/fetch-options.txt >> > index 036edfb..b05a834 100644 >> > --- a/Documentation/fetch-options.txt >> > +++ b/Documentation/fetch-options.txt >> > @@ -52,7 +52,7 @@ ifndef::git-pull[] >> > >> > -p:: >> > --prune:: >> > - After fetching, remove any remote-tracking references that no >> > + Before fetching, remove any remote-tracking references that no >> > longer exist on the remote. Tags are not subject to pruning >> > if they are fetched only because of the default tag >> > auto-following or due to a --tags option. However, if tags >> >> What's the difference in behavior due to pruning before instead of >> after? Curious. It seems like pruning after would make more sense? > > See 10a6cc8. :) > > Basically, you have to prune first to make way for new incoming refs > when there is a D/F conflict. > > The downside is that there is a moment where objects may be unreferenced > (e.g., if upstream moved "foo" to "bar", we delete "foo" and _then_ > create "bar"). And due to the way refs are stored, we do not keep even a > reflog for the deleted ref in the interim. > > -Peff Ah that makes sense, thanks. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fetch: document that pruning happens before fetching
On Mon, Jun 13, 2016 at 11:14:36PM -0700, Jacob Keller wrote: > On Mon, Jun 13, 2016 at 4:58 PM, Jeff King wrote: > > This was changed in 10a6cc8 (fetch --prune: Run prune before > > fetching, 2014-01-02), but it seems that nobody in that > > discussion realized we were advertising the "after" > > explicitly. > > > > Signed-off-by: Jeff King > > --- > > I include myself in that "nobody" of course. :) > > > > Documentation/fetch-options.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/fetch-options.txt > > b/Documentation/fetch-options.txt > > index 036edfb..b05a834 100644 > > --- a/Documentation/fetch-options.txt > > +++ b/Documentation/fetch-options.txt > > @@ -52,7 +52,7 @@ ifndef::git-pull[] > > > > -p:: > > --prune:: > > - After fetching, remove any remote-tracking references that no > > + Before fetching, remove any remote-tracking references that no > > longer exist on the remote. Tags are not subject to pruning > > if they are fetched only because of the default tag > > auto-following or due to a --tags option. However, if tags > > What's the difference in behavior due to pruning before instead of > after? Curious. It seems like pruning after would make more sense? See 10a6cc8. :) Basically, you have to prune first to make way for new incoming refs when there is a D/F conflict. The downside is that there is a moment where objects may be unreferenced (e.g., if upstream moved "foo" to "bar", we delete "foo" and _then_ create "bar"). And due to the way refs are stored, we do not keep even a reflog for the deleted ref in the interim. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fetch: document that pruning happens before fetching
On Mon, Jun 13, 2016 at 4:58 PM, Jeff King wrote: > This was changed in 10a6cc8 (fetch --prune: Run prune before > fetching, 2014-01-02), but it seems that nobody in that > discussion realized we were advertising the "after" > explicitly. > > Signed-off-by: Jeff King > --- > I include myself in that "nobody" of course. :) > > Documentation/fetch-options.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt > index 036edfb..b05a834 100644 > --- a/Documentation/fetch-options.txt > +++ b/Documentation/fetch-options.txt > @@ -52,7 +52,7 @@ ifndef::git-pull[] > > -p:: > --prune:: > - After fetching, remove any remote-tracking references that no > + Before fetching, remove any remote-tracking references that no > longer exist on the remote. Tags are not subject to pruning > if they are fetched only because of the default tag > auto-following or due to a --tags option. However, if tags What's the difference in behavior due to pruning before instead of after? Curious. It seems like pruning after would make more sense? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fetch: document that pruning happens before fetching
This was changed in 10a6cc8 (fetch --prune: Run prune before fetching, 2014-01-02), but it seems that nobody in that discussion realized we were advertising the "after" explicitly. Signed-off-by: Jeff King --- I include myself in that "nobody" of course. :) Documentation/fetch-options.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 036edfb..b05a834 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -52,7 +52,7 @@ ifndef::git-pull[] -p:: --prune:: - After fetching, remove any remote-tracking references that no + Before fetching, remove any remote-tracking references that no longer exist on the remote. Tags are not subject to pruning if they are fetched only because of the default tag auto-following or due to a --tags option. However, if tags -- 2.9.0.150.g8bd4cf6 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
