Hello! Ludovic Courtès 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. >> - ;; 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? Yes. [...] > 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 Using the worktree approach, checking for local changes is made unnecessary; as long as the tool clearly says which commit will be used for the updated Guix package, I don't see why we should force the developer to stash their changes (in the same way git checkout doesn't forces you to do so in the absence of conflicts). Also, this bug reported was triggered by something like: --8<---------------cut here---------------start------------->8--- echo ";;some comment" >> .dir-locals \ && git update-index --skip-worktree .dir-locals \ && make update-guix-package) --8<---------------cut here---------------end--------------->8--- leading 'update-guix-package' to produce a wrong hash. Git has been told to ignore changes on .dir-locals so checking for changes would not have been sufficient (you'd also need to 'git ls-files -v' and interpret the result). > 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. That's a good idea; implemented in the v2 patch I'm about to send. Thanks, Maxim