make update-guix-package produced an incorrect hash

> I'm guessing it has something to do with update-guix-package using git-predicate

> to add only git-known (but not necessarily committed) files to the store and then

> calculating the checksum of that--but the git-fetch for the guix package not

> necessarily doing the same.

> Then update-guix-package.scm does one worse and actively prevents guix from

> doing the checkout from git when building that "guix" package. That means the

> person invoking update-guix-package.scm can't notice even when the sha256 hash

> is definitely wrong--because guix will have the source for package "guix" in

> the store already (a faked entry added by update-guix-package.scm) and thus

> won't fetch it again.

>

> Also, doesn't this entire approach have a problem?

>

> If you make a commit into the git repo of guix in order to update the

> package "guix" to commit A, at that point you can't know what commit hash

> commit A will have (since you haven't committed it yet) and yet you have

> to know the commit hash of commit A in order to write it into the package

> definition of package "guix".

>

> That cannot work.

> The only way it works, more or less by accident is that,

>

> (1) At first, update-guix-package.scm does NOT update the "guix" package

> inside, and calculates the hash of the working copy (hash A).

> (2) Then, it updates the "guix" package inside to refer to hash A and to a

> USER-SPECIFIED COMMIT HASH (the latter is determined by the user via

> git rev-parse HEAD).

> (3) Then, it commits that changed working copy as commit B. Commit B is

> essentially not referred-to by anyone--it's just to make it to the

> git repository so guix pull can pick it up.

> That works only as long as there will be no reference to a nested-nested "guix"

> package, by the eventual user.

> @Maxim: I think this entire thing has to assume that

>

> git rev-parse HEAD

>

> (which it did at the very beginning of make update-guix-package) actually

> refers to a commit that is available on the guix git repository on savannah.

>

> That means as soon as you change anything (no matter what) (and not actually

> commit that) before invoking

>

> make update-guix-package

>

> the commit it refers to in the "guix" package will be one which cannot be

> resolved by users.

> Long story short, we should make "make update-guix-package" check for

> uncommitted changes in the working copy, and fail if any such exist[1].

> There are no downsides that I can see. Even building from local working

> copy still works then.

> Also, let's please document update-guix-package.

>#FIXME: This doesn't work (recursion?)

>./pre-inst-env guix build guix --with-git-url=guix=file://$PWD

> By using a fresh copy of the last commit, we ensure the computed hash is

> stable in the face of local edits. This change also computes the hash

> externally from the store, which allows to verify that the hashes are valid

> using, e.g.:

>

> #FIXME: This doesn't work (recursion?)

> ./pre-inst-env guix build guix --with-git-url=guix=file://$PWD

> * build-aux/update-guix-package.scm (git-add-worktree): New procedure.

> (main): Use it to checkout a clean copy of the used commit, and compute the

> hash from it. Print a user warning after completion.

> - ;; Re-add SOURCE to the store, but this time under the real name used

> - ;; in the 'origin'. This allows us to build the package without

> - ;; having to make a real checkout; thus, it also works when working

> - ;; on a private branch.

> + (format #t "Updated Guix to commit ~s. You must ensure this

> +commit hash exists in the public repository, else 'guix pull' will break.

> +Beware of 'git rebase'~%" commit))))))

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

>

> > #FIXME: This doesn't work (recursion?)

> > ./pre-inst-env guix build guix --with-git-url=guix=file://$PWD

>

> Works for me, please open a separate bug report. :-)

> Hi Maxim,

>

> hmm, git worktree can fail if the commit already is checked out somewhere (for

> example if you invoke make update-guix-package twice in a row), or if the user

> used git worktree on that repo for that commit for other purposes. That would

> mean that

>

> make update-guix-package

> would fail in weird undocumented ways again. Please please let's document

> stuff at least.

>

> Also, why not just fail when there's uncommitted stuff?

> This patch looks like it goes to quite some length to enable you to build a

> guix package of committed stuff only (which is NOT what your working copy is

> actually like). Is there a use case for that? Sounds weird to me.

> Even if there's a use case for that, please add a warning if there are

> uncommitted changes that are now not included in the "guix" package.

> Other than that, okay.

>

>>#FIXME: This doesn't work (recursion?)

>>./pre-inst-env guix build guix --with-git-url=guix=file://$PWD

>

> Why doesn't it work? That sounds like a big limitation--that basically means

> you can't test with local-only commits, you'd always have to push. We

> should find out why this doesn't work and fix it.

>> * build-aux/update-guix-package.scm (git-add-worktree): New procedure.

>> (main): Use it to checkout a clean copy of the used commit, and compute the

>> hash from it. Print a user warning after completion.

>

> I’m not quite enthusiastic about the tool creating a worktree behind my

> back.

>> - ;; Re-add SOURCE to the store, but this time under the real name used

>> - ;; in the 'origin'. This allows us to build the package without

>> - ;; having to make a real checkout; thus, it also works when working

>> - ;; on a private branch.

>

> So this preserves this possibility, right?

> I could also suggest running ‘guix build guix --check’.

>

> Honestly, I would simply add this last message; better yet, we could use

> Guile-Git to (1) check whether we’re on a dirty tree and stop right away

> if we are

> and (2) check whether the commit exists in the official Git

> repo and error out if it doesn’t, unless

> GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT is set.

> Hi Ludo,

> Doesn't work for me on x86_64, using a checkout of guix at commit 93d3cfec32bbbe1dfbe0be686b371973545b35b8.

>

> $ guix environment --pure guix --ad-hoc git guile-readline guile-json

> nano guile-zlib guile-lzlib bash -- ./pre-inst-env guix build guix

> --with-git-url=guix=file://$PWD

>

> I get:

>

> [...]

> updating checkout of 'file:///home/dannym/src/guix-master/guix'...

> retrieved commit 93d3cfec32bbbe1dfbe0be686b371973545b35b8

> [...]

> test-name: channel-instances->manifest

> location: /tmp/guix-build-guix-1.1.0-30.5918cb5.drv-0/source/tests/channels.scm:233

> source:

> + (test-assert

> + "channel-instances->manifest"

> actual-value: #f

> actual-error:

> + (wrong-type-arg

> + "struct-vtable"

> + "Wrong type argument in position 1 (expecting struct): ~S"

> + (#f)

> + (#f))

> result: FAIL

> [...]

> command "make" "check" failed with status 2

> builder for `/gnu/store/csagsyh01rq7ilqkcpaa2d7vp1bn41w3-guix-1.1.0-30.5918cb5.drv' failed with exit code 1

> build of /gnu/store/csagsyh01rq7ilqkcpaa2d7vp1bn41w3-guix-1.1.0-30.5918cb5.drv failed

> View build log at '/var/log/guix/drvs/cs/agsyh01rq7ilqkcpaa2d7vp1bn41w3-guix-1.1.0-30.5918cb5.drv.bz2'.

> guix build: error: build of `/gnu/store/csagsyh01rq7ilqkcpaa2d7vp1bn41w3-guix-1.1.0-30.5918cb5.drv' failed

>

> I didn't even RUN update-guix-package or apply this patch--and the above still fails.

>

> This is reproducible every time--both on my laptop and on a x86_64 build host.

>

> So I'm confused how there's a substitute for the package "guix" available. How was it built? O_o.

> Doesn't work for me on x86_64, using a checkout of guix at commit 93d3cfec32bbbe1dfbe0be686b371973545b35b8.

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

>

> [...]

>

>>> * build-aux/update-guix-package.scm (git-add-worktree): New procedure.

>>> (main): Use it to checkout a clean copy of the used commit, and compute the

>>> hash from it. Print a user warning after completion.

>>

>> I’m not quite enthusiastic about the tool creating a worktree behind my

>> back.

>

> May I ask why? It's not something you'd (need to) be aware of (it

> doesn't leave traces in my v2 as 'git worktree prune' clears up the

> temporary worktree entries), and it's quite cheap to create.

> Danny Milosavljevic <dannym@scratchpost.org> skribis:

>

> > Doesn't work for me on x86_64, using a checkout of guix at commit 93d3cfec32bbbe1dfbe0be686b371973545b35b8.

>

> Oh I see, you’re hitting a test failure; I thought you were saying that

> ‘--with-git-url’ didn’t work.

> Hi,

>

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

>

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

>>

>> [...]

>>

>>>> * build-aux/update-guix-package.scm (git-add-worktree): New procedure.

>>>> (main): Use it to checkout a clean copy of the used commit, and compute the

>>>> hash from it. Print a user warning after completion.

>>>

>>> I’m not quite enthusiastic about the tool creating a worktree behind my

>>> back.

>>

>> May I ask why? It's not something you'd (need to) be aware of (it

>> doesn't leave traces in my v2 as 'git worktree prune' clears up the

>> temporary worktree entries), and it's quite cheap to create.

>

> To me, this is all my workspace, and I generally assume I’m the only one

> touching it; it’s more about the mental model, I guess.

> Fixes https://issues.guix.gnu.org/43893.

>

> This changes the 'update-guix-package' tool so that it:

>

> 1. Always uses a clean checkout to compute the hash of the updated 'guix'

> package.

> 2. Ensures the commit used in the updated 'guix' package definition has already

> been pushed upstream.

>

> * build-aux/update-guix-package.scm (%savannah-guix-git-repo-push-url): New

> variable.

> (with-input-pipe-to-string): New syntax.

> (find-origin-remote, git-add-worktree): New procedures.

> (commit-already-pushed?): New predicate.

> (main): Check the commit used has already been pushed upstream and compute the

> hash from a clean checkout.

> * doc/contributing.texi (Updating the Guix Package): Document it.

> (define %top-srcdir

> (string-append (current-source-directory) "/.."))

> @@ -101,44 +109,69 @@ COMMIT."

> (exp

> (error "'guix' package definition is not as expected" exp)))))

>

> -

> -(define (main . args)

> - (match args

> - ((commit version)

> - (with-store store

> - (let* ((source (add-to-store store

> - "guix-checkout" ;dummy name

> - #t "sha256" %top-srcdir

> - #:select? version-controlled?))

> - (hash (query-path-hash store source))

> - (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))

> +(define (git-add-worktree directory commit-ish)

> + "Create a new git worktree at DIRECTORY, detached on commit COMMIT-ISH."

> + (invoke "git" "worktree" "add" "--detach" directory commit-ish))

> +(define %savannah-guix-git-repo-push-url

> + "git.savannah.gnu.org/srv/git/guix.git")

>

> - ;; Re-add SOURCE to the store, but this time under the real name used

> - ;; in the 'origin'. This allows us to build the package without

> - ;; having to make a real checkout; thus, it also works when working

> - ;; on a private branch.

> - (reload-module

> - (resolve-module '(gnu packages package-management)))

> +(define-syntax-rule (with-input-pipe-to-string prog arg ...)

> + (let* ((input-pipe (open-pipe* OPEN_READ prog arg ...))

> + (output (get-string-all input-pipe))

> + (exit-val (status:exit-val (close-pipe input-pipe))))

> + (unless (zero? exit-val)

> + (error (format #f "Command ~s exited with non-zero exit status: ~s"

> + (string-join (list prog arg ...)) exit-val)))

> + (string-trim-both output)))

>

> - (let* ((source (add-to-store store

> - (origin-file-name (package-source guix))

> - #t "sha256" source))

> - (root (store-path-package-name source)))

> +(define (find-origin-remote)

> + "Find the name of the git remote with the Savannah Guix git repo URL."

> + (and-let* ((remotes (string-split (with-input-pipe-to-string

> + "git" "remote" "-v")

> + #\newline))

> + (origin-entry (find (cut string-contains <>

> + (string-append

> + %savannah-guix-git-repo-push-url

> + " (push)"))

> + remotes)))

> + (first (string-split origin-entry #\tab))))

>

> - ;; Add an indirect GC root for SOURCE in the current directory.

> - (false-if-exception (delete-file root))

> - (symlink source root)

> - (add-indirect-root store

> - (string-append (getcwd) "/" root))

> +(define (commit-already-pushed? remote commit)

> + "True if COMMIT is found in the REMOTE repository."

> + (not (string-null? (with-input-pipe-to-string

> + "git" "branch" "-r" "--contains" commit

> + (string-append remote "/master")))))

> - (format #t "source code for commit ~a: ~a (GC root: ~a)~%"

> - commit source root)))))

> +

> +(define (main . args)

> + (match args

> + ((commit version)

> + (with-directory-excursion %top-srcdir

> + (or (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")

> + (commit-already-pushed? (find-origin-remote) commit)

> + (leave (G_ "Commit ~a is not pushed upstream. Aborting.~%") commit))

> + (dynamic-wind

> + (lambda ()

> + #t)

> + (lambda ()

> + (call-with-temporary-directory

> + (lambda (tmp-directory)

> + (let* ((dummy (git-add-worktree tmp-directory commit))

> + (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))))))

> + (lambda ()

> + (invoke "git" "worktree" "prune")))))

>> +(define (git-add-worktree directory commit-ish)

>> + "Create a new git worktree at DIRECTORY, detached on commit COMMIT-ISH."

>> + (invoke "git" "worktree" "add" "--detach" directory commit-ish))

>

> Is it feasible to use Guile-Git here (given appropriate bindings)?

>> - (string-append (getcwd) "/" root))

>> +(define (commit-already-pushed? remote commit)

>> + "True if COMMIT is found in the REMOTE repository."

>> + (not (string-null? (with-input-pipe-to-string

>> + "git" "branch" "-r" "--contains" commit

>> + (string-append remote "/master")))))

>

> ...because parsing git CLI output is error-prone and "ugly" (IMO). But

> not a strong opinion.

>> + (lambda ()

>> + (invoke "git" "worktree" "prune")))))

>

> This is not great, because users (well, developers who run this script)

> may have worktrees that are temporarily inaccessible (e.g. on a USB

> drive or whatever). Better to just leave the stale reference instead of

> potentially destroying users worktrees.

> Perhaps the script could 'git clone --maxdepth=1' instead of creating

> a worktree?

> (define (main . args)

> (match args

> ((commit version)

> - (with-store store

> - (let* ((source (add-to-store store

> - "guix-checkout" ;dummy name

> - #t "sha256" %top-srcdir

> - #:select? version-controlled?))

> - (hash (query-path-hash store source))

> + (with-directory-excursion %top-srcdir

> + (or (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")

> + (commit-already-pushed? (find-origin-remote) commit)

> + (leave (G_ "Commit ~a is not pushed upstream. Aborting.~%") commit))

> + (let* ((hash (with-temporary-git-worktree commit

> + (nix-base32-string->bytevector

> + (string-trim-both

> + (with-output-to-string

> + (lambda ()

> + (guix-hash "-rx" ".")))))))

> (location (package-definition-location))

> (old-hash (content-hash-value

> - (origin-hash (package-source guix)))))

> + (origin-hash (package-source guix)))))

> (edit-expression location

> (update-definition commit hash

> #:old-hash old-hash

> - #:version version))

> -

> - ;; Re-add SOURCE to the store, but this time under the real name used

> - ;; in the 'origin'. This allows us to build the package without

> - ;; having to make a real checkout; thus, it also works when working

> - ;; on a private branch.

> - (reload-module

> - (resolve-module '(gnu packages package-management)))

> -

> - (let* ((source (add-to-store store

> - (origin-file-name (package-source guix))

> - #t "sha256" source))

> - (root (store-path-package-name source)))

> -

> - ;; Add an indirect GC root for SOURCE in the current directory.

> - (false-if-exception (delete-file root))

> - (symlink source root)

> - (add-indirect-root store

> - (string-append (getcwd) "/" root))

> -

> - (format #t "source code for commit ~a: ~a (GC root: ~a)~%"

> - commit source root)))))

> Hi Maxim,

>

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

>

>> (define (main . args)

>> (match args

>> ((commit version)

>> - (with-store store

>> - (let* ((source (add-to-store store

>> - "guix-checkout" ;dummy name

>> - #t "sha256" %top-srcdir

>> - #:select? version-controlled?))

>> - (hash (query-path-hash store source))

>> + (with-directory-excursion %top-srcdir

>> + (or (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")

>> + (commit-already-pushed? (find-origin-remote) commit)

>> + (leave (G_ "Commit ~a is not pushed upstream. Aborting.~%") commit))

>> + (let* ((hash (with-temporary-git-worktree commit

>> + (nix-base32-string->bytevector

>> + (string-trim-both

>> + (with-output-to-string

>> + (lambda ()

>> + (guix-hash "-rx" ".")))))))

>> (location (package-definition-location))

>> (old-hash (content-hash-value

>> - (origin-hash (package-source guix)))))

>> + (origin-hash (package-source guix)))))

>> (edit-expression location

>> (update-definition commit hash

>> #:old-hash old-hash

>> - #:version version))

>> -

>> - ;; Re-add SOURCE to the store, but this time under the real name used

>> - ;; in the 'origin'. This allows us to build the package without

>> - ;; having to make a real checkout; thus, it also works when working

>> - ;; on a private branch.

>> - (reload-module

>> - (resolve-module '(gnu packages package-management)))

>> -

>> - (let* ((source (add-to-store store

>> - (origin-file-name (package-source guix))

>> - #t "sha256" source))

>> - (root (store-path-package-name source)))

>> -

>> - ;; Add an indirect GC root for SOURCE in the current directory.

>> - (false-if-exception (delete-file root))

>> - (symlink source root)

>> - (add-indirect-root store

>> - (string-append (getcwd) "/" root))

>> -

>> - (format #t "source code for commit ~a: ~a (GC root: ~a)~%"

>> - commit source root)))))

>

> I realize it was maybe enough to wrap this whole portion (starting from

> “Re-add SOURCE”) in (unless (getenv

> "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT") …)? Running ‘guix build guix’

> would have forced the source derivation to be built.

>

> Anyhow, thanks for working on it!

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

>>> - ;; Re-add SOURCE to the store, but this time under the real name used

>>> - ;; in the 'origin'. This allows us to build the package without

>>> - ;; having to make a real checkout; thus, it also works when working

>>> - ;; on a private branch.

>>> - (reload-module

>>> - (resolve-module '(gnu packages package-management)))

>>> -

>>> - (let* ((source (add-to-store store

>>> - (origin-file-name (package-source guix))

>>> - #t "sha256" source))

>>> - (root (store-path-package-name source)))

>>> -

>>> - ;; Add an indirect GC root for SOURCE in the current directory.

>>> - (false-if-exception (delete-file root))

>>> - (symlink source root)

>>> - (add-indirect-root store

>>> - (string-append (getcwd) "/" root))

>>> -

>>> - (format #t "source code for commit ~a: ~a (GC root: ~a)~%"

>>> - commit source root)))))

>>

>> I realize it was maybe enough to wrap this whole portion (starting from

>> “Re-add SOURCE”) in (unless (getenv

>> "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT") …)? Running ‘guix build guix’

>> would have forced the source derivation to be built.

>>

>> Anyhow, thanks for working on it!

>

> Sorry, I'm a bit lost. Are you suggesting that we should restore the

> code following the ;; Re-add SOURCE [...], but wrapped with unless to

> make it conditional to GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT being

> defined? That part depends on SOURCE, a store file name, which we don't

> have anymore since we no longer add the sources to the store to compute

> the hash.

> Hi,

>

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

>

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

>

>> Sorry, I'm a bit lost. Are you suggesting that we should restore the

>> code following the ;; Re-add SOURCE [...], but wrapped with unless to

>> make it conditional to GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT being

>> defined? That part depends on SOURCE, a store file name, which we don't

>> have anymore since we no longer add the sources to the store to compute

>> the hash.

>

> I’m saying that the solution to the initial problem (that if you don’t

> run ‘guix build guix -S --check’ you’re not sure ‘guix’ is referring to

> a valid upstream commit)

> above. The trick at “Re-add SOURCE” is here precisely to prevent

> attempts to re-download, as the comment explains, and this is what has

> been causing these troubles.

>

> The advantages (to me) would have been simpler code, no shelling out to

> ‘git’, and no fiddling with files under $PWD.

> BTW, in ‘make release’ does ‘make update-guix-package’ and expects it to

> work with a not-pushed-yet commit. So it’s a case where we need

> GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT=yes.

> The original problem was about the updated Guix package containing a

> faulty hash (due to being computed from a uncontrolled checkout that

> could be dirty). The other concern about preventing the use of a not

> yet published commit was added based on earlier feedback.

>

>> above. The trick at “Re-add SOURCE” is here precisely to prevent

>> attempts to re-download, as the comment explains, and this is what has

>> been causing these troubles.

>>

>> The advantages (to me) would have been simpler code, no shelling out to

>> ‘git’, and no fiddling with files under $PWD.

>

> Less code would also mean poorer diagnostics:

>

> time ./pre-inst-env guix build guix -S

> The following derivation will be built:

> /gnu/store/zhfchz831mncd2kyqmh5d2q0r2rpg57s-guix-1.1.0-31.07c13ae-checkout.drv

> building /gnu/store/zhfchz831mncd2kyqmh5d2q0r2rpg57s-guix-1.1.0-31.07c13ae-checkout.drv...

> guile: warning: failed to install locale

> environment variable `PATH' set to `/gnu/store/378zjf2kgajcfd7mfr98jn5xyc5wa3qv-gzip-1.10/bin:/gnu/store/sf3rbvb6iqcphgm1afbplcs72hsywg25-tar-1.32/bin'

> Initialized empty Git repository in /gnu/store/02da8jb3wzzi3bqvrl214gdg0kkxmaf8-guix-1.1.0-31.07c13ae-checkout/.git/

> error: Server does not allow request for unadvertised object 07c13aeb5abb1a5bc3cabffb9b2212993a0d5a0e

> Failed to do a shallow fetch; retrying a full fetch...

> From https://git.savannah.gnu.org/git/guix

> * [new branch] core-updates -> origin/core-updates

> [...]

> * [new tag] v1.1.0rc2 -> v1.1.0rc2

> fatal: reference is not a tree: 07c13aeb5abb1a5bc3cabffb9b2212993a0d5a0e

> git-fetch: '/gnu/store/i5b1vv7qc6l2gi4xwa9mqzjy3shvgk30-git-minimal-2.28.0/bin/git checkout 07c13aeb5abb1a5bc3cabffb9b2212993a0d5a0e' failed with exit code 128

> Trying content-addressed mirror at berlin.guix.gnu.org...

> Trying content-addressed mirror at berlin.guix.gnu.org...

> Trying to download from Software Heritage...

> builder for `/gnu/store/zhfchz831mncd2kyqmh5d2q0r2rpg57s-guix-1.1.0-31.07c13ae-checkout.drv' failed to produce output path `/gnu/store/02da8jb3wzzi3bqvrl214gdg0kkxmaf8-guix-1.1.0-31.07c13ae-checkout'

> build of /gnu/store/zhfchz831mncd2kyqmh5d2q0r2rpg57s-guix-1.1.0-31.07c13ae-checkout.drv failed

> View build log at '/var/log/guix/drvs/zh/fchz831mncd2kyqmh5d2q0r2rpg57s-guix-1.1.0-31.07c13ae-checkout.drv.bz2'.

> guix build: error: build of `/gnu/store/zhfchz831mncd2kyqmh5d2q0r2rpg57s-guix-1.1.0-31.07c13ae-checkout.drv' failed

>

> real 1m42.825s

> user 0m2.191s

> sys 0m0.189s

>

> This took nearly 2 minutes, fetching the full Guix git repo just to tell

> me that *something* is not right.

>

> Currently, we have:

>

> time make update-guix-package

> git rev-parse HEAD

> 4893a1394e2eb8b97995b491f2f37ed85513a20f

> ./pre-inst-env "/gnu/store/i7z4pfa0c22q0qkxyl7fy2nlp3w658yg-profile/bin/guile" \

> ./build-aux/update-guix-package.scm \

> "`git rev-parse HEAD`"

> error: Commit 4893a1394e2eb8b97995b491f2f37ed85513a20f is not pushed upstream. Aborting.

> make: *** [Makefile:6507: update-guix-package] Error 1

>> BTW, in ‘make release’ does ‘make update-guix-package’ and expects it to

>> work with a not-pushed-yet commit. So it’s a case where we need

>> GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT=yes.

>

> Ah, good point. I'd like to fix this,

> but running 'make release', it fails on:

>

> make[3]: *** No rule to make target 'po/doc/guix-manual.pot', needed by

> 'distdir-am'. Stop.

>

> What did I miss?

>> Currently, we have:

>>

>> time make update-guix-package

>> git rev-parse HEAD

>> 4893a1394e2eb8b97995b491f2f37ed85513a20f

>> ./pre-inst-env "/gnu/store/i7z4pfa0c22q0qkxyl7fy2nlp3w658yg-profile/bin/guile" \

>> ./build-aux/update-guix-package.scm \

>> "`git rev-parse HEAD`"

>> error: Commit 4893a1394e2eb8b97995b491f2f37ed85513a20f is not pushed upstream. Aborting.

>> make: *** [Makefile:6507: update-guix-package] Error 1

>

> I agree that the better diagnostic is nice. Though it’s a script that’s

> essentially for a handful of people, who can certainly cope with the

> ugly error.

>

> Anyway, I think we didn’t analyze the initial situation well enough

> (myself included, by not commenting early and accurately). I’m also not

> fond of the added complexity and the risk of surprises when we make the

> release, but OTOH, it’s no big deal in the big picture!

>>> BTW, in ‘make release’ does ‘make update-guix-package’ and expects it to

>>> work with a not-pushed-yet commit. So it’s a case where we need

>>> GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT=yes.

> +(define %savannah-guix-git-repo-push-url

> + "git.savannah.gnu.org/srv/git/guix.git")

> +(define (find-origin-remote)

> + "Find the name of the git remote with the Savannah Guix git repo URL."

> + (and-let* ((remotes (string-split (with-input-pipe-to-string

> + "git" "remote" "-v")

> + #\newline))

> + (origin-entry (find (cut string-contains <>

> + (string-append

> + %savannah-guix-git-repo-push-url

> + " (push)"))

> + remotes)))

> + (first (string-split origin-entry #\tab))))

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

>

> [...]

>

>>> Currently, we have:

>>>

>>> time make update-guix-package

>>> git rev-parse HEAD

>>> 4893a1394e2eb8b97995b491f2f37ed85513a20f

>>> ./pre-inst-env "/gnu/store/i7z4pfa0c22q0qkxyl7fy2nlp3w658yg-profile/bin/guile" \

>>> ./build-aux/update-guix-package.scm \

>>> "`git rev-parse HEAD`"

>>> error: Commit 4893a1394e2eb8b97995b491f2f37ed85513a20f is not pushed upstream. Aborting.

>>> make: *** [Makefile:6507: update-guix-package] Error 1

>>

>> I agree that the better diagnostic is nice. Though it’s a script that’s

>> essentially for a handful of people, who can certainly cope with the

>> ugly error.

>>

>> Anyway, I think we didn’t analyze the initial situation well enough

>> (myself included, by not commenting early and accurately). I’m also not

>> fond of the added complexity and the risk of surprises when we make the

>> release, but OTOH, it’s no big deal in the big picture!

>

> I'm sorry but I don't agree with the "we didn't analyze the initial

> situation well enough";

> if I had to think about the best way to solve this problem now, I'd

> still choose the way that was chosen then, as it provides the best

> guarantee against producing a broken Guix package, something that

> happened a couple times in the past, judging from git log. About

> complexity, I'd much rather the tool break on me than breaking 'guix

> pull' for everyone :-).

>>>> BTW, in ‘make release’ does ‘make update-guix-package’ and expects it to

>>>> work with a not-pushed-yet commit. So it’s a case where we need

>>>> GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT=yes.

>

> I want to be able to run 'make release' first to test this works

> correctly, but even after rebuilding my source tree from scratch

> (following a 'make distclean'), and also attempting 'make download-po',

> and following release.org from guix-maintenance, I still get:

>

> make[3]: *** No rule to make target 'po/doc/guix-manual.pot', needed by 'distdir-am'. Stop.

> Hi,

>

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

>

>> +(define %savannah-guix-git-repo-push-url

>> + "git.savannah.gnu.org/srv/git/guix.git")

>

> [...]

>

>> +(define (find-origin-remote)

>> + "Find the name of the git remote with the Savannah Guix git repo URL."

>> + (and-let* ((remotes (string-split (with-input-pipe-to-string

>> + "git" "remote" "-v")

>> + #\newline))

>> + (origin-entry (find (cut string-contains <>

>> + (string-append

>> + %savannah-guix-git-repo-push-url

>> + " (push)"))

>> + remotes)))

>> + (first (string-split origin-entry #\tab))))

>

> I noticed that this returns #f for me because I’m using git.sv.gnu.org,

> not git.savannah.gnu.org.

> Hi again,

>

> How about this variant of the initial script? I think it addresses the

> main issues we discussed here:

>

> 1. By default it doesn’t re-add the source in the store, so wrong

> commit/hash issues are caught when running ‘guix build guix’.

>

> 2. It diagnoses dirty trees early on. It does not explicitly diagnose

> missing upstream commits though, but again they’re caught when

> running ‘guix build guix’.

>

> WDYT?

>

> Sorry for all the back-and-forth on what looks like a tiny issue. I do

> think we’re making progress anyway!

>

> Thanks,

> Ludo’

> @@ -23,12 +24,15 @@

> ;;;

> ;;; Code:

> -(use-modules (guix)

> +(use-modules (git)

> + (guix)

> (guix git-download)

> (guix upstream)

> (guix utils)

> (guix base32)

> (guix build utils)

> + (guix i18n)

> + (guix diagnostics)

> (gnu packages package-management)

> (ice-9 match))

> @@ -101,7 +105,43 @@ COMMIT."

> (exp

> (error "'guix' package definition is not as expected" exp)))))

> -

> +(define (keep-source-in-store store source)

> + "Add SOURCE to the store under the name that the 'guix' package expects."

> +

> + ;; Add SOURCE to the store, but this time under the real name used in the

> + ;; 'origin'. This allows us to build the package without having to make a

> + ;; real checkout; thus, it also works when working on a private branch.

> + (reload-module

> + (resolve-module '(gnu packages package-management)))

> +

> + (let* ((source (add-to-store store

> + (origin-file-name (package-source guix))

> + #t "sha256" source))

> + (root (store-path-package-name source)))

> +

> + ;; Add an indirect GC root for SOURCE in the current directory.

> + (false-if-exception (delete-file root))

> + (symlink source root)

> + (add-indirect-root store

> + (string-append (getcwd) "/" root))

> +

> + (info (G_ "source code kept in ~a (GC root: ~a)~%")

> + source root)))

> +

> +(define (assert-clean-checkout repository)

> + "Error out if the working directory at REPOSITORY contains local

> +modifications."

> + (define description

> + (let ((format-options (make-describe-format-options

> + #:dirty-suffix "-dirty")))

> + (describe-format (describe-workdir repository) format-options)))

> +

> + (when (string-suffix? "-dirty" description)

> + (leave (G_ "attempt to update 'guix' package from a dirty tree (~a)~%")

> + description))

> +

> + (info (G_ "updating 'guix' package to '~a'~%") description))

> (define (main . args)

> (match args

> ((commit version)

> @@ -113,32 +153,20 @@ COMMIT."

> (hash (query-path-hash store source))

> (location (package-definition-location))

> (old-hash (content-hash-value

> - (origin-hash (package-source guix)))))

> + (origin-hash (package-source guix)))))

> +

> + (unless (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")

> + (let ((repository (repository-open ".")))

> + (assert-clean-checkout repository)

> + (repository-close! repository)))

> +

> (edit-expression location

> (update-definition commit hash

> #:old-hash old-hash

> #:version version))

> - ;; Re-add SOURCE to the store, but this time under the real name used

> - ;; in the 'origin'. This allows us to build the package without

> - ;; having to make a real checkout; thus, it also works when working

> - ;; on a private branch.

> - (reload-module

> - (resolve-module '(gnu packages package-management)))

> -

> - (let* ((source (add-to-store store

> - (origin-file-name (package-source guix))

> - #t "sha256" source))

> - (root (store-path-package-name source)))

> -

> - ;; Add an indirect GC root for SOURCE in the current directory.

> - (false-if-exception (delete-file root))

> - (symlink source root)

> - (add-indirect-root store

> - (string-append (getcwd) "/" root))

> -

> - (format #t "source code for commit ~a: ~a (GC root: ~a)~%"

> - commit source root)))))

> + (when (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")

> + (keep-source-in-store store source)))))

>> +(define (assert-clean-checkout repository)

>> + "Error out if the working directory at REPOSITORY contains local

>> +modifications."

>> + (define description

>> + (let ((format-options (make-describe-format-options

>> + #:dirty-suffix "-dirty")))

>> + (describe-format (describe-workdir repository) format-options)))

>> +

>> + (when (string-suffix? "-dirty" description)

>> + (leave (G_ "attempt to update 'guix' package from a dirty tree (~a)~%")

>> + description))

>> +

>> + (info (G_ "updating 'guix' package to '~a'~%") description))

>

> Unfortunately this doesn't catch the case where git has explicitly been

> told to '--skip-worktree' on a path or file (the original cause of this

> bug report). See

> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=43893#11.

>> (define (main . args)

>> (match args

>> ((commit version)

>> @@ -113,32 +153,20 @@ COMMIT."

>> (hash (query-path-hash store source))

>> (location (package-definition-location))

>> (old-hash (content-hash-value

>> - (origin-hash (package-source guix)))))

>> + (origin-hash (package-source guix)))))

>> +

>> + (unless (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")

>> + (let ((repository (repository-open ".")))

>> + (assert-clean-checkout repository)

>> + (repository-close! repository)))

>> +

>> (edit-expression location

>> (update-definition commit hash

>> #:old-hash old-hash

>> #:version version))

>

>> - ;; Re-add SOURCE to the store, but this time under the real name used

>> - ;; in the 'origin'. This allows us to build the package without

>> - ;; having to make a real checkout; thus, it also works when working

>> - ;; on a private branch.

>> - (reload-module

>> - (resolve-module '(gnu packages package-management)))

>> -

>> - (let* ((source (add-to-store store

>> - (origin-file-name (package-source guix))

>> - #t "sha256" source))

>> - (root (store-path-package-name source)))

>> -

>> - ;; Add an indirect GC root for SOURCE in the current directory.

>> - (false-if-exception (delete-file root))

>> - (symlink source root)

>> - (add-indirect-root store

>> - (string-append (getcwd) "/" root))

>> -

>> - (format #t "source code for commit ~a: ~a (GC root: ~a)~%"

>> - commit source root)))))

>> + (when (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")

>> + (keep-source-in-store store source)))))

>

> That environment variable would now do more than it advertises. I'd

> prefer to keep the behavior consistent in both modes, unless there's a

> very good reason not too?

> Hi Maxim,

>

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

>

>>> +(define (assert-clean-checkout repository)

>>> + "Error out if the working directory at REPOSITORY contains local

>>> +modifications."

>>> + (define description

>>> + (let ((format-options (make-describe-format-options

>>> + #:dirty-suffix "-dirty")))

>>> + (describe-format (describe-workdir repository) format-options)))

>>> +

>>> + (when (string-suffix? "-dirty" description)

>>> + (leave (G_ "attempt to update 'guix' package from a dirty tree (~a)~%")

>>> + description))

>>> +

>>> + (info (G_ "updating 'guix' package to '~a'~%") description))

>>

>> Unfortunately this doesn't catch the case where git has explicitly been

>> told to '--skip-worktree' on a path or file (the original cause of this

>> bug report). See

>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=43893#11.

>

> Any such issue is caught when one eventually runs ‘guix build guix’

> (wrong commit ID, wrong hash, etc.).

>

> But you’re right that the above test isn’t fool-proof: it’s just a way

> to catch this common mistake early and report it nicely.

>>> (define (main . args)

>>> (match args

>>> ((commit version)

>>> @@ -113,32 +153,20 @@ COMMIT."

>>> (hash (query-path-hash store source))

>>> (location (package-definition-location))

>>> (old-hash (content-hash-value

>>> - (origin-hash (package-source guix)))))

>>> + (origin-hash (package-source guix)))))

>>> +

>>> + (unless (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")

>>> + (let ((repository (repository-open ".")))

>>> + (assert-clean-checkout repository)

>>> + (repository-close! repository)))

>>> +

>>> (edit-expression location

>>> (update-definition commit hash

>>> #:old-hash old-hash

>>> #:version version))

>>

>>> - ;; Re-add SOURCE to the store, but this time under the real name used

>>> - ;; in the 'origin'. This allows us to build the package without

>>> - ;; having to make a real checkout; thus, it also works when working

>>> - ;; on a private branch.

>>> - (reload-module

>>> - (resolve-module '(gnu packages package-management)))

>>> -

>>> - (let* ((source (add-to-store store

>>> - (origin-file-name (package-source guix))

>>> - #t "sha256" source))

>>> - (root (store-path-package-name source)))

>>> -

>>> - ;; Add an indirect GC root for SOURCE in the current directory.

>>> - (false-if-exception (delete-file root))

>>> - (symlink source root)

>>> - (add-indirect-root store

>>> - (string-append (getcwd) "/" root))

>>> -

>>> - (format #t "source code for commit ~a: ~a (GC root: ~a)~%"

>>> - commit source root)))))

>>> + (when (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT")

>>> + (keep-source-in-store store source)))))

>>

>> That environment variable would now do more than it advertises. I'd

>> prefer to keep the behavior consistent in both modes, unless there's a

>> very good reason not too?

>

> Adding the source to the store, under the right name, with a GC root, is

> a prerequisite for use cases like ‘make release’: there you not only

> want to update the package definition to refer to your private commit

> and corresponding hash, you also want to be able to build it. If the

> source isn’t already in the store, ‘guix build guix’ tries to look it up

> on Savannah, which fails.

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

>>

>>>> +(define (assert-clean-checkout repository)

>>>> + "Error out if the working directory at REPOSITORY contains local

>>>> +modifications."

>>>> + (define description

>>>> + (let ((format-options (make-describe-format-options

>>>> + #:dirty-suffix "-dirty")))

>>>> + (describe-format (describe-workdir repository) format-options)))

>>>> +

>>>> + (when (string-suffix? "-dirty" description)

>>>> + (leave (G_ "attempt to update 'guix' package from a dirty tree (~a)~%")

>>>> + description))

>>>> +

>>>> + (info (G_ "updating 'guix' package to '~a'~%") description))

>>>

>>> Unfortunately this doesn't catch the case where git has explicitly been

>>> told to '--skip-worktree' on a path or file (the original cause of this

>>> bug report). See

>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=43893#11.

>>

>> Any such issue is caught when one eventually runs ‘guix build guix’

>> (wrong commit ID, wrong hash, etc.).

>>

>> But you’re right that the above test isn’t fool-proof: it’s just a way

>> to catch this common mistake early and report it nicely.

>

> Right. I still don't like that it wouldn't work from a checkout where

> someone would have modified, e.g., the .dir-locals file locally and

> marked it with 'git --skip-worktree'. Having to revert this kind of

> 'developer setup' is painful. The current approach makes it unnecessary

> (only committed changes are taken into account, not just git tracked

> files).

> Following discussions in https://issues.guix.gnu.org/43893, keeping a copy

> of the updated package source is desirable when generating a release.

> * build-aux/update-guix-package.scm (version-controlled?): Remove variable.

> (call-with-temporary-git-worktree): Renamed from

> 'with-temporary-git-worktree'. Update doc. Do not change directory

> implicitly.

> (keep-source-in-store): New procedure.

> (main): Adjust to use with call-with-temporary-git-worktree. Add the sources

> to the store when GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT is set.

> * .dir-locals.el (scheme-mode): Update.

> * doc/contributing.texi (Updating the Guix Package): Update doc.

>

> Co-authored-by: Ludovic Courtès <ludo@gnu.org>

> -(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."

> + (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))))))))

> Following discussions in https://issues.guix.gnu.org/43893, keeping a copy

> of the updated package source is desirable when generating a release.

>

> * build-aux/update-guix-package.scm (version-controlled?): Remove variable.

> (call-with-temporary-git-worktree): Renamed from

> 'with-temporary-git-worktree'. Update doc. Do not change directory

> implicitly.

> (keep-source-in-store): New procedure.

> (main): Adjust to use with call-with-temporary-git-worktree. Add the sources

> to the store when GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT is set. Exit gracefully

> when FIND-ORIGIN-REMOTE returns #f.

> (%savannah-guix-git-repo-push-url-regexp): Adjust match for a potential colon

> separator.

> * Makefile.am (GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT): Adjust.

> * .dir-locals.el (scheme-mode): Update.

> * doc/contributing.texi (Updating the Guix Package): Update doc.

>

> Co-authored-by: Ludovic Courtès <ludo@gnu.org>

> 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.

> [...]

>

>> + (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.

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

> the extra dependency on Git,

> the extra copies of the whole tree,

> the extra code

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

> Perhaps that suggests there are unwritten coding guidelines we should

> eventually discuss and write. We’ll see!

> Hi!

>

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

>

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

>>>

>>>>> +(define (assert-clean-checkout repository)

>>>>> + "Error out if the working directory at REPOSITORY contains local

>>>>> +modifications."

>>>>> + (define description

>>>>> + (let ((format-options (make-describe-format-options

>>>>> + #:dirty-suffix "-dirty")))

>>>>> + (describe-format (describe-workdir repository) format-options)))

>>>>> +

>>>>> + (when (string-suffix? "-dirty" description)

>>>>> + (leave (G_ "attempt to update 'guix' package from a dirty tree (~a)~%")

>>>>> + description))

>>>>> +

>>>>> + (info (G_ "updating 'guix' package to '~a'~%") description))

>>>>

>>>> Unfortunately this doesn't catch the case where git has explicitly been

>>>> told to '--skip-worktree' on a path or file (the original cause of this

>>>> bug report). See

>>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=43893#11.

>>>

>>> Any such issue is caught when one eventually runs ‘guix build guix’

>>> (wrong commit ID, wrong hash, etc.).

>>>

>>> But you’re right that the above test isn’t fool-proof: it’s just a way

>>> to catch this common mistake early and report it nicely.

>>

>> Right. I still don't like that it wouldn't work from a checkout where

>> someone would have modified, e.g., the .dir-locals file locally and

>> marked it with 'git --skip-worktree'. Having to revert this kind of

>> 'developer setup' is painful. The current approach makes it unnecessary

>> (only committed changes are taken into account, not just git tracked

>> files).

>

> “Wouldn’t work” is a strong statement: like I wrote, mistakes would

> always be caught when you try to build ‘guix’, as with any other package

> (we don’t have special support for other packages, why would this one

> have to be different?).

>> 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!

>> 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 :-).

> Hi,

>

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

>

>>> 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!

>

> Thanks!

>

>>> 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 ;-).

>

> I think we had everything needed in Guile-Git.

> I hope this discussion can at least help improve mutual understanding on

> future patches and review processes.

>

> Thanks again for your time and patience!