mbox series

[0/2] clone: shallow-submodules should be single-branch by default

Message ID pull.1740.git.git.1719947271.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series clone: shallow-submodules should be single-branch by default | expand

Message

Philippe Blain via GitGitGadget July 2, 2024, 7:07 p.m. UTC
I noticed a couple places where the behavior of recursive clones for shallow
submodules does not match what is implied by the documentation. Shallow
submodules should be, but aren't, single branch by default. It would also be
useful to allow users to override the shallow specification in gitmodules on
the command line for clones as they can for submodule update. The
modification here for the former is a bit ugly, but hopefully at least gets
the point across to start a discussion. First time submitting a patch here,
hopefully I'm getting the process right.

Bruce Perry (2):
  clone: shallow-submodules should be single-branch by default
  clone: no-shallow-submodules clone overrides option in gitmodules

 Documentation/git-clone.txt         |  3 ++
 Documentation/gitmodules.txt        |  4 +--
 builtin/clone.c                     | 10 ++++--
 t/t5614-clone-submodules-shallow.sh | 52 +++++++++++++++++++++++------
 4 files changed, 53 insertions(+), 16 deletions(-)


base-commit: daed0c68e94967bfbb3f87e15f7c9090dc1aa1e1
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1740%2Fbaperry2%2Fsubmods-clone-bug-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1740/baperry2/submods-clone-bug-v1
Pull-Request: https://github.com/git/git/pull/1740

Comments

Randall S. Becker July 2, 2024, 7:12 p.m. UTC | #1
On Tuesday, July 2, 2024 3:08 PM, Bruce Perry wrote:
>I noticed a couple places where the behavior of recursive clones for shallow
>submodules does not match what is implied by the documentation. Shallow
>submodules should be, but aren't, single branch by default. It would also be useful
>to allow users to override the shallow specification in gitmodules on the command
>line for clones as they can for submodule update. The modification here for the
>former is a bit ugly, but hopefully at least gets the point across to start a discussion.
>First time submitting a patch here, hopefully I'm getting the process right.
>
>Bruce Perry (2):
>  clone: shallow-submodules should be single-branch by default
>  clone: no-shallow-submodules clone overrides option in gitmodules
>
> Documentation/git-clone.txt         |  3 ++
> Documentation/gitmodules.txt        |  4 +--
> builtin/clone.c                     | 10 ++++--
> t/t5614-clone-submodules-shallow.sh | 52 +++++++++++++++++++++++------
> 4 files changed, 53 insertions(+), 16 deletions(-)
>
>
>base-commit: daed0c68e94967bfbb3f87e15f7c9090dc1aa1e1
>Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-
>1740%2Fbaperry2%2Fsubmods-clone-bug-v1
>Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-
>1740/baperry2/submods-clone-bug-v1
>Pull-Request: https://github.com/git/git/pull/1740

I am concerned about this one. Many CI systems (including Jenkins GitSCM) assume a detached head for submodule clone/checkout. Adding a branch to the mix will change the expected semantics. Am I missing something?
--Randall
Bruce Perry July 3, 2024, 5 a.m. UTC | #2
Randall,

Perhaps I was using imprecise terminology. This change should not
impact whether submodule clones land in a detached head state, so it
should not impact anything that assumes submodule clones are detached
head.

The change being made is this: "git clone --recurse-submodules
--shallow-submodules" currently gives you a submodule with a detached
head at the desired state, but also downloads data for the tips of all
branches in the remote being cloned (potentially a lot of unneeded
data as in my use case). The modification means the same command would
give you a detached head at the desired state, plus the tip of only
the default branch in the remote. The modified behavior matches the
current behavior for a simple "git clone" followed by "git submodule
update --init --recurse--submodules --depth=1".

Thanks,
Bruce

(Resent due to a formatting failure)

