bug#43893: [PATCH v3] maint: update-guix-package: Prevent accidentally breaking guix pull.

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hello Ludovic,

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

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

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.

We could add the sources to the store from the clean checkout, but I
thought one great thing about the patch was that it removed interactions
with the store, allowing for the source derivations to happen normally
when testing with 'guix build guix' (previously you'd have had to 'guix
build guix --check', as the store had silently been pre-populated with
the sources).  I see value in using the usual mechanism to get the
source rather than a side-channel, optimization hack, as it will help
ensuring correctness.

Perhaps I misunderstood your point?

Thank you,

Maxim