[bug#41688] Add rsyslog

Hi Katherine,

Thanks for this long patch series!  I’ve applied half of it.  Here are
some comments:

Katherine Cox-Buday <cox.katherine.e@gmail.com> skribis:

> From 8d8d97441b397a48ae26761c2d826f35ae5d56e9 Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e@gmail.com>
> Date: Tue, 2 Jun 2020 14:55:23 -0500
> Subject: [PATCH 1/5] gnu: Add libestr.
>
> * gnu/packages/c.scm (libestr): New variable.

Applied with the changes below.
> From 0e0e9cc550ff002d7bebb4f93b573c5ba527ac4d Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e@gmail.com>
> Date: Tue, 2 Jun 2020 15:47:41 -0500
> Subject: [PATCH 5/5] gnu: Add rsyslog.
>
> * gnu/packages/logging.scm (rsyslog): New variable.

[...]

> +(define-public rsyslog
> +  (let ((modules (list "fmhash" "fmhttp" "imbatchreport"
> +                       "imczmq" "imdiag" "imdocker"
> +                       "imfile" "imgssapi" "imkafka"
> +                       "imklog" "imkmsg" "immark"
> +                       "improg" "impstats" "imptcp" "imtcp"
> +                       "imtuxedoulog" "imudp" "imuxsock"
> +                       "mmanon" "mmaudit" "mmcount"
> +                       "mmdblookup" "mmexternal"
> +                       "mmfields" "mmjsonparse"
> +                       "mmkubernetes" "mmnormalize"
> +                       "mmpstrucdata" "mmrfc5424addhmac"
> +                       "mmrm1stspace" "mmsequence"
> +                       "mmsnmptrapd" "mmtaghostname"
> +                       "mmutf8fix" "omclickhouse"
> +                       "omczmq" "omelasticsearch"
> +                       "omfile-hardened" "omgssapi"
> +                       "omhttpfs" "omhttp" "omkafka"
> +                       "omlibdbi" "ommail" "ommysql" "ompgsql"
> +                       "omprog" "omruleset" "omsnmp"
> +                       "omstdout" "omtcl" "omtesting"
> +                       "omudpspoof" "omuxsock"
> +                       "pmaixforwardedfrom" "pmciscoios"
> +                       "pmcisconames" "pmdb2diag"
> +                       "pmlastmsg" "pmnormalize"
> +                       "pmnull" "pmpanngfw" "pmsnare")))

Could you add a comment explaining where this list comes from?
Alternatively, do you think it could be generated at build time?

> +         #:configure-flags
> +         (list
> +          "--enable-largefile"
> +          "--enable-inet"
> +          "--enable-regexp"
> +          "--enable-rsyslogrt"
> +          "--enable-rsyslogd"

That’s also a very long list.  :-)

Can we rely on defaults?  Or, alternatively, could you add a comment
explaining how to come up with the list?

> +      (outputs `("out" ,@modules))

Oh.  Is it a good idea to have one output per module?  We’ve never done
such a thing before.  Usually, extra outputs are justified if not having
them would lead to a big closure (as reported by ‘guix size’).  The
solution is usually to have two or three outputs, but not 20ish.  :-)

WDYT?

> +      (home-page "https://www.rsyslog.com/")
> +      (synopsis "RSYSLOG is the rocket-fast system for log processing")
> +      (description
> +       "It offers high-performance, great security features and a modular

s/It/Rsyslog/

> +      (license (list license:gpl3
> +                     license:asl2.0
> +                     license:lgpl3)))))

Please add a comment above explaining if it’s triple-licensing or a
combination, and possibly add missing ‘+’ signs.

> From 3667290c9d419ce5404d6a0631bf20ddbcf1c286 Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e@gmail.com>
> Date: Tue, 2 Jun 2020 15:34:19 -0500
> Subject: [PATCH 4/5] gnu: Add liblognorm.
>
> * gnu/packages/c.scm (liblognorm): New variable.

[...]

> +    (arguments
> +     ;; Bash scripts interact with the filesystem
> +     `(#:tests? #f

Could you clarify what this means with regards to running tests?

> +       #:configure-flags
> +       (list (string-append "--includedir="
> +                            (assoc-ref %outputs "dev")
> +                            "/include"))
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'install 'fix-circular-dependency
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (write (string-append "KT: " (assoc-ref outputs "lib")
> +                                   "/lib/pkgconfig"))
> +             (write (string-append "KT: " (assoc-ref outputs "dev")
> +                                   "/lib/pkgconfig"))

Leftovers.  :-)

> +             (let ((pkgconfig (string-append (assoc-ref outputs "dev")
> +                                             "/lib/pkgconfig")))
> +               (mkdir-p pkgconfig)
> +               (rename-file (string-append (assoc-ref outputs "lib")
> +                                           "/lib/pkgconfig")
> +                            pkgconfig)))))))

Missing #t return value.

> +    (outputs '("out" "lib" "dev"))

“dev” is not part of the output names conventionally used.  However,
there’s “include”, which is automatically recognized by gnu-build-system
and turned into a ‘--includedir’ flag.  Perhaps you could try this?

Or you can just have “out” and “lib”: so far, unless headers are very
large, we just keep them alongside the library.

> +    (home-page "https://www.liblognorm.com")
> +    (synopsis
> +     "Fast samples-based log normalization library")
> +    (description
> +     "Liblognorm normalizes event data into well-defined name-value
> +pairs and a set of tags describing the message.")
> +    (license license:lgpl2.1)))

Could it be lgpl2.1+?  (I haven’t checked.)

> From db4bcfc18e95fa39851c72414261b19b25c49db0 Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e@gmail.com>
> Date: Tue, 2 Jun 2020 15:24:59 -0500
> Subject: [PATCH 2/5] gnu: Add libfastjson.
>
> * gnu/packages/c.scm (libfastjson): New variable.

Applied with minor changes.
> From 9e0c24cc6e0666581b11b45f831ab49486adbdf8 Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e@gmail.com>
> Date: Tue, 2 Jun 2020 15:29:08 -0500
> Subject: [PATCH 3/5] gnu: Add liblogging.
>
> * gnu/packages/c.scm (liblogging): New variable.

Applied.

Thank you!

Ludo’.