[PATCH 0/2] Handle corner cases of 'guix import go'

  • Open
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Sharlatan Hellseher
  • Simon Tournier
Owner
unassigned
Submitted by
Simon Tournier
Severity
normal
S
S
Simon Tournier wrote on 22 Nov 20:05 +0100
(address . guix-patches@gnu.org)(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
cover.1732302033.git.zimon.toutoune@gmail.com
Hi,

Instead of that:

Toggle snippet (52 lines)
$ guix import go software.sslmate.com/src/go-pkcs12
guix import: Importing package "software.sslmate.com/src/go-pkcs12"...
guix import: warning: Unable to determine repository root of 'software.sslmate.com/src/go-pkcs12'. Guessing 'software.sslmate.com/src/go-pkcs12'.
SWH: found revision fa70679f0f1622a2705336a97225ee8d6c555f96 with directory at 'https://archive.softwareheritage.org/api/1/directory/43d3e9f7167cdcd9f179e8d1de33d0d22e8fbd46/'
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/HEAD
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/branches/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/config
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/description
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/hooks/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/info/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/info/exclude
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/info/refs
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/info/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/info/packs
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/pack/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/pack/pack-19cac523aaea8e45710dd5cc36bbee1ecc68369c.idx
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/pack/pack-19cac523aaea8e45710dd5cc36bbee1ecc68369c.pack
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/refs/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/refs/heads/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/refs/heads/master
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/refs/tags/
guix import: warning: Backtrace:
In ice-9/boot-9.scm:
1752:10 12 (with-exception-handler _ _ #:unwind? _ # _)
In unknown file:
11 (apply-smob/0 #<thunk 7fa8659d9300>)
In ice-9/boot-9.scm:
724:2 10 (call-with-prompt _ _ #<procedure default-prompt-handle?>)
In ice-9/eval.scm:
619:8 9 (_ #(#(#<directory (guile-user) 7fa8659dcc80>)))
In guix/ui.scm:
2330:7 8 (run-guix . _)
2293:10 7 (run-guix-command _ . _)
In guix/scripts/import.scm:
82:6 6 (guix-import . _)
In ice-9/boot-9.scm:
1752:10 5 (with-exception-handler _ _ #:unwind? _ # _)
In guix/scripts/import/go.scm:
116:29 4 (_)
In ice-9/eval.scm:
619:8 3 (_ #(#(#<directory (guix import go) 7fa85b580d20> # ?) ?))
174:20 2 (_ #(#(#<directory (guix import go) 7fa85b580d20> # ?) ?))
177:32 1 (lp (#<procedure 7fa8598a1380 at ice-9/eval.scm:191:12?>))
In ice-9/boot-9.scm:
1685:16 0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
In procedure git-error-message: Wrong type argument: #<&compound-exception components: (#<&error> #<&irritants irritants: (#<<git-error> code: -3 message: "reference 'refs/tags/v0.5.0' not found" class: 4>)> #<&exception-with-kind-and-args kind: git-error args: (#<<git-error> code: -3 message: "reference 'refs/tags/v0.5.0' not found" class: 4>)>)>

which is annoying because it is blocking when using the importer recursively,
the patch set produces that:

Toggle snippet (53 lines)
$ ./pre-inst-env guix import go software.sslmate.com/src/go-pkcs12
guix import: Importing package "software.sslmate.com/src/go-pkcs12"...
guix import: warning: Unable to determine repository root of 'software.sslmate.com/src/go-pkcs12'. Guessing 'software.sslmate.com/src/go-pkcs12'.
SWH: found revision fa70679f0f1622a2705336a97225ee8d6c555f96 with directory at 'https://archive.softwareheritage.org/api/1/directory/43d3e9f7167cdcd9f179e8d1de33d0d22e8fbd46/'
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/HEAD
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/branches/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/config
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/description
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/hooks/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/info/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/info/exclude
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/info/refs
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/info/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/info/packs
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/pack/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/pack/pack-19cac523aaea8e45710dd5cc36bbee1ecc68369c.idx
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/objects/pack/pack-19cac523aaea8e45710dd5cc36bbee1ecc68369c.pack
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/refs/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/refs/heads/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/refs/heads/master
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/refs/tags/
guix import: warning: Git cannot find the reference v0.5.0
(define-public go-software-sslmate-com-src-go-pkcs12
(package
(name "go-software-sslmate-com-src-go-pkcs12")
(version "0.5.0")
(source
(origin
(method git-fetch)
(uri (git-reference
(url "https://software.sslmate.com/src/go-pkcs12.git")
(commit (string-append "v" version))))
(file-name (git-file-name name version))
(sha256
(base32 "0000000000000000000000000000000000000000000000000000"))))
(build-system go-build-system)
(arguments
(list
#:import-path "software.sslmate.com/src/go-pkcs12"))
(home-page "https://software.sslmate.com/src/go-pkcs12")
(synopsis "package pkcs12")
(description
"Package pkcs12 implements some of PKCS#12 (also known as P12 or PFX). It is
intended for decoding DER-encoded P12/PFX files for use with the crypto/tls
package, and for encoding P12/PFX files for use by legacy applications which do
not support newer formats. Since PKCS#12 uses weak encryption primitives, it
SHOULD NOT be used for new applications.")
(license license:bsd-3)))


Please double check because it tweaks 'update-cached-checkout' which is used
by "guix pull". :-)

Cheers,
simon


Simon Tournier (2):
git: Catch Git errors when updating cached checkout.
import: go: Warn instead of error out when reference is missing.

guix/git.scm | 128 ++++++++++++++++++++++++++-------------------
guix/import/go.scm | 10 ++--
2 files changed, 82 insertions(+), 56 deletions(-)


base-commit: 043f02462766a913080723ad286028a288b79373
--
2.46.0
S
S
Simon Tournier wrote on 22 Nov 20:09 +0100
[PATCH 2/2] import: go: Warn instead of error out when reference is missing.
(address . 74481@debbugs.gnu.org)(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
cf8ee24549f66a8e34619b07ec6730103c120ba9.1732302033.git.zimon.toutoune@gmail.com
* guix/import/go.scm (git-checkout-hash): Return some hash although the
checkout does not contain the expected reference.

Change-Id: I40287fae29042a0ba106939a413239df6efde97d
---
guix/import/go.scm | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

Toggle diff (37 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index 32cba25b33..25ae989378 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -6,7 +6,7 @@
;;; Copyright © 2021-2022 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
-;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2021, 2024 Simon Tournier <zimon.toutoune@gmail.com>
;;; Copyright © 2023 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2024 Christina O'Donnell <cdo@mutix.org>
;;;
@@ -38,7 +38,8 @@ (define-module (guix import go)
#:use-module (guix http-client)
#:use-module (guix memoization)
#:autoload (htmlprag) (html->sxml) ;from Guile-Lib
- #:autoload (guix base32) (bytevector->nix-base32-string)
+ #:autoload (guix base32) (bytevector->nix-base32-string
+ nix-base32-string->bytevector)
#:autoload (guix build utils) (mkdir-p)
#:autoload (guix ui) (warning)
#:autoload (gcrypt hash) (hash-algorithm sha256)
@@ -566,7 +567,10 @@ (define* (git-checkout-hash url reference algorithm)
(update-cached-checkout url
#:ref
`(tag-or-commit . ,reference)))))
- (file-hash* checkout #:algorithm algorithm #:recursive? #true)))
+ (if (and checkout commit)
+ (file-hash* checkout #:algorithm algorithm #:recursive? #true)
+ (nix-base32-string->bytevector
+ "0000000000000000000000000000000000000000000000000000"))))
(define (vcs->origin vcs-type vcs-repo-url version subdir)
"Generate the `origin' block of a package depending on what type of source
--
2.46.0
S
S
Simon Tournier wrote on 22 Nov 20:09 +0100
[PATCH 1/2] git: Catch Git errors when updating cached checkout.
(address . 74481@debbugs.gnu.org)(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
cac32323de53f86d27508edc676ee7ed601d7b98.1732302033.git.zimon.toutoune@gmail.com
* guix/git.scm (resolve-reference): Catch Git error when reference does not
exist and return #false.
(switch-to-ref): Adjust.
(update-cached-checkout)[close-and-clean!]: New helper.
Catch Git error when reference does not exist and warn.
Return #false values when reference does not exist.

Change-Id: If6e244fe40ebee978ec8de51a6a68bcbd4a2c79e
---
guix/git.scm | 128 ++++++++++++++++++++++++++++++---------------------
1 file changed, 75 insertions(+), 53 deletions(-)

Toggle diff (193 lines)
diff --git a/guix/git.scm b/guix/git.scm
index 410cd4c153..328e2d5c8c 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -5,7 +5,7 @@
;;; Copyright © 2021 Marius Bakke <marius@gnu.org>
;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
;;; Copyright © 2023 Tobias Geerinckx-Rice <me@tobias.gr>
-;;; Copyright © 2023 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2023, 2024 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -265,7 +265,7 @@ (define (tag->commit repository tag)
(define (resolve-reference repository ref)
"Resolve the branch, commit or tag specified by REF, and return the
-corresponding Git object."
+corresponding Git object. Return #false if REF is not found."
(let resolve ((ref ref))
(match ref
(('branch . branch)
@@ -281,9 +281,12 @@ (define (resolve-reference repository ref)
;; can't be sure it's available. Furthermore, 'string->oid' used to
;; read out-of-bounds when passed a string shorter than 40 chars,
;; which is why we delay calls to it below.
- (if (< len 40)
- (object-lookup-prefix repository (string->oid commit) len)
- (object-lookup repository (string->oid commit)))))
+ (catch 'git-error
+ (lambda ()
+ (if (< len 40)
+ (object-lookup-prefix repository (string->oid commit) len)
+ (object-lookup repository (string->oid commit))))
+ (lambda _ #f))))
(('tag-or-commit . str)
(cond ((and (string-contains str "-g")
(match (string-split str #\-)
@@ -300,7 +303,10 @@ (define (resolve-reference repository ref)
=> (lambda (commit) (resolve `(commit . ,commit))))
((or (> (string-length str) 40)
(not (string-every char-set:hex-digit str)))
- (resolve `(tag . ,str))) ;definitely a tag
+ (catch 'git-error ;definitely a tag
+ (lambda ()
+ (resolve `(tag . ,str)))
+ (lambda _ #f)))
(else
(catch 'git-error
(lambda ()
@@ -336,13 +342,15 @@ (define (switch-to-ref repository ref)
(define obj
(resolve-reference repository ref))
- (reset repository obj RESET_HARD)
+ (and obj
+ (begin
+ (reset repository obj RESET_HARD)
- ;; There might still be untracked files in REPOSITORY due to an interrupted
- ;; checkout for example; delete them.
- (delete-untracked-files repository)
+ ;; There might still be untracked files in REPOSITORY due to an interrupted
+ ;; checkout for example; delete them.
+ (delete-untracked-files repository)
- (object-id obj))
+ (object-id obj))))
(define (call-with-repository directory proc)
(let ((repository #f))
@@ -562,6 +570,31 @@ (define* (update-cached-checkout url
(string-append directory "/" file)))
(or (scandir directory) '())))
+ (define (close-and-clean! repository)
+ (repository-close! repository)
+
+ ;; Update CACHE-DIRECTORY's mtime to so the cache logic sees it.
+ (match (gettimeofday)
+ ((seconds . microseconds)
+ (let ((nanoseconds (* 1000 microseconds)))
+ (utime cache-directory
+ seconds seconds
+ nanoseconds nanoseconds))))
+
+ ;; Run 'git gc' if needed.
+ (maybe-run-git-gc cache-directory)
+
+ ;; When CACHE-DIRECTORY is a sub-directory of the default cache
+ ;; directory, remove expired checkouts that are next to it.
+ (let ((parent (dirname cache-directory)))
+ (when (string=? parent (%repository-cache-directory))
+ (maybe-remove-expired-cache-entries parent cache-entries
+ #:entry-expiration
+ cached-checkout-expiration
+ #:delete-entry delete-checkout
+ #:cleanup-period
+ %checkout-cache-cleanup-period))))
+
(define canonical-ref
;; We used to require callers to specify "origin/" for each branch, which
;; made little sense since the cache should be transparent to them. So
@@ -583,56 +616,45 @@ (define* (update-cached-checkout url
;; Only fetch remote if it has not been cloned just before.
(when (and cache-exists?
(not (reference-available? repository ref)))
- (remote-fetch (remote-lookup repository "origin")
- #:fetch-options (make-default-fetch-options)))
+ (catch 'git-error
+ (lambda ()
+ (remote-fetch (remote-lookup repository "origin")
+ #:fetch-options (make-default-fetch-options)))
+ (lambda _
+ (warning (G_ "Git cannot update the cache of ~a~%") url))))
(when recursive?
(update-submodules repository #:log-port log-port
#:fetch-options (make-default-fetch-options)))
;; Note: call 'commit-relation' from here because it's more efficient
;; than letting users re-open the checkout later on.
- (let* ((oid (if check-out?
+ (let ((oid (if check-out?
(switch-to-ref repository canonical-ref)
(object-id
- (resolve-reference repository canonical-ref))))
- (new (and starting-commit
- (commit-lookup repository oid)))
- (old (and starting-commit
- (false-if-git-not-found
- (commit-lookup repository
- (string->oid starting-commit)))))
- (relation (and starting-commit
- (if old
- (commit-relation old new)
- 'unrelated))))
-
- ;; Reclaim file descriptors and memory mappings associated with
- ;; REPOSITORY as soon as possible.
- (repository-close! repository)
-
- ;; Update CACHE-DIRECTORY's mtime to so the cache logic sees it.
- (match (gettimeofday)
- ((seconds . microseconds)
- (let ((nanoseconds (* 1000 microseconds)))
- (utime cache-directory
- seconds seconds
- nanoseconds nanoseconds))))
-
- ;; Run 'git gc' if needed.
- (maybe-run-git-gc cache-directory)
-
- ;; When CACHE-DIRECTORY is a sub-directory of the default cache
- ;; directory, remove expired checkouts that are next to it.
- (let ((parent (dirname cache-directory)))
- (when (string=? parent (%repository-cache-directory))
- (maybe-remove-expired-cache-entries parent cache-entries
- #:entry-expiration
- cached-checkout-expiration
- #:delete-entry delete-checkout
- #:cleanup-period
- %checkout-cache-cleanup-period)))
-
- (values cache-directory (oid->string oid) relation)))))
+ (resolve-reference repository canonical-ref)))))
+ (if (not oid)
+ (begin
+ (warning (G_ "Git cannot find the reference ~a~%") (match canonical-ref
+ ((_ . ref) ref)))
+ (close-and-clean! repository)
+ (values #f #f #f))
+ (let* ((new (and starting-commit
+ (commit-lookup repository oid)))
+ (old (and starting-commit
+ (false-if-git-not-found
+ (commit-lookup repository
+ (string->oid starting-commit)))))
+ (relation (and starting-commit
+ (if old
+ (commit-relation old new)
+ 'unrelated))))
+
+ ;; Reclaim file descriptors and memory mappings associated with
+ ;; REPOSITORY as soon as possible.
+
+ (close-and-clean! repository)
+
+ (values cache-directory (oid->string oid) relation)))))))
(define* (latest-repository-commit store url
#:key
--
2.46.0
S
S
Simon Tournier wrote on 10 Dec 15:09 +0100
Re: [bug#74481] [PATCH 0/2] Handle corner cases of 'guix import go'
(address . 74481@debbugs.gnu.org)
874j3b1vf7.fsf@gmail.com
Hi,

On Fri, 22 Nov 2024 at 20:05, Simon Tournier <zimon.toutoune@gmail.com> wrote:

Toggle quote (3 lines)
> Please double check because it tweaks 'update-cached-checkout' which is used
> by "guix pull". :-)

If no more comment on that, I will push shortly.

Cheers,
simon
S
S
Sharlatan Hellseher wrote on 10 Dec 19:40 +0100
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
CAO+9K5o-QVqupDEd7wS4KeYY0+5yYQvsmZiz2VTtdoVuFuiDMQ@mail.gmail.com
Hi,

If it solves import issues let's have it merged.
I've not faced with such an issue before after importing 1k packages,
maybe it's just specific for some particular ones.

I've got go-build-system modifications pending to review,
improving test coverage for subdirs and relaxing build noise.

Your patches are LGFM.

Thanks,
Oleg
Attachment: file
L
L
Ludovic Courtès wrote on 12 Dec 12:34 +0100
Re: [bug#74481] [PATCH 1/2] git: Catch Git errors when updating cached checkout.
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
87bjxhb0da.fsf@gnu.org
Hi,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

Toggle quote (9 lines)
> * guix/git.scm (resolve-reference): Catch Git error when reference does not
> exist and return #false.
> (switch-to-ref): Adjust.
> (update-cached-checkout)[close-and-clean!]: New helper.
> Catch Git error when reference does not exist and warn.
> Return #false values when reference does not exist.
>
> Change-Id: If6e244fe40ebee978ec8de51a6a68bcbd4a2c79e

[...]

Toggle quote (33 lines)
> (define (resolve-reference repository ref)
> "Resolve the branch, commit or tag specified by REF, and return the
> -corresponding Git object."
> +corresponding Git object. Return #false if REF is not found."
> (let resolve ((ref ref))
> (match ref
> (('branch . branch)
> @@ -281,9 +281,12 @@ (define (resolve-reference repository ref)
> ;; can't be sure it's available. Furthermore, 'string->oid' used to
> ;; read out-of-bounds when passed a string shorter than 40 chars,
> ;; which is why we delay calls to it below.
> - (if (< len 40)
> - (object-lookup-prefix repository (string->oid commit) len)
> - (object-lookup repository (string->oid commit)))))
> + (catch 'git-error
> + (lambda ()
> + (if (< len 40)
> + (object-lookup-prefix repository (string->oid commit) len)
> + (object-lookup repository (string->oid commit))))
> + (lambda _ #f))))
> (('tag-or-commit . str)
> (cond ((and (string-contains str "-g")
> (match (string-split str #\-)
> @@ -300,7 +303,10 @@ (define (resolve-reference repository ref)
> => (lambda (commit) (resolve `(commit . ,commit))))
> ((or (> (string-length str) 40)
> (not (string-every char-set:hex-digit str)))
> - (resolve `(tag . ,str))) ;definitely a tag
> + (catch 'git-error ;definitely a tag
> + (lambda ()
> + (resolve `(tag . ,str)))
> + (lambda _ #f)))

Sorry for not replynig earlier. I would avoid such a change: it changes
the semantics of ‘resolve-reference’ in a non-trivial and hard-to-test
way.

Instead, since the goal is to address a problem that’s specific to
importers (or to one importer), I would suggest making a local change in
the importer itself, or in code that is shared among importers only.

(This comment applies to this entire patch.)

Ludo’.
S
S
Simon Tournier wrote on 12 Dec 13:04 +0100
(name . Ludovic Courtès)(address . ludo@gnu.org)
CAJ3okZ00aNLAbA6_bJCxUkhYHq4y1J-qADtgX16wjNMW9bXfWQ@mail.gmail.com
Hi Ludo,

On Thu, 12 Dec 2024 at 12:35, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (4 lines)
> Sorry for not replynig earlier. I would avoid such a change: it changes
> the semantics of ‘resolve-reference’ in a non-trivial and hard-to-test
> way.

Could you explain more what you mean for "the semantics" of
'resolve-reference'?

Toggle quote (4 lines)
> Instead, since the goal is to address a problem that’s specific to
> importers (or to one importer), I would suggest making a local change in
> the importer itself, or in code that is shared among importers only.

Well, from my understanding this suggestion is not possible because of the
way it's implemented. Or the importer needs a rewrite. The crash comes
from the call of 'update-cached-checkout' and it appears to me impossible
to catch the error at the level of the importer because it's too late.

If you think that's possible to have a local change somewhere in the
importers, please point me because I'm clueless. :-)

BTW, the bug of looking for an non-existent reference still remains. In
other words, all the calls to Guile-Git as 'object-lookup repository'
should be protected with some error-catch, IMHO.

Cheers,
simon
Attachment: file
L
L
Ludovic Courtès wrote on 17 Dec 11:55 +0100
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
877c7yr336.fsf@gnu.org
HI,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

Toggle quote (9 lines)
> On Thu, 12 Dec 2024 at 12:35, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> Sorry for not replynig earlier. I would avoid such a change: it changes
>> the semantics of ‘resolve-reference’ in a non-trivial and hard-to-test
>> way.
>
> Could you explain more what you mean for "the semantics" of
> 'resolve-reference'?

We have something that can now return #f instead of throwing an
exception. The many users of this interface are not prepared for this;
worse, getting #f instead of an exception means we lose information as
to why ‘update-cached-checkout’ or similar failed.

Toggle quote (4 lines)
>> Instead, since the goal is to address a problem that’s specific to
>> importers (or to one importer), I would suggest making a local change in
>> the importer itself, or in code that is shared among importers only.

[...]

Toggle quote (3 lines)
> If you think that's possible to have a local change somewhere in the
> importers, please point me because I'm clueless. :-)

I haven’t looked in detail, so I’m starting from the use case at the
beginning of this thread:

Toggle snippet (23 lines)
$ guix import go software.sslmate.com/src/go-pkcs12
guix import: Importing package "software.sslmate.com/src/go-pkcs12"...
guix import: warning: Unable to determine repository root of 'software.sslmate.com/src/go-pkcs12'. Guessing 'software.sslmate.com/src/go-pkcs12'.
SWH: found revision fa70679f0f1622a2705336a97225ee8d6c555f96 with directory at 'https://archive.softwareheritage.org/api/1/directory/43d3e9f7167cdcd9f179e8d1de33d0d22e8fbd46/'
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/HEAD
swh:1:rev:fa70679f0f1622a2705336a97225ee8d6c555f96.git/branches/

[...]

In guix/scripts/import/go.scm:
116:29 4 (_)
In ice-9/eval.scm:
619:8 3 (_ #(#(#<directory (guix import go) 7fa85b580d20> # ?) ?))
174:20 2 (_ #(#(#<directory (guix import go) 7fa85b580d20> # ?) ?))
177:32 1 (lp (#<procedure 7fa8598a1380 at ice-9/eval.scm:191:12?>))
In ice-9/boot-9.scm:
1685:16 0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
In procedure git-error-message: Wrong type argument: #<&compound-exception components: (#<&error> #<&irritants irritants: (#<<git-error> code: -3 message: "reference 'refs/tags/v0.5.0' not found" class: 4>)> #<&exception-with-kind-and-args kind: git-error args: (#<<git-error> code: -3 message: "reference 'refs/tags/v0.5.0' not found" class: 4>)>)>

Would it be an option to catch 'git-error around the
‘update-cached-checkout’ call in (guix import go)? If an exception is
thrown, it would print a warning and return a fake hash.

Ludo’.
S
S
Simon Tournier wrote on 17 Dec 18:45 +0100
[PATCH v2] import: go: Warn instead of error out when Git fails.
(address . 74481@debbugs.gnu.org)(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
494fef0566a7105bf4ae04f5efeae742561f3abb.1734457415.git.zimon.toutoune@gmail.com
* guix/import/go.scm (git-checkout-hash): Return some hash although updating
the cache fails.

Change-Id: I87e7701023a5f76f5d1494827473616e6a1275aa
---
guix/import/go.scm | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)

Toggle diff (41 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index 32cba25b33..6ede152601 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -38,7 +38,8 @@ (define-module (guix import go)
#:use-module (guix http-client)
#:use-module (guix memoization)
#:autoload (htmlprag) (html->sxml) ;from Guile-Lib
- #:autoload (guix base32) (bytevector->nix-base32-string)
+ #:autoload (guix base32) (bytevector->nix-base32-string
+ nix-base32-string->bytevector)
#:autoload (guix build utils) (mkdir-p)
#:autoload (guix ui) (warning)
#:autoload (gcrypt hash) (hash-algorithm sha256)
@@ -563,10 +564,18 @@ (define* (git-checkout-hash url reference algorithm)
(chmod cache #o700)
(let-values (((checkout commit _)
(parameterize ((%repository-cache-directory cache))
- (update-cached-checkout url
- #:ref
- `(tag-or-commit . ,reference)))))
- (file-hash* checkout #:algorithm algorithm #:recursive? #true)))
+ (catch 'git-error
+ (lambda ()
+ (update-cached-checkout url
+ #:ref
+ `(tag-or-commit . ,reference)))
+ (lambda (key err)
+ (warning (G_ "update-cached-checkout: ~s ~s~%") key err)
+ (values #f #f #f))))))
+ (if (and checkout commit)
+ (file-hash* checkout #:algorithm algorithm #:recursive? #true)
+ (nix-base32-string->bytevector
+ "0000000000000000000000000000000000000000000000000000"))))
(define (vcs->origin vcs-type vcs-repo-url version subdir)
"Generate the `origin' block of a package depending on what type of source

base-commit: a9003b8e6b40b59c9545ae87bb441d3549630db7
--
2.45.2
S
S
Simon Tournier wrote on 17 Dec 19:06 +0100
Re: [bug#74481] [PATCH 1/2] git: Catch Git errors when updating cached checkout.
(name . Ludovic Courtès)(address . ludo@gnu.org)
87seqmfall.fsf@gmail.com
Hi Ludo,

On Tue, 17 Dec 2024 at 11:55, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (5 lines)
> We have something that can now return #f instead of throwing an
> exception. The many users of this interface are not prepared for this;
> worse, getting #f instead of an exception means we lose information as
> to why ‘update-cached-checkout’ or similar failed.

Thanks for explaining.

Well, I’d say “mouais“ :-) about « lose information » since I added some
’pk’ here or there before finding the relevant piece of code. So the
exception is not very helpful, not to say fully useless, IMHO.

That’s said, I understand your concern and I will not discuss it
here. :-) If I read you correctly and apply the argument, you propose to
guard all the calls of ’update-cached-checkout’, it means:

Toggle snippet (6 lines)
./guix/import/egg.scm:94: (directory commit _ (update-cached-checkout url)))
./guix/scripts/system/reconfigure.scm:355: (update-cached-checkout
./guix/git.scm:670: (update-cached-checkout url
./guix/inferior.scm:877: (update-cached-checkout (channel-url channel)

Either using ‘(catch 'git-error’ or either using
‘(with-git-error-handling’.

Right?

Toggle quote (4 lines)
> Would it be an option to catch 'git-error around the
> ‘update-cached-checkout’ call in (guix import go)? If an exception is
> thrown, it would print a warning and return a fake hash.

See v2 for this case of ‘guix import go’.

Cheers,
simon

PS: As I said earlier, I think all the call to Guile-Git should be
guarded and each call should manage the exception instead of propagate
it… and so barely catch them. :-)
S
S
Sharlatan Hellseher wrote 7 days ago
[PATCH 0/2] Handle corner cases of 'guix import go'
(address . 74481@debbugs.gnu.org)(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
875xngpwxy.fsf@gmail.com
Hi Simon,

With your patch applied I could import 971 packages' templates which
100% cover this tasks!

Toggle snippet (17 lines)
(define-public sqls
(package
(name "sqls")
;; TODO: The latest version requires a way more packages to be available
;; in Guix.
(version "0.2.18")
(source (origin
(method git-fetch)
(uri (git-reference
(url "https://github.com/sqls-server/sqls")
(commit (string-append "v" version))))
(file-name (git-file-name name version))
(sha256
(base32
"13837v27avdp2nls3vyy7ml12nj7rxragchwf92adn10ffp4aj6c"))))

Packages were not tested but importer did not break and completed after
2h.

--
Oleg
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEmEeB3micIcJkGAhndtcnv/Ys0rUFAmdjLfkACgkQdtcnv/Ys
0rUUNg/8Dmp33G+c4iHNUHPgikwumVBPQE2Y4UwNtI9BptD5HE5xphLjespgOVIv
xZKBWQVw2iZguCmh45mKvSbsD46YaGoo6xFe6T5xmwTboNY0K2Uj+tUEGwWnVuSq
O1fer5BUW1DYO9F25jacrM/9eWvC2DjSSCrWGlpjTHgNhMBSXYMTnqukj+k9QgD5
Q/TWB070HBrUnrC0P4yI+qu8wz+jKtGQ1panCTp/Xb7/zzU7aSdU4GqSljtMBfGk
/Aycepj/k4MTRKrCjCbJbUmQlY0DiXCRZtCXbrpXR2Hw85366n8/UF7961VvVO9m
Kql/Usq1GyAMXoBQby/f4ElPx/iODaiZpGieAqn5GMdLYX8opYZOlfkovfjXPzO0
ZbGF59sO+OEN6LO6uN98JkVS8WhsFCr4PaNerh2v0nUEoVel7HU8+SnA84mv/OPE
bhCAL9+Fq9jY3kWmaE/JXnzu0Ng0Fchacri7fMVutZ6fjAtLltKsN1kiBnqxkl+4
6VJbTaDTeKOLIE/PWG8loQMaxy33/+QoE5dmBNKyvq58c9JVuzZcOEDAkC6QA8TS
LwGy48xrwGLMwJ5DzUo4jRQyEMBxclXaBrAgt92t/sVzxThPmK04lufCUyBgljNF
xqCNTOQ400Urex8cJCv4MUXLuOkYHjSEb8cbpGd6XpbjrlFxg1I=
=s1pK
-----END PGP SIGNATURE-----

?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 74481@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 74481
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch