bug-guix
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#43893: [PATCH] maint: update-guix-package: Optionally add sources to


From: Maxim Cournoyer
Subject: bug#43893: [PATCH] maint: update-guix-package: Optionally add sources to store.
Date: Tue, 10 Nov 2020 09:02:58 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

Hello Ludovic,

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

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

[...]

>> -(define-syntax-rule (with-temporary-git-worktree commit body ...)
>> -  "Execute BODY in the context of a temporary git worktree created from 
>> COMMIT."
>> +(define-syntax-rule (call-with-temporary-git-worktree commit proc)
>> +  "Execute PROC in the context of a temporary git worktree created from
>> +COMMIT.  PROC receives the temporary directory file name as an argument."
>
> This could be a procedure rather a macro now.

I've changed it to a plain define in the latest version (now merged).

> [...]
>
>> +       (call-with-temporary-git-worktree commit
>> +           (lambda (tmp-directory)
>> +             (let* ((hash (nix-base32-string->bytevector
>> +                           (string-trim-both
>> +                            (with-output-to-string
>> +                          (lambda ()
>> +                            (guix-hash "-rx" tmp-directory))))))
>> +                    (location (package-definition-location))
>> +                    (old-hash (content-hash-value
>> +                               (origin-hash (package-source guix)))))
>> +               (edit-expression location
>> +                                (update-definition commit hash
>> +                                                   #:old-hash old-hash
>> +                                                   #:version version))
>> +               ;; When GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT is set, the 
>> sources are
>> +               ;; added to the store.  This is used as part of 'make 
>> release'.
>> +               (when (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")
>> +                 (with-store store
>> +                   (keep-source-in-store store tmp-directory))))))))
>
> OK, that should do the job.
>
> Thanks for the patch, that should break the deadlock and allow us to
> proceed with the release!
>
> Next we need to update the ‘release’ target so
> GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT is set.

Done!

> I like that the initial issue is fixed, but I still don’t buy

[...]

Hehe.  Last round of weighing the + and - of this change

> the extra dependency on Git,

To me, this script (update-guix-package), is an extension of the
Make-based build system (that's currently it's sole entry-point).  There
are already calls to git in this build system (for example, to get the
commit corresponding to HEAD), so I don't perceive it as nasty in this
context.  It can also be used as a reminder of things that are missing
in (guile git), for the purists ;-).

> the extra copies of the whole tree,

There used to be 3 copies required in total (the Guix checkout, a first
dummy copy in the store, and a final copy in the store).

Now, we have 2 copies unless GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT is set,
in which case we get a third one in the store.  Seems pretty even to me!

> the extra code

The extra code make things smoother (better/faster diagnostic), causes
less friction in the workflow (I don't need to go paranoid about my tree
being in pristine condition before 'make update-guix-package' -- I can
rely on Guix computing it deterministically from the last commit).

> and the shell pipelines, something avoided in the rest of Guix.

Again, to me this script is a standalone extension of the build system.
It's not defined as a module, cannot be used in the rest of the code
base, so that it does things a bit differently doesn't strike me as bad.

> Perhaps that suggests there are unwritten coding guidelines we should
> eventually discuss and write.  We’ll see!

That could be nice.  Although a linter included with Guile (ala Rust or
Go) and configurable through a config file could have even more impact,
in my opinion.  In any case I'd be honored that my code got to be the
spark behind such guidelines/tool, eh :-).

Thank you,

Maxim





reply via email to

[Prev in Thread] Current Thread [Next in Thread]