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 |
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, 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 >
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
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 >