Re: [PATCH] fpm2 package derivation

[Ludovic Courtès]

Hello Thomas,

Thomas Sigurdsen <address@hidden> skribis:

> Hi, just managed to make this work, ran it through guix lint and think it
> should be allright. With that said I'm more than happy for feedback seeing as
> I'm a guile newbie and never contributed much to anything before (even though
> my papers say I have training as a programmer :P ).

Well, good job so far!  :-)

> Not sure if this is the right place to ask, but I'll go ahead: the previous
> version that did not build was this:
>
> https://notabug.org/thomassgn/guixsd-configuration/src/8dd3f613371b7f1ab28111f061f6735646174ee0/modules/tms/fpm2.scm
>
> I don't understand why this did not work and the working definition does. I
> arrived at the new working definition by looking at the definition of xfig
> and nvi (also in gnu/packages/).

I don’t know how this did not work exactly, but one thing that comes to
mind is that perhaps the open port didn’t get flushed because that
version didn’t have a ‘close-port’ call at the end.

BTW, when dealing with files it’s safer to use ‘call-with-output-file’
instead of ‘open-output-file’:

  (call-with-output-file "foo.txt"
    (lambda (port)
      (display something port)
      …))

It takes care of closing the port when the lambda is left one way or
another (exceptions, etc.)

> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-before
> +             'configure 'pre-configure
> +           (lambda _
> +               (let* ((poinc (open-output-file "po/POTFILES.in")))
> +                 (begin
> +                   (newline poinc)
> +                   (display "data/fpm2.desktop.in" poinc)
> +                   (newline poinc)
> +                   (display "data/fpm2.desktop.in.in" poinc)
> +                   (newline poinc)
> +                   (display "fpm2.glade" poinc)
> +                   (newline poinc)
> +                   (display "src/callbacks.c" poinc)
> +                   (newline poinc)
> +                   (display "src/fpm.c" poinc)
> +                   (newline poinc)
> +                   (display "src/fpm_file.c" poinc)
> +                   (newline poinc)
> +                   (display "src/interface.c" poinc)
> +                   (newline poinc)
> +                   (display "src/support.c" poinc)
> +                   (newline poinc)
> +                   (display "fpm2.glade" poinc)
> +                   (newline poinc)
> +                   (close-port poinc))))))))

So I’d suggest ‘call-with-output-file’ and a loop:

  (for-each (lambda (file)
              (display file port))
            '("fmp2.glade" …))

However, could you explain why this is needed in the first place?

> +    (synopsis "Manages, generates and stores passwords encrypted")
> +    (description "FPM2 is GTK2 port from Figaro's Password Manager
> +originally developed by John Conneely, with some new enhancements.
> +
> +Upstream development seems to have stopped. It is therefore recommended
> +to use a different password manager.  ")

This last part is not confidence-inspiring.  :-)  Do you think it’s a
good idea to provide it to our users anyway, when there’s already a
number of password managers available?  Many won’t notice that last
sentence.

Thank you,
Ludo’.