[bug#71764] [PATCH v7] gnu: Add got.

[Ludovic Courtès]

Hi,

Ashish SHUKLA <ashish.is@lostca.se> skribis:

> * gnu/packages/version-control.scm (got): New variable.
>
> Change-Id: I34d19d90ab36e8831fa88668676aa2399356b002

[...]

> +(define-public got

[...]

> +    (inputs
> +     `(("libevent" ,libevent)
> +       ("libuuid" ,util-linux "lib")
> +       ("zlib" ,zlib)
> +       ("libressl" ,libressl)
> +       ("libmd" ,libmd)
> +       ("libbsd" ,libbsd)
> +       ("ncurses" ,ncurses)))

Please run ‘guix style -S inputs --input-simplification=always got’.

> +     `(;; disable runpath validation, courtesy: libbsd's special
> +       ;; treatment of libmd
> +       #:validate-runpath? #f

It isn’t clear from the comment why RUNPATH validation is disabled, and
there has to be a very good reason because usually any problem it
detects is going to be an issue at run time.

Could you double-check?

> +       #:configure-flags
> +       '("CFLAGS=-DGOT_DIAL_PATH_SSH=\\\"ssh\\\" 
> -DGOT_TAG_PATH_SSH_KEYGEN=\\\"ssh-keygen\\\" 
> -DGOT_TAG_PATH_SIGNIFY=\\\"signify\\\"")

Please wrap lines to 80 characters.

Also, setting CFLAGS like this runs the risk of overriding the default,
which are usually “-O2 -g”, and thus building without optimizations.

Could you check whether this is the case and perhaps add “-O2 -g”?

> +      #:phases ,#~(modify-phases %standard-phases
> +                    (add-after 'unpack 'patch-execv-to-execvp
> +                      (lambda _
> +                        ;; got sources has paths hardcoded to /usr/bin
> +                        (substitute* "lib/dial.c"
> +                          (("execv\\(GOT_DIAL_") "execvp(GOT_DIAL_")
> +                          (("execv %s\", GOT_DIAL") "execvp %s\", GOT_DIAL"))
> +                        (substitute* "lib/sigs.c"
> +                          (("execv\\(GOT_TAG") "execvp(GOT_TAG")
> +                          (("execv %s\", GOT_TAG") "execvp %s\", GOT_TAG"))
> +                        #t)))))

Please remove the trailing #t.

Could you send an updated patch?

Thanks,
Ludo’.