[bug#52691] [PATCH] git: Display a hint when CLONE* lacks permission.

[bug#52691] [PATCH] git: Display a hint when CLONE* lacks permission.

[Tobias Geerinckx-Rice via Guix-patches via]

Without this, it's not obvious where users should even start looking:

  $ guix pull
  Updating channel 'guix' from Git repository at URL...
  guix pull: error: mkdir: Permission denied
  $

* guix/git.scm (clone*): Print DIRECTORY as a hint on EPERM.

Reported by karrq on #guix.
---
 guix/git.scm | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/guix/git.scm b/guix/git.scm
index dc2ca1be84..1b322be0fe 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -35,6 +35,7 @@ (define-module (guix git)
   #:use-module (guix gexp)
   #:use-module (guix sets)
   #:use-module ((guix diagnostics) #:select (leave warning))
+  #:use-module (guix ui)
   #:use-module (guix progress)
   #:autoload   (guix swh) (swh-download commit-id?)
   #:use-module (rnrs bytevectors)
@@ -200,8 +201,16 @@ (define (clone* url directory)
       (clone url directory
              (make-clone-options
               #:fetch-options (make-default-fetch-options))))
-    (lambda _
-      (false-if-exception (rmdir directory)))))
+    (lambda args
+      (false-if-exception (rmdir directory))
+      ;; Compensate for unixy errors, e.g., ‘error: mkdir: Permission denied’.
+      ;; XXX This displays the hint before the error.  After would be nicer.
+      ;; XXX So would a generic mechanism for dealing with such errors.
+      (match args
+        ((system-error _ _ _ EPERM)
+         (display-hint (format #f (G_ "can you create and write to ~a?")
+                               directory)))
+        (_ #f)))))
 
 (define (url+commit->name url sha1)
   "Return the string \"<REPO-NAME>-<SHA1:7>\" where REPO-NAME is the name of
-- 
2.34.0

[bug#52691] [PATCH] git: Display a hint when CLONE* lacks permission.

[zimoun]

[-- Attachment #1: Type: text/plain, Size: 849 bytes --]

Hi Tobias,

On mar., 21 déc. 2021 at 01:51, Tobias Geerinckx-Rice <me@tobias.gr> wrote:
> Without this, it's not obvious where users should even start looking:
>
>   $ guix pull
>   Updating channel 'guix' from Git repository at URL...
>   guix pull: error: mkdir: Permission denied

Instead of hint, I propose this patch which displays:

--8<---------------cut here---------------start------------->8---
Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...
guix pull: error: mkdir: Permission non accordée /home/sitour/.cache/guix/checkouts/pjmkglp4t7znuugeurpurzikxq3tnlaywmisyr27shj7apsnalwq
--8<---------------cut here---------------end--------------->8---

Somehow, the URL is already reported and the patch exposes the hidden
’directory’ variable whatever the error could be.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: clone.patch --]
[-- Type: text/x-diff, Size: 764 bytes --]

diff --git a/guix/git.scm b/guix/git.scm
index dc2ca1be84..e2285f5f55 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -200,8 +200,12 @@ (define (clone* url directory)
       (clone url directory
              (make-clone-options
               #:fetch-options (make-default-fetch-options))))
-    (lambda _
-      (false-if-exception (rmdir directory)))))
+    (lambda (key subr fmt args . rest)
+      (let ((message (match args
+                       ((reason tail ...)
+                        (list (string-append reason " " directory))))))
+        (false-if-exception (rmdir directory))
+        (apply throw key subr fmt message rest)))))
 
 (define (url+commit->name url sha1)
   "Return the string \"<REPO-NAME>-<SHA1:7>\" where REPO-NAME is the name of

[-- Attachment #3: Type: text/plain, Size: 1568 bytes --]



> +  #:use-module (guix ui)

I think that less UI is outside guix/scripts/ and easy maintenance we
have.  Ideally, G_ should only be under guix/scripts/.


Last, if my proposal does not fit, from this snippet,

--8<---------------cut here---------------start------------->8---
+    (lambda args
+      (false-if-exception (rmdir directory))
+      ;; Compensate for unixy errors, e.g., ‘error: mkdir: Permission denied’.
+      ;; XXX This displays the hint before the error.  After would be nicer.
+      ;; XXX So would a generic mechanism for dealing with such errors.
+      (match args
+        ((system-error _ _ _ EPERM)
+         (display-hint (format #f (G_ "can you create and write to ~a?")
+                               directory)))
+        (_ #f)))))
--8<---------------cut here---------------end--------------->8---

the ’match’ can be avoided

--8<---------------cut here---------------start------------->8---
    (lambda (key subr message args . rest)
      (false-if-exception (rmdir directory))
      ;; Compensate for unixy errors, e.g., ‘error: mkdir: Permission denied’.
      ;; XXX This displays the hint before the error.  After would be nicer.
      ;; XXX So would a generic mechanism for dealing with such errors.
      (when (= EPERM (car rest))
         (display-hint (format #f (G_ "can you create and write to ~a?")
                               directory))))
--8<---------------cut here---------------end--------------->8---

Or someting along these lines. :-)


Cheers,
simon

[bug#52691] [PATCH] git: Display a hint when CLONE* lacks permission.

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

Simon,

zimoun 写道：
> Instead of hint, I propose this patch which displays:

Perfect!  That's what I wanted to write, but couldn't :-)

(I still don't really understand THROW and friends, or what 
exactly your snippet does, but the output is exactly what I had in 
mind.)

Thanks!  LGTM, obviously.

> the ’match’ can be avoided

True, but I chose it deliberately, and still prefer it over CAR or 
equivalent.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

[bug#52691] [PATCH] git: Display a hint when CLONE* lacks permission.

[zimoun]

Hi,

On Tue, 21 Dec 2021 at 16:03, Tobias Geerinckx-Rice <me@tobias.gr> wrote:

> zimoun 写道：
> > Instead of hint, I propose this patch which displays:
>
> Perfect!  That's what I wanted to write, but couldn't :-)
>
> (I still don't really understand THROW and friends, or what
> exactly your snippet does, but the output is exactly what I had in
> mind.)

Me neither!  Especially with new and old way,  And the examples in the
manual do not help very well to understand, IMHO.  Another story. :-)

> Thanks!  LGTM, obviously.

Feel free to proceed. :-)


> > the ’match’ can be avoided
>
> True, but I chose it deliberately, and still prefer it over CAR or
> equivalent.

Yes, me too.  This 'car' was because I had been lazy.  It was about
this pattern:

  (lambda args
    (match args
      ((a b c) #t)
      (_ #f)

when 'args' is an error. :-)


Cheers,
simon