GNU bug report logs - #72803
Add restic commands to the restic-guix package

Previous Next

Full log

Message #72 received at 72803 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Giacomo Leidi <goodoldpaul <at> autistici.org>
Cc: 72803 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: bug#72803: Add restic commands to the restic-guix package
Date: Mon, 10 Mar 2025 14:52:32 +0100

Hi,

Giacomo Leidi <goodoldpaul <at> autistici.org> skribis:

> This patch refactors the way restic commands can be added to the
> restic-guix package with a more general approach.  This way new
> subcommands for restic-guix can be added more easily.
>
> * gnu/services/backup.scm (restic-backup-job-program): Generalize to
> restic-program;
> (restic-guix): allow for multiple actions.
>
> * doc/guix.texi: Document it.
>
> Change-Id: Ib2b5d74bebc51e35f1ae6e1aa32cedee0da59697
> +(define (restic-program config)
> +  #~(lambda* (action action-args job-restic repository password-file verbose? extra-flags)
> +      (use-modules (ice-9 format)
> +                   (ice-9 popen)
> +                   (ice-9 rdelim))

Please move ‘use-modules’ to the top-level; that’s the only guaranteed
way to use it.

> +(define* (restic-guix config #:key (supported-actions
> +                                    %restic-guix-supported-actions))
>    (program-file
>     "restic-guix"
>     #~(begin
>         (use-modules (ice-9 match)
>                      (srfi srfi-1))
>  
> -       (define names '#$(map restic-backup-job-name jobs))
> -       (define programs '#$(map restic-backup-job-program jobs))
> -
> -       (define (get-program name)
> -         (define idx
> -           (list-index (lambda (n) (string=? n name)) names))
> -         (unless idx
> -           (error (string-append "Unknown job name " name "\n\n"
> -                                 "Possible job names are: "
> -                                 (string-join names " "))))
> -         (list-ref programs idx))
> +       (define jobs
> +         (list
> +          #$@(map restic-backup-job->kv
> +                  (restic-backup-configuration-jobs config))))
> +       (define names (map first jobs))
> +       (define (get-job key)
> +         (first
> +          (filter-map
> +           (match-lambda
> +             ((k v)
> +              (and (string=? key k) v)))
> +           jobs)))
>  
> -       (define (backup args)
> -         (define name (third args))
> -         (define program (get-program name))
> -         (execlp program program))
> +       (define restic-exec
> +         #$(restic-program config))
>  
>         (define (validate-args args)
> -         (when (not (>= (length args) 3))
> -           (error (string-append "Usage: " (basename (car args))
> -                                 " backup NAME"))))
> +         (unless (>= (length args) 2)
> +           (error (string-append "Usage: " (basename (first args))
> +                                 " ACTION [ARGS]\n\nSupported actions are: "
> +                                 #$(string-join supported-actions ", ") ".")))
> +         (unless (member (second args) '#$supported-actions)
> +           (error (string-append "Unknown action: " (second args) ". Supported"
> +                                 "actions are: "
> +                                 #$(string-join supported-actions ", ") "."))))
> +
> +       (define (validate-action-args action args)
> +         (define argc (length args))
> +         (when (not (>= argc 3))
> +           (error (string-append "Usage: " (basename (first args))
> +                                 " " action " JOB_NAME [ARGS]\n\nPossible job "
> +                                 "names are: " (string-join names ", ") ".")))
> +         (define job-name (third args))
> +         (unless (member job-name names)
> +           (error (string-append "Unknown job name: " job-name ". Possible job "
> +                                 "names are: " (string-join names ", ") ".")))
> +         (let ((job (get-job job-name))
> +               (action-args
> +                (if (> argc 3)
> +                    (take-right args (- argc 3))
> +                    '())))
> +           (values job action-args)))
>  
>         (define (main args)
>           (validate-args args)
>           (define action (second args))
> -         (match action
> -           ("backup"
> -            (backup args))
> -           (_
> -            (error (string-append "Unknown action: " action)))))
> +         (define-values (job action-args) (validate-action-args action args))
> +         (apply restic-exec `(,action ,action-args ,@job)))

I see two issues here:

  1. This is stepping on the toes of upstream: why are we providing a
     non-trivial program like this downstream?

  2. There are stylistic issues: use of ‘first’ & co. (info "(guix) Data
     Types and Pattern Matching"), use of ‘error’ (it is too generic and
     user-unfriendly), custom argument parsing procedure.

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.