GNU bug report logs - #45891
[PATCH] packages: 'patch-and-repack' returns a directory when given a directory.

Previous Next

Full log

Message #11 received at 45891 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 45891 <at> debbugs.gnu.org
Subject: Re: bug#45891: [PATCH] packages: 'patch-and-repack' returns a
 directory when given a directory.
Date: Sat, 16 Jan 2021 22:28:54 +0100

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Previously, 'patch-and-repack' would always create a tar.xz archive as a
>> result, even if the input was a directory (a checkout).  This change
>> reduces gratuitous CPU and storage overhead.
>
> I like it!
>
> Note that on core-updates, xz compression is relatively fast on modern
> machines as it can do multi-threading.  About space the savings; could
> the 'temporary' pristine source be cleared from the store always?

No, it’s not possible—the GC will remove what’s unreachable when it
eventually runs.

>>    (define (tarxz-name file-name)
>>      ;; Return a '.tar.xz' file name based on FILE-NAME.
>> -    (let ((base (cond ((numeric-extension? file-name)
>> -                       original-file-name)
>> -                      ((checkout? file-name)
>> -                       (string-drop-right file-name 9))
>> -                      (else (file-sans-extension file-name)))))
>> +    (let ((base (if (numeric-extension? file-name)
>> +                    original-file-name
>> +                    (file-sans-extension file-name))))
>
> This is not new code, but I'm wondering what's the purpose of
> numeric-extension?

It’s for file names like “hello-2.10”, where you wouldn’t want to strip
“.10”.  (Such file names should be rare, but it’s not impossible.)

> What kind of files does it expect to catch?  Also, what happened to
> stripping the '-checkout' suffix that used to be done?  It doesn't
> seem like it will happen anymore.

Stripping “-checkout” is no longer necessary because for these we keep
the original name.

>> -    (let ((name (tarxz-name original-file-name)))
>> +    (let ((name (if (checkout? original-file-name)
>> +                    original-file-name
>> +                    (tarxz-name original-file-name))))
>>        (gexp->derivation name build
>>                          #:graft? #f
>>                          #:system system
>
> Was these cases (tar archive source derivation, directory source
> derivation) already covered by tests under tests/packages.cm?  How did
> you otherwise test it?  World rebuilding changes are not fun to test
> without unit tests.

In this case I rebuilt the world and tested ‘guix build -S’ on a
git-fetch package with a snippet, but I agree that’s super expensive (I
tested the handful of commits I recently pushed to ‘core-updates’ at the
same time.)

There are no unit tests specifically for this procedure, but I think
we’ll quickly find out if it doesn’t behave as intended.

WDYT?

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.