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

["Ludovic Courtès" <ludo@gnu.org>]

Hi Maxim,

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

> Ludovic Courtès <ludo@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’.