On Tue, Jul 2, 2024, 1:12 PM <rsbecker@nexbridge.com> wrote:
>
> On Tuesday, July 2, 2024 3:08 PM, Bruce Perry wrote:
> >I noticed a couple places where the behavior of recursive clones for shallow
> >submodules does not match what is implied by the documentation. Shallow
> >submodules should be, but aren't, single branch by default. It would also be useful
> >to allow users to override the shallow specification in gitmodules on the command
> >line for clones as they can for submodule update. The modification here for the
> >former is a bit ugly, but hopefully at least gets the point across to start a discussion.
> >First time submitting a patch here, hopefully I'm getting the process right.
> >
> >Bruce Perry (2):
> >  clone: shallow-submodules should be single-branch by default
> >  clone: no-shallow-submodules clone overrides option in gitmodules
> >
> > Documentation/git-clone.txt         |  3 ++
> > Documentation/gitmodules.txt        |  4 +--
> > builtin/clone.c                     | 10 ++++--
> > t/t5614-clone-submodules-shallow.sh | 52 +++++++++++++++++++++++------
> > 4 files changed, 53 insertions(+), 16 deletions(-)
> >
> >
> >base-commit: daed0c68e94967bfbb3f87e15f7c9090dc1aa1e1
> >Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-
> >1740%2Fbaperry2%2Fsubmods-clone-bug-v1
> >Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-
> >1740/baperry2/submods-clone-bug-v1
> >Pull-Request: https://github.com/git/git/pull/1740
>
> I am concerned about this one. Many CI systems (including Jenkins GitSCM) assume a detached head for submodule clone/checkout. Adding a branch to the mix will change the expected semantics. Am I missing something?
> --Randall
>
Randall S. Becker July 3, 2024, 2:05 p.m. UTC | #3
On Wednesday, July 3, 2024 1:00 AM, Bruce Perry wrote:
>Perhaps I was using imprecise terminology. This change should not impact whether
>submodule clones land in a detached head state, so it should not impact anything
>that assumes submodule clones are detached head.
>
>The change being made is this: "git clone --recurse-submodules --shallow-
>submodules" currently gives you a submodule with a detached head at the desired
>state, but also downloads data for the tips of all branches in the remote being
>cloned (potentially a lot of unneeded data as in my use case). The modification
>means the same command would give you a detached head at the desired state,
>plus the tip of only the default branch in the remote. The modified behavior
>matches the current behavior for a simple "git clone" followed by "git submodule
>update --init --recurse--submodules --depth=1".
>
>Thanks,
>Bruce
>
>(Resent due to a formatting failure)
>
>On Tue, Jul 2, 2024, 1:12 PM <rsbecker@nexbridge.com> wrote:
>>
>> On Tuesday, July 2, 2024 3:08 PM, Bruce Perry wrote:
>> >I noticed a couple places where the behavior of recursive clones for
>> >shallow submodules does not match what is implied by the
>> >documentation. Shallow submodules should be, but aren't, single
>> >branch by default. It would also be useful to allow users to override
>> >the shallow specification in gitmodules on the command line for
>> >clones as they can for submodule update. The modification here for the former is
>a bit ugly, but hopefully at least gets the point across to start a discussion.
>> >First time submitting a patch here, hopefully I'm getting the process right.
>> >
>> >Bruce Perry (2):
>> >  clone: shallow-submodules should be single-branch by default
>> >  clone: no-shallow-submodules clone overrides option in gitmodules
>> >
>> > Documentation/git-clone.txt         |  3 ++
>> > Documentation/gitmodules.txt        |  4 +--
>> > builtin/clone.c                     | 10 ++++--
>> > t/t5614-clone-submodules-shallow.sh | 52
>> > +++++++++++++++++++++++------
>> > 4 files changed, 53 insertions(+), 16 deletions(-)
>> >
>> >
>> >base-commit: daed0c68e94967bfbb3f87e15f7c9090dc1aa1e1
>> >Published-As:
>> >https://github.com/gitgitgadget/git/releases/tag/pr-git-
>> >1740%2Fbaperry2%2Fsubmods-clone-bug-v1
>> >Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-
>> >1740/baperry2/submods-clone-bug-v1
>> >Pull-Request: https://github.com/git/git/pull/1740
>>
>> I am concerned about this one. Many CI systems (including Jenkins GitSCM)
>assume a detached head for submodule clone/checkout. Adding a branch to the mix
>will change the expected semantics. Am I missing something?

