[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#45891] [PATCH] packages: 'patch-and-repack' returns a directory whe
From: |
Ludovic Courtès |
Subject: |
[bug#45891] [PATCH] packages: 'patch-and-repack' returns a directory when given a directory. |
Date: |
Sat, 16 Jan 2021 22:28:54 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
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’.