Subject: Re: [PATCH v4] guix: download: Add support for git repositories.

From: Romain Garbage <[email-masked]>
Date: Mon, 22 Jan 2024 11:39:51 +0100 (CET)

Hello,

----- Mail original -----
> De: "Maxim Cournoyer" <[email-masked]>
> À: "Romain Garbage" <[email-masked]>
> Cc: "68405" <[email-masked]>, "Ludovic Courtès" <[email-masked]>
> Envoyé: Samedi 20 Janvier 2024 03:23:01
> Objet: Re: [PATCH v4] guix: download: Add support for git repositories.

[...]

>> +    (with-store store
>> +      ;; TODO: Verify certificate support and deactivation.
>> +      (with-git-error-handling
>> +       (latest-repository-commit store url #:recursive? recursive? #:ref
>> reference)))))
> 
> The above contains too long lines still :-).

Sorry, I missed them, I sent a v5 hopefully wrapping all the lines longer than 80 characters :)

[...]
 
> Otherwise, I've tested it with:
> 
> --8<---------------cut here---------------start------------->8---
> ./pre-inst-env guix download -gr
> https://git.jami.net/savoirfairelinux/jami-client-qt -o /tmp/jami
> --8<---------------cut here---------------end--------------->8---
> 
> and it worked as advertised; very nice!
> 
> Some idea for the future: the --recurse option could take an optional
> argument that'd be a comma-separated list of submodules to fetch, e.g.
> 
> --8<---------------cut here---------------start------------->8---
> ./pre-inst-env guix download --git \
>  --recurse=daemon,3rdparty/SortFilterProxyModel \
>  https://git.jami.net/savoirfairelinux/jami-client-qt -o /tmp/jami
> --8<---------------cut here---------------end--------------->8---
> 
> and it'd recurse *only* the listed submodules.  This would be useful as
> some projects contain submodules for windows or other platforms we do
> not care about and they may be very large (heavy) to download.
> 
> The same idea could be implemented for our git-reference, where
> recursive? could accept a git submodule names list.

That's actually a nice idea. I will see if I can find some time to add it to a later patch.

> But back to the current scope:
> 
> Reviewed-by: Maxim Cournoyer <maxim.cournoyer@gmail>

Thank you for your review !

-- 
Romain