Thanks for clearing that up. Fine with me.
Regards
Randall
Bruce Perry July 25, 2024, 5:10 p.m. UTC | #4
Any further comments on this or changes I should make so it can be merged?

The current code behavior does not match the documentation, so
something should be changed. If these changes are not desired, I could
submit an alternative patch that changes the documentation to clarify
what currently happens instead.


On Wed, Jul 3, 2024 at 8:05 AM <rsbecker@nexbridge.com> wrote:
>
> On Wednesday, July 3, 2024 1:00 AM, Bruce Perry wrote:
> >Perhaps I was using imprecise terminology. This change should not impact whether
> >submodule clones land in a detached head state, so it should not impact anything
> >that assumes submodule clones are detached head.
> >
> >The change being made is this: "git clone --recurse-submodules --shallow-
> >submodules" currently gives you a submodule with a detached head at the desired
> >state, but also downloads data for the tips of all branches in the remote being
> >cloned (potentially a lot of unneeded data as in my use case). The modification
> >means the same command would give you a detached head at the desired state,
> >plus the tip of only the default branch in the remote. The modified behavior
> >matches the current behavior for a simple "git clone" followed by "git submodule
> >update --init --recurse--submodules --depth=1".
> >
> >Thanks,
> >Bruce
> >
> >(Resent due to a formatting failure)
> >
> >On Tue, Jul 2, 2024, 1:12 PM <rsbecker@nexbridge.com> wrote:
> >>
> >> On Tuesday, July 2, 2024 3:08 PM, Bruce Perry wrote:
> >> >I noticed a couple places where the behavior of recursive clones for
> >> >shallow submodules does not match what is implied by the
> >> >documentation. Shallow submodules should be, but aren't, single
> >> >branch by default. It would also be useful to allow users to override
> >> >the shallow specification in gitmodules on the command line for
> >> >clones as they can for submodule update. The modification here for the former is
> >a bit ugly, but hopefully at least gets the point across to start a discussion.
> >> >First time submitting a patch here, hopefully I'm getting the process right.
> >> >
> >> >Bruce Perry (2):
> >> >  clone: shallow-submodules should be single-branch by default
> >> >  clone: no-shallow-submodules clone overrides option in gitmodules
> >> >
> >> > Documentation/git-clone.txt         |  3 ++
> >> > Documentation/gitmodules.txt        |  4 +--
> >> > builtin/clone.c                     | 10 ++++--
> >> > t/t5614-clone-submodules-shallow.sh | 52
> >> > +++++++++++++++++++++++------
> >> > 4 files changed, 53 insertions(+), 16 deletions(-)
> >> >
> >> >
> >> >base-commit: daed0c68e94967bfbb3f87e15f7c9090dc1aa1e1
> >> >Published-As:
> >> >https://github.com/gitgitgadget/git/releases/tag/pr-git-
> >> >1740%2Fbaperry2%2Fsubmods-clone-bug-v1
> >> >Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-
> >> >1740/baperry2/submods-clone-bug-v1
> >> >Pull-Request: https://github.com/git/git/pull/1740
> >>
> >> I am concerned about this one. Many CI systems (including Jenkins GitSCM)
> >assume a detached head for submodule clone/checkout. Adding a branch to the mix
> >will change the expected semantics. Am I missing something?
>
> Thanks for clearing that up. Fine with me.
> Regards
> Randall
>