Re: [PATCH v5 00/17] document & test fetch pruning & add fetch.pruneTags

[Ævar Arnfjörð Bjarmason]

On Fri, Feb 23 2018, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason  writes:
>
>>> Let's see how others find it useful and/or if the changed code gets
>>> in the way of others (I am not absolutely sure if the changes are
>>> free of regression to existing users who do not use the new
>>> feature).
>>
>> I think if you're on the fence about merging it down (and others don't
>> chime in saying the want it / like it) it makes sense to merge down
>> 1-14/17 and we could discard 15-17/17 for now for a later re-submission
>> and discussion once the earlier part of the series lands in master.
>
> Or we merge everything and let people discover glitches and
> complain.  Reverting the last pieces can wait until then ;-).

Thanks, that obviously works for me.

Re: [PATCH v5 00/17] document & test fetch pruning & add fetch.pruneTags

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

>> Let's see how others find it useful and/or if the changed code gets
>> in the way of others (I am not absolutely sure if the changes are
>> free of regression to existing users who do not use the new
>> feature).
>
> I think if you're on the fence about merging it down (and others don't
> chime in saying the want it / like it) it makes sense to merge down
> 1-14/17 and we could discard 15-17/17 for now for a later re-submission
> and discussion once the earlier part of the series lands in master.

Or we merge everything and let people discover glitches and
complain.  Reverting the last pieces can wait until then ;-).

Re: [PATCH v5 00/17] document & test fetch pruning & add fetch.pruneTags

[Ævar Arnfjörð Bjarmason]

On Thu, Feb 22 2018, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason  writes:
>
>> On Thu, Feb 22 2018, Junio C. Hamano jotted:
>>
>>> Ævar Arnfjörð Bjarmason   writes:
>>>
 Here's a v5 (correct subject line this time!). Many thanks to Eric for
 a thorough review.
>>>
>>> We haven't seen any comments on this round.  Is everybody happy?
>>>
>>> I do not have a strong opinion on the new feature, either for or
>>> against.  I didn't find anything majorly questionable in the
>>> execution, though, so...
>>
>> I've been running that here on thousands of boxes (that are actively
>> using it) for 2 weeks now without issue. Would be great to have it
>> merged down & in 2.17.
>
> If those thousands of boxes are all employing one specific workflow
> that is helped by these changes, and the workflow is that other
> people do not care about (or even worse, actively do not want to let
> their junior project members to use without thinking), then a
> data-point from the original author does not amount to much ;-)

Of course, I should have been clearer. I just meant to chime in with the
datapoint that I'm fairly sure this doesn't have any serious bugs given
the wide internal testing it's gotten.

> Let's see how others find it useful and/or if the changed code gets
> in the way of others (I am not absolutely sure if the changes are
> free of regression to existing users who do not use the new
> feature).

I think if you're on the fence about merging it down (and others don't
chime in saying the want it / like it) it makes sense to merge down
1-14/17 and we could discard 15-17/17 for now for a later re-submission
and discussion once the earlier part of the series lands in master.

The earlier part of the series is just trivial code changes that don't
change any functionality, more documentation for how existing
functionality works, and more thorough testing of existing
functionality.

Re: [PATCH v5 00/17] document & test fetch pruning & add fetch.pruneTags

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> On Thu, Feb 22 2018, Junio C. Hamano jotted:
>
>> Ævar Arnfjörð Bjarmason   writes:
>>
>>> Here's a v5 (correct subject line this time!). Many thanks to Eric for
>>> a thorough review.
>>
>> We haven't seen any comments on this round.  Is everybody happy?
>>
>> I do not have a strong opinion on the new feature, either for or
>> against.  I didn't find anything majorly questionable in the
>> execution, though, so...
>
> I've been running that here on thousands of boxes (that are actively
> using it) for 2 weeks now without issue. Would be great to have it
> merged down & in 2.17.

If those thousands of boxes are all employing one specific workflow
that is helped by these changes, and the workflow is that other
people do not care about (or even worse, actively do not want to let
their junior project members to use without thinking), then a
data-point from the original author does not amount to much ;-)

Let's see how others find it useful and/or if the changed code gets
in the way of others (I am not absolutely sure if the changes are
free of regression to existing users who do not use the new
feature).

Thanks.

Re: [PATCH v5 00/17] document & test fetch pruning & add fetch.pruneTags

[Ævar Arnfjörð Bjarmason]

On Thu, Feb 22 2018, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason   writes:
>
>> Here's a v5 (correct subject line this time!). Many thanks to Eric for
>> a thorough review.
>
> We haven't seen any comments on this round.  Is everybody happy?
>
> I do not have a strong opinion on the new feature, either for or
> against.  I didn't find anything majorly questionable in the
> execution, though, so...

I've been running that here on thousands of boxes (that are actively
using it) for 2 weeks now without issue. Would be great to have it
merged down & in 2.17.

Re: [PATCH v5 00/17] document & test fetch pruning & add fetch.pruneTags

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> Here's a v5 (correct subject line this time!). Many thanks to Eric for
> a thorough review.

We haven't seen any comments on this round.  Is everybody happy?

