guix git-fetch doesn't specify "--depth 1" - git clone clones a lot without any use

Done

Details

4 participants

Danny Milosavljevic
Leo Famulari
Ludovic Courtès
Marius Bakke

Owner: unassigned

Submitted by: Danny Milosavljevic

Severity: normal

Debbugs page

Danny Milosavljevic wrote 7 years ago

Recipients:(address . bug-guix@gnu.org)

Message-ID:20180212011641.3deb7fd6@scratchpost.org

git-fetch doesn't allow specifying "--depth 1".

That means the repo clones are needlessly large.

Since in packages we only need one specific commit anyhow why do we fetch

all the other commits?

Leo Famulari wrote 7 years ago

Recipients:(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 30428@debbugs.gnu.org)

Message-ID:20180212150939.GA5852@jasmine.lan

On Mon, Feb 12, 2018 at 01:16:41AM +0100, Danny Milosavljevic wrote:

Toggle quote (7 lines)

> git-fetch doesn't allow specifying "--depth 1".

> That means the repo clones are needlessly large.

> Since in packages we only need one specific commit anyhow why do we fetch

> all the other commits?

I think it's worth adding, but as an option, because there are Git

server implementations, like JGit, that don't support shallow cloning.

Danny Milosavljevic wrote 7 years ago

Recipients:(name . Leo Famulari)(address . leo@famulari.name)(address . 30428@debbugs.gnu.org)

Message-ID:20180212235950.4638aad6@scratchpost.org

Hi Leo,

On Mon, 12 Feb 2018 10:09:39 -0500

Leo Famulari <leo@famulari.name> wrote:

Toggle quote (3 lines)

> I think it's worth adding, but as an option, because there are Git

> server implementations, like JGit, that don't support shallow cloning.

Thanks for that! I didn't consider that before...

Possible patch (do you know such servers and can test whether they still work?):

Toggle diff (26 lines)diff --git a/guix/build/git.scm b/guix/build/git.scm
index c1af545a7..e54d92be7 100644
--- a/guix/build/git.scm
+++ b/guix/build/git.scm
@@ -37,12 +37,18 @@ recursively.  Return #t on success, #f otherwise."
   ;; in advance anyway.
   (setenv "GIT_SSL_NO_VERIFY" "true")
 
+  (mkdir-p directory)
+
   ;; We cannot use "git clone --recursive" since the following "git checkout"
   ;; effectively removes sub-module checkouts as of Git 2.6.3.
