Re: [PATCH v3 0/5] clone: --no-tags option

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> Junio, can you please just take patch 1-3 in this series, i.e.:

Given that the old one has been already sitting in 'next' since
April 23rd, I'd have to revert it and requeue them anew.  Which is
possible but cumbersome,...

Let me take a look at these three patches and then decide what to
do.  Thanks.

Re: [PATCH v3 0/5] clone: --no-tags option

[Stefan Beller]

On Fri, Apr 28, 2017 at 12:11 PM, Ævar Arnfjörð Bjarmason
 wrote:
> On Thu, Apr 27, 2017 at 1:12 AM, Ævar Arnfjörð Bjarmason
>  wrote:
>> This is an expansion of the previously solo 02/05 "clone: add a
>> --no-tags option to clone without tags" patch (see
>> <20170418191553.15464-1-ava...@gmail.com>).
>>
>> This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a
>> lot), and in addition implements a --no-tags-submodules option. That
>> code was implemented by Brandon & sent to me privately after I'd
>> failed to come up with it, but I added tests, a commit message & bash
>> completion to it.
>>
>> The WIP 5/5 patch implements a submodule.NAME.tags config facility for
>> the option, but is broken currently & floats along in this submission
>> as an RFC patch. AFAICT it *should* work and it goes through all the
>> motions the similar existing *.shallow config does, but for some
>> reason the tags=false option isn't picked up & propagated in a freshly
>> cloned submodule.
>>
>> I'm probably missing something trivial, but I can't see what it is,
>> I'm hoping thath either Stefan or Brandon will see what that is.
>
> Junio, can you please just take patch 1-3 in this series, i.e.:
>
> DROP:
>
>> Brandon Williams (1):
>>   clone: add a --no-tags-submodules to pass --no-tags to submodules
>> [...]
>>   WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config
>
> KEEP:
>
>> Ævar Arnfjörð Bjarmason (4):
>>   tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>>   clone: add a --no-tags option to clone without tags
>>   tests: rename a test having to do with shallow submodules
>
> I think a fair summary of the discussion so far is that the submodule
> interaction I copy/pasted from --shallow-submodules isn't how we want
> to do things at all, I defer to Stefan & Brandon on that.

ok. In that case we'd want to discuss what we actually want with no-tags
in submodules.

>
> The only other objection to the patches marked as KEEP is from Stefan
> saying it should be --tags on by default not --no-tags off by default.
> As noted in 
> 
> I don't disagree, but a lot of flags in git now use that pattern, and
> this change is consistent with those. Makes sense to discuss changing
> that as another series.

Ok. I assumed with that next series on the radar, we'd not want to intentionally
add more of the no-OPTIONs as that would produce more work for that series.

Re: [PATCH v3 0/5] clone: --no-tags option

[Ævar Arnfjörð Bjarmason]

On Thu, Apr 27, 2017 at 1:12 AM, Ævar Arnfjörð Bjarmason
 wrote:
> This is an expansion of the previously solo 02/05 "clone: add a
> --no-tags option to clone without tags" patch (see
> <20170418191553.15464-1-ava...@gmail.com>).
>
> This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a
> lot), and in addition implements a --no-tags-submodules option. That
> code was implemented by Brandon & sent to me privately after I'd
> failed to come up with it, but I added tests, a commit message & bash
> completion to it.
>
> The WIP 5/5 patch implements a submodule.NAME.tags config facility for
> the option, but is broken currently & floats along in this submission
> as an RFC patch. AFAICT it *should* work and it goes through all the
> motions the similar existing *.shallow config does, but for some
> reason the tags=false option isn't picked up & propagated in a freshly
> cloned submodule.
>
> I'm probably missing something trivial, but I can't see what it is,
> I'm hoping thath either Stefan or Brandon will see what that is.

Junio, can you please just take patch 1-3 in this series, i.e.:

DROP:

> Brandon Williams (1):
>   clone: add a --no-tags-submodules to pass --no-tags to submodules
> [...]
>   WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config

KEEP:

> Ævar Arnfjörð Bjarmason (4):
>   tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>   clone: add a --no-tags option to clone without tags
>   tests: rename a test having to do with shallow submodules

I think a fair summary of the discussion so far is that the submodule
interaction I copy/pasted from --shallow-submodules isn't how we want
to do things at all, I defer to Stefan & Brandon on that.

The only other objection to the patches marked as KEEP is from Stefan
saying it should be --tags on by default not --no-tags off by default.
As noted in 
I don't disagree, but a lot of flags in git now use that pattern, and
this change is consistent with those. Makes sense to discuss changing
that as another series.

Re: [PATCH v3 0/5] clone: --no-tags option

[Ævar Arnfjörð Bjarmason]

