[bug#66436] [PATCH 2/2] git-download: Add support for Git Large File Storage (LFS).

[Maxim Cournoyer]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> * guix/git-download.scm (<git-reference>) [lfs?]: New field.
>> (git-fetch/in-band): New #:git-lfs argument.
>> <inputs>: Remove labels.  Conditionally add git-lfs.
>> <build>: Read "git lfs?" environment
>> variable and pass its value to the #:lfs? argument of 
>> git-fetch-with-fallback.
>> Use INPUTS directly; update comment.
>> <gexp->derivation>: Add "git lfs?" to #:env-vars.
>> (git-fetch/built-in): Add "lfs?" to #:env-vars.
>> * guix/build/git.scm (git-fetch) [lfs?]: New argument, doc and setup code.
>> (git-fetch-with-fallback) [lfs?]: New argument.  Pass it to git-fetch.
>> * guix/scripts/perform-download.scm (perform-git-download): Honor the 'lfs?'
>> environment variable.
>> * doc/guix.texi (origin Reference) <git-reference>: Document the new 'lfs?'
>> field.
>>  (Requirements): Mention the optional 'git-lfs' dependency.
>> * configure.ac: Add a check for the 'git-lfs' command.
>> * guix/config.scm.in (%git-lfs): New variable.
>> * guix/self.scm (%packages): Add git-lfs.
>> (compiled-guix): Add git-lfs to guix-config.
>> (make-config.scm): New #:git-lfs argument.
>
> I wonder whether this is a desirable feature, in the sense that the
> store is not designed to hold large amounts of data: just like Git has
> Git-LFS and Git-Annex, we’d need Guix-Annex (in fact, the GWL is kinda
> doing that already!).

Interesting thoughts, though in the case of running the 'orcus' package
test suite, the "large" files are simply .ods files (Open Document
spreadsheet).  It seems LFS is useful to store binary data files of any
kind, notwithstanding their size.

> Furthermore…
>
>> +dnl Git Large File Storage is an optional dependency for the
>> +dnl "builtin:git-download" derivation builder.
>> +AC_PATH_PROG([GIT_LFS], [git-lfs])
>> +if test "x$GIT_LFS" = "x"; then
>> +  AC_MSG_WARN([Git Large File Storage (git-lfs) is missing;
>> +  The builtin:git-download derivation builder of the Guix daemon will
>> +  not be able to use it.])
>
> … I don’t think we want to spend more words on the effect of increasing
> the closure size and getting locked with Git (libgit2 doesn’t implement
> LFS I suppose.)  To me this part is a showstopper; we should just make
> it clear that “builtin:git-download” does not implement LFS.

I thought we already did get locked with having git as a hard
dependency.  I guess you are keeping some hope for that to be reversed
in the future, pending new libgit2 developments such as gaining garbage
collection (git gc) support?  git-lfs would increase the closure of guix
from 688 to 700 MiB.

> Also, it is crucial for the “builtin:git-download” semantics to be the
> same across all installations and to be very stable.
>
>> +(define* (make-config.scm #:key gzip xz bzip2 git git-lfs
>>                            (package-name "GNU Guix")
>>                            (package-version "0")
>>                            (channel-metadata #f)
>> @@ -1140,6 +1145,7 @@ (define* (make-config.scm #:key gzip xz bzip2 git
>>                                 %store-database-directory
>>                                 %config-directory
>>                                 %git
>> +                               %git-lfs
>
> This I’d like to avoid, too (for the size issue).
>
> Overall, I feel like LFS support, if needed, should be “on the side”,
> with a custom ‘git-fetch/lfs’ or something along these lines (just like
> we have ‘url-fetch/tarbomb’).
>
> We might still need to change (guix build git) to implement it, but I
> would make sure that “builtin:git-download” remains unaffected and never
> fetches LFS stuff, even if ‘git-lfs’ happens to be on $PATH or
> something.
>
> WDYT?

Your arguments make sense.  I'll try to see how git-fetch/lfs could be
implemented.

-- 
Thanks,
Maxim