Re: [PATCH] fetch: document that pruning happens before fetching

2016-06-14 Thread Jeff King
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

2016-06-14 Thread Duy Nguyen
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

2016-06-14 Thread Duy Nguyen
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

2016-06-14 Thread Jacob Keller
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

2016-06-14 Thread Jeff King
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

2016-06-14 Thread Jacob Keller
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