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

Previous Next

Package: guix-patches;

Reported by: paul <goodoldpaul <at> autistici.org>

Date: Sun, 25 Aug 2024 13:57:02 UTC

Severity: normal

Done: paul <goodoldpaul <at> autistici.org>

Full log


View this message in rfc822 format

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: [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’.




This bug report was last modified 27 days ago.

Previous Next


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