I do not have a strong opinion on the new feature, either for or
against.  I didn't find anything majorly questionable in the
execution, though, so...

[PATCH v5 00/17] document & test fetch pruning & add fetch.pruneTags

[Ævar Arnfjörð Bjarmason]

Here's a v5 (correct subject line this time!). Many thanks to Eric for
a thorough review.

I'll spare you the per-patch changelog. These are all minor commit
message / doc / comment wording changes, with the exception of making
a bit of the test code better, and adding a \n for grep
portability. tbdiff at the end.

Ævar Arnfjörð Bjarmason (17):
  fetch: don't redundantly NULL something calloc() gave us
  fetch: trivially refactor assignment to ref_nr
  fetch: stop accessing "remote" variable indirectly
  remote: add a macro for "refs/tags/*:refs/tags/*"
  fetch tests: refactor in preparation for testing tag pruning
  fetch tests: re-arrange arguments for future readability
  fetch tests: add a tag to be deleted to the pruning tests
  fetch tests: test --prune and refspec interaction
  fetch tests: double quote a variable for interpolation
  fetch tests: expand case/esac for later change
  fetch tests: fetch   as well as fetch []
  git fetch doc: add a new section to explain the ins & outs of pruning
  git remote doc: correct dangerous lies about what prune does
  git-fetch & config doc: link to the new PRUNING section
  fetch tests: add scaffolding for the new fetch.pruneTags
  fetch: add a --prune-tags option and fetch.pruneTags config
  fetch: make the --prune-tags work with 

 Documentation/config.txt   |  20 ++-
 Documentation/fetch-options.txt|  17 ++-
 Documentation/git-fetch.txt|  87 
 Documentation/git-remote.txt   |  14 +-
 builtin/fetch.c|  54 ++--
 contrib/completion/git-completion.bash |   2 +-
 remote.c   |  15 +++
 remote.h   |   5 +
 t/t5510-fetch.sh   | 238 +++--
 9 files changed, 391 insertions(+), 61 deletions(-)

tbdiff:

 1: da0992d97c =  1: da0992d97c fetch: don't redundantly NULL something 
calloc() gave us
 2: c781d18a29 !  2: 899430a3b2 fetch: trivially refactor assignment to ref_nr
@@ -7,8 +7,8 @@
 "j" is, and "j" is only incremented in that loop, so this change isn't
 a logic error.
 
-This change makes a subsequent change which splits the incrementing of
-"ref_nr" into two blocks.
+This change simplifies a subsequent change, which will split the
+incrementing of "ref_nr" into two blocks.
 
 Signed-off-by: Ævar Arnfjörð Bjarmason 
 
 3: 1203ef6e35 =  3: 98e3a28bdf fetch: stop accessing "remote" variable 
indirectly
 4: 1d7956c444 =  4: 384c1fc318 remote: add a macro for 
"refs/tags/*:refs/tags/*"
 5: a0dc3eb024 =  5: ec92777861 fetch tests: refactor in preparation for 
testing tag pruning
 6: 7ed1561e3d =  6: 1c23526223 fetch tests: re-arrange arguments for future 
readability
 7: 6dbf0e688d =  7: 59abe07b71 fetch tests: add a tag to be deleted to the 
pruning tests
 8: 9fc3589793 =  8: af7acef671 fetch tests: test --prune and refspec 
interaction
 9: a4487f9389 =  9: cb72187362 fetch tests: double quote a variable for 
interpolation
10: b8c07e2d42 = 10: 300c1c0985 fetch tests: expand case/esac for later change
11: 6341131ee0 ! 11: 550e7df594 fetch tests: fetch   as well as 
fetch []
@@ -60,15 +60,12 @@
 -  test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${5:+ 
$5}; branch:$3 tag:$4" '
 +  mode=$6
 +
-+  if ! test -e prune-type-setup-done
++  if test -z "$cmdline_setup"
 +  then
-+  test_expect_success 'prune_type setup' '
-+  git -C one config remote.origin.url >one.remote-url &&
-+  git -C one config remote.origin.fetch >one.remote-fetch 
&&
-+  remote_url="file://$(cat one.remote-url)" &&
-+  remote_fetch="$(cat one.remote-fetch)" &&
-+  cmdline_setup="\"$remote_url\" \"$remote_fetch\"" &&
-+  touch prune-type-setup-done
++  test_expect_success 'setup cmdline_setup variable for 
subsequent test' '
++  remote_url="file://$(git -C one config 
remote.origin.url)" &&
++  remote_fetch="$(git -C one config remote.origin.fetch)" 
&&
++  cmdline_setup="\"$remote_url\" \"$remote_fetch\""
 +  '
 +  fi
 +
12: 824c3ed4c1 ! 12: 273f4c603f git fetch doc: add a new section to explain the 
ins & outs of pruning
@@ -32,7 +32,7 @@
 +` needlessly verbose, as well as impacting anything else
 +that'll work with the complete set of known references.
 +
-+These remote tracking references can be deleted as a one-off with
++These remote-tracking references can be deleted as a one-off with
 +either of:
 +
 +
@@ -44,13 +44,13 @@
 +
 +
 +To prune references as part of your normal workflow without needing to