-  (and (zero? (system* git-command "clone" url directory))
+  (and ;(zero? (system* git-command "clone" url directory))
        (with-directory-excursion directory
-         (system* git-command "tag" "-l")
-         (and (zero? (system* git-command "checkout" commit))
+         ;(system* git-command "tag" "-l")
+         (invoke git-command "init")
+         (invoke git-command "remote" "add" "origin" url)
+         (and (or (zero? (system* git-command "fetch" "--depth" "1" "origin" commit))
+                  (zero? (system* git-command "fetch" "origin" commit)))
+              (zero? (system* git-command "checkout" "FETCH_HEAD"))
               (begin
                 (when recursive?
                   ;; Now is the time to fetch sub-modules.

Leo Famulari wrote 7 years ago

Recipients:(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 30428@debbugs.gnu.org)

Message-ID:7D9604D1-3DAE-4D13-AE73-420E655AC711@famulari.name

I think Google uses JGit for their public facing Git servers, but I'm not sure.

On February 12, 2018 5:59:50 PM EST, Danny Milosavljevic <dannym@scratchpost.org> wrote:

Toggle quote (43 lines)>Hi Leo,
>
>On Mon, 12 Feb 2018 10:09:39 -0500
>Leo Famulari <leo@famulari.name> wrote:
>
>> I think it's worth adding, but as an option, because there are Git
>> server implementations, like JGit, that don't support shallow
>cloning.
>
>Thanks for that!  I didn't consider that before...
>
>Possible patch (do you know such servers and can test whether they
>still work?):
>
>diff --git a/guix/build/git.scm b/guix/build/git.scm
>index c1af545a7..e54d92be7 100644
>--- a/guix/build/git.scm
>+++ b/guix/build/git.scm
>@@ -37,12 +37,18 @@ recursively.  Return #t on success, #f otherwise."
>   ;; in advance anyway.
>   (setenv "GIT_SSL_NO_VERIFY" "true")
> 
>+  (mkdir-p directory)
>+
>;; We cannot use "git clone --recursive" since the following "git
>checkout"
>   ;; effectively removes sub-module checkouts as of Git 2.6.3.
>-  (and (zero? (system* git-command "clone" url directory))
>+  (and ;(zero? (system* git-command "clone" url directory))
>        (with-directory-excursion directory
>-         (system* git-command "tag" "-l")
>-         (and (zero? (system* git-command "checkout" commit))
>+         ;(system* git-command "tag" "-l")
>+         (invoke git-command "init")
>+         (invoke git-command "remote" "add" "origin" url)
>+         (and (or (zero? (system* git-command "fetch" "--depth" "1"
>"origin" commit))
>+                  (zero? (system* git-command "fetch" "origin"
>commit)))
>+              (zero? (system* git-command "checkout" "FETCH_HEAD"))
>               (begin
>                 (when recursive?
>                   ;; Now is the time to fetch sub-modules.

Attachment: file

Leo Famulari wrote 7 years ago

Recipients:(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 30428@debbugs.gnu.org)

Message-ID:20180213142258.GA18012@jasmine.lan

On Mon, Feb 12, 2018 at 11:59:50PM +0100, Danny Milosavljevic wrote:

Toggle quote (9 lines)

> Leo Famulari <leo@famulari.name> wrote:

> > I think it's worth adding, but as an option, because there are Git

> > server implementations, like JGit, that don't support shallow cloning.

> Thanks for that! I didn't consider that before...

> Possible patch (do you know such servers and can test whether they still work?):

I think that Google's public-facing Git servers use JGit. So, perhaps

try shallow cloning Chromium and see if it works.

Marius Bakke wrote 7 years ago

Re: bug#30428: guix git-fetch doesn't specify "--depth 1" - git clone clones a lot without any use

Recipients:(address . 30428@debbugs.gnu.org)

Message-ID:5FB5D5D3-87BC-470E-8B89-0F572BFF623A@fastmail.com

On February 13, 2018 3:22:58 PM GMT+01:00, Leo Famulari <leo@famulari.name> wrote:

Toggle quote (15 lines)>On Mon, Feb 12, 2018 at 11:59:50PM +0100, Danny Milosavljevic wrote:
>> Leo Famulari <leo@famulari.name> wrote:
>> 
>> > I think it's worth adding, but as an option, because there are Git
>> > server implementations, like JGit, that don't support shallow
>cloning.
>> 
>> Thanks for that!  I didn't consider that before...
>> 
>> Possible patch (do you know such servers and can test whether they
>still work?):
>
>I think that Google's public-facing Git servers use JGit. So, perhaps
>try shallow cloning Chromium and see if it works.

"libvpx" in Guix uses Chromiums git infrastructure and is fairly small.

Attachment: file

Marius Bakke wrote 7 years ago

Recipients:(address . 30428@debbugs.gnu.org)

Message-ID:EADCB7A1-BAB9-449C-B7CD-6BF0194EB0F7@fastmail.com

On February 13, 2018 3:22:58 PM GMT+01:00, Leo Famulari <leo@famulari.name> wrote:

Toggle quote (15 lines)>On Mon, Feb 12, 2018 at 11:59:50PM +0100, Danny Milosavljevic wrote:
>> Leo Famulari <leo@famulari.name> wrote:
>> 
>> > I think it's worth adding, but as an option, because there are Git
>> > server implementations, like JGit, that don't support shallow
>cloning.
>> 
>> Thanks for that!  I didn't consider that before...
>> 
>> Possible patch (do you know such servers and can test whether they
>still work?):
>
>I think that Google's public-facing Git servers use JGit. So, perhaps
>try shallow cloning Chromium and see if it works.

"libvpx" in Guix uses Chromiums git infrastructure and is fairly small.

Leo Famulari wrote 7 years ago

Recipients:(name . Marius Bakke)(address . mbakke@fastmail.com)

Message-ID:20180213180806.GA1242@jasmine.lan

On Tue, Feb 13, 2018 at 06:08:58PM +0100, Marius Bakke wrote:

Toggle quote (20 lines)> 
> 
> On February 13, 2018 3:22:58 PM GMT+01:00, Leo Famulari <leo@famulari.name> wrote:
> >On Mon, Feb 12, 2018 at 11:59:50PM +0100, Danny Milosavljevic wrote:
> >> Leo Famulari <leo@famulari.name> wrote:
> >> 
> >> > I think it's worth adding, but as an option, because there are Git
> >> > server implementations, like JGit, that don't support shallow
> >cloning.
> >> 
> >> Thanks for that!  I didn't consider that before...
> >> 
> >> Possible patch (do you know such servers and can test whether they
> >still work?):
> >
> >I think that Google's public-facing Git servers use JGit. So, perhaps
> >try shallow cloning Chromium and see if it works.
> 
> "libvpx" in Guix uses Chromiums git infrastructure and is fairly small.

Looks like the Chromium and libvpx Git repos both support shallow

cloning now. That's great news for anyone building Chromium!

But it doesn't help find a Git server that doesn't support shallow

cloning.

Ludovic Courtès wrote 7 years ago

Re: bug#30428: guix git-fetch doesn't specify "--depth 1" - git clone clones a lot without any use

Recipients:(name . Danny Milosavljevic)(address . dannym@scratchpost.org)

Message-ID:87fu63sme8.fsf@gnu.org

Hello,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

Toggle quote (10 lines)> On Mon, 12 Feb 2018 10:09:39 -0500
> Leo Famulari <leo@famulari.name> wrote:
>
>> I think it's worth adding, but as an option, because there are Git
>> server implementations, like JGit, that don't support shallow cloning.
>
> Thanks for that!  I didn't consider that before...
>
> Possible patch (do you know such servers and can test whether they still work?):

I think it’s a great idea. FWIW, Andy proposed something along these

lines, but the idea was to use shallow clones for tags only because in

other cases it might not work (?):

https://lists.gnu.org/archive/html/guix-devel/2015-08/msg00258.html

It got stuck on a minor issue: the patch added a ‘tag’ field to

‘git-reference’, which changed the API, and I suggested making things

slightly differently:

https://lists.gnu.org/archive/html/guix-devel/2015-08/msg00558.html

Would you like to see if both patches could be merged?

Thanks,

Ludo’.

Danny Milosavljevic wrote 7 years ago

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:20180218143612.32fa5fc7@scratchpost.org

Hi Ludo,

On Wed, 14 Feb 2018 14:58:55 +0100

ludo@gnu.org (Ludovic Courtès) wrote:

Toggle quote (6 lines)

> I think it’s a great idea. FWIW, Andy proposed something along these

> lines, but the idea was to use shallow clones for tags only because in

> other cases it might not work (?):

> https://lists.gnu.org/archive/html/guix-devel/2015-08/msg00258.html

Yeah, some git servers either don't support searching by commit at all
or only support it after enabling it manually in the config file
(justification is some kind of privacy thing where accidentially a
private file could have been pushed to a public repo and then
reverted - with the commit hash you'd still get to it).
 
I'd rather not do the more involved patchset at the time being.
I don't understand git all that well.

It's already much nicer just to try the shallow commit checkout and fall back
to the current way if it doesn't work - and it's low risk.

Ludovic Courtès wrote 7 years ago

Recipients:(name . Danny Milosavljevic)(address . dannym@scratchpost.org)

Message-ID:877eraietu.fsf@gnu.org

Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

Toggle quote (3 lines)

> It's already much nicer just to try the shallow commit checkout and fall back

> to the current way if it doesn't work - and it's low risk.

Oh right, I hadn’t groked that this is what your patch does.

In that case I’m all for it, it seems to be low-risk indeed. Perhaps

leave a comment explaining that for some servers “--depth 1” won’t work

when we’re passing a commit and not a tag.

Thank you!

Ludo’.

Leo Famulari wrote 5 years ago

Recipients:(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 30428-done@debbugs.gnu.org)

Message-ID:20200322204810.GA19747@jasmine.lan

On Mon, Feb 12, 2018 at 01:16:41AM +0100, Danny Milosavljevic wrote:

Toggle quote (7 lines)

> git-fetch doesn't allow specifying "--depth 1".

> That means the repo clones are needlessly large.

> Since in packages we only need one specific commit anyhow why do we fetch

> all the other commits?

This was fixed in 329dabe13bf98b899b907b45565434c5140804f5. Closing.

Closed

Your comment

This issue is archived.

To comment on this conversation send an email to 30428@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it

mumi current 30428

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags

mumi command -t +confirmed -t +easy

Or, remove the moreinfo tag and set the help tag

mumi command -t -moreinfo -t +help

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date