On Fri, Apr 28, 2017 at 12:27 AM, Brandon Williams  wrote:
> On 04/26, Ævar Arnfjörð Bjarmason wrote:
>> This is an expansion of the previously solo 02/05 "clone: add a
>> --no-tags option to clone without tags" patch (see
>> <20170418191553.15464-1-ava...@gmail.com>).
>>
>> This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a
>> lot), and in addition implements a --no-tags-submodules option. That
>> code was implemented by Brandon & sent to me privately after I'd
>> failed to come up with it, but I added tests, a commit message & bash
>> completion to it.
>
> Na you would have come up with it, I've just lived in submodule land a
> little too long (though not as long as Stephan has!) :D
>
>> The WIP 5/5 patch implements a submodule.NAME.tags config facility for
>> the option, but is broken currently & floats along in this submission
>> as an RFC patch. AFAICT it *should* work and it goes through all the
>> motions the similar existing *.shallow config does, but for some
>> reason the tags=false option isn't picked up & propagated in a freshly
>> cloned submodule.
>>
>> I'm probably missing something trivial, but I can't see what it is,
>> I'm hoping thath either Stefan or Brandon will see what that is.
>
> Overall the series looks good.  I've mentioned in the other threads that
> it probably makes more sense to have --recurse-submodules simply pass
> through known good commands to its children (e.g. --no-tags) simply
> because it makes the UX a little bit easier to work with (I don't have
> to remember all the fancy --OPT-submodules stuff, only
> --recurse-submodules).  That is unless you have some good rational that
> I'm not considering (completely possible :D).

I have no good (or bad) reason for that other than just wanting to add
--no-tags to submodules while I was at it, and then I was just
following the pattern the option to pass along --depth was
establishing.

But if that's some anti-pattern and the consensus is that this
submodule feature should instead work as you describe (which looks
like the case) I'll change it to work like that.

>>
>> Brandon Williams (1):
>>   clone: add a --no-tags-submodules to pass --no-tags to submodules
>>
>> Ævar Arnfjörð Bjarmason (4):
>>   tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>>   clone: add a --no-tags option to clone without tags
>>   tests: rename a test having to do with shallow submodules
>>   WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config
>>
>>  Documentation/git-clone.txt|  21 
>>  Documentation/git-submodule.txt|   8 +-
>>  builtin/clone.c|  19 +++-
>>  builtin/submodule--helper.c|  21 +++-
>>  contrib/completion/git-completion.bash |   3 +
>>  git-submodule.sh   |  13 ++-
>>  submodule-config.c |   8 ++
>>  submodule-config.h |   1 +
>>  t/t5612-clone-refspec.sh   | 103 
>> +---
>>  ...odules.sh => t5614-clone-submodules-shallow.sh} |   0
>>  t/t5616-clone-submodules-tags.sh   | 106 
>> +
>>  11 files changed, 284 insertions(+), 19 deletions(-)
>>  rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} 
>> (100%)
>>  create mode 100755 t/t5616-clone-submodules-tags.sh
>>
>> --
>> 2.11.0
>>
>
> --
> Brandon Williams

Re: [PATCH v3 0/5] clone: --no-tags option

[Brandon Williams]

On 04/26, Ævar Arnfjörð Bjarmason wrote:
> This is an expansion of the previously solo 02/05 "clone: add a
> --no-tags option to clone without tags" patch (see
> <20170418191553.15464-1-ava...@gmail.com>).
> 
> This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a
> lot), and in addition implements a --no-tags-submodules option. That
> code was implemented by Brandon & sent to me privately after I'd
> failed to come up with it, but I added tests, a commit message & bash
> completion to it.

Na you would have come up with it, I've just lived in submodule land a
little too long (though not as long as Stephan has!) :D

> The WIP 5/5 patch implements a submodule.NAME.tags config facility for
> the option, but is broken currently & floats along in this submission
> as an RFC patch. AFAICT it *should* work and it goes through all the
> motions the similar existing *.shallow config does, but for some
> reason the tags=false option isn't picked up & propagated in a freshly
> cloned submodule.
> 
> I'm probably missing something trivial, but I can't see what it is,
> I'm hoping thath either Stefan or Brandon will see what that is.

Overall the series looks good.  I've mentioned in the other threads that
it probably makes more sense to have --recurse-submodules simply pass
through known good commands to its children (e.g. --no-tags) simply
because it makes the UX a little bit easier to work with (I don't have
to remember all the fancy --OPT-submodules stuff, only
--recurse-submodules).  That is unless you have some good rational that
I'm not considering (completely possible :D).

> 
> Brandon Williams (1):
>   clone: add a --no-tags-submodules to pass --no-tags to submodules
> 
> Ævar Arnfjörð Bjarmason (4):
>   tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>   clone: add a --no-tags option to clone without tags
>   tests: rename a test having to do with shallow submodules
>   WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config
> 
>  Documentation/git-clone.txt|  21 
>  Documentation/git-submodule.txt|   8 +-
>  builtin/clone.c|  19 +++-
>  builtin/submodule--helper.c|  21 +++-
>  contrib/completion/git-completion.bash |   3 +
>  git-submodule.sh   |  13 ++-
>  submodule-config.c |   8 ++
>  submodule-config.h |   1 +
>  t/t5612-clone-refspec.sh   | 103 +---
>  ...odules.sh => t5614-clone-submodules-shallow.sh} |   0
>  t/t5616-clone-submodules-tags.sh   | 106 
> +
>  11 files changed, 284 insertions(+), 19 deletions(-)
>  rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} 
> (100%)
>  create mode 100755 t/t5616-clone-submodules-tags.sh
> 
> -- 
> 2.11.0
> 

-- 
Brandon Williams