Hello Ludovic! Ludovic Courtès writes: > Hi Maxim, > > Maxim Cournoyer 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). >>> (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. Thanks for providing a rational for it. I'll git-send a new patch which adds the source to the store using the procedure you shared above, but otherwise keeps the existing mechanism intact. Thank you for you patience! Maxim