Package: guix-patches;
Reported by: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
Date: Wed, 3 Jun 2020 14:31:01 UTC
Severity: normal
Done: Ricardo Wurmus <rekado <at> elephly.net>
Bug is archived. No further changes may be made.
Message #10 received at 41688 <at> debbugs.gnu.org (full text, mbox):
From: Ludovic Courtès <ludo <at> gnu.org> To: Katherine Cox-Buday <cox.katherine.e <at> gmail.com> Cc: 41688 <at> debbugs.gnu.org Subject: Re: [bug#41688] Add rsyslog Date: Fri, 12 Jun 2020 18:45:26 +0200
[Message part 1 (text/plain, inline)]
Hi Katherine, Thanks for this long patch series! I’ve applied half of it. Here are some comments: Katherine Cox-Buday <cox.katherine.e <at> gmail.com> skribis: > From 8d8d97441b397a48ae26761c2d826f35ae5d56e9 Mon Sep 17 00:00:00 2001 > From: Katherine Cox-Buday <cox.katherine.e <at> 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.
[Message part 2 (text/x-patch, inline)]
diff --git a/gnu/packages/c.scm b/gnu/packages/c.scm
index d3820c88b2..12a9b56c51 100644
--- a/gnu/packages/c.scm
+++ b/gnu/packages/c.scm
@@ -370,6 +370,6 @@ releases.")
(home-page "https://github.com/rsyslog/libestr")
(synopsis "Helper functions for handling strings")
(description
- "A library which contains some essential string manipulation
-functions and more, like escaping special characters.")
- (license license:lgpl2.1)))
+ "This C library contains some essential string manipulation functions and
+more, like escaping special characters.")
+ (license license:lgpl2.1+)))
[Message part 3 (text/plain, inline)]
> From 0e0e9cc550ff002d7bebb4f93b573c5ba527ac4d Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e <at> 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 <at> 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 <at> 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.
[Message part 4 (text/x-patch, inline)]
diff --git a/gnu/packages/c.scm b/gnu/packages/c.scm
index e935042572..2e62111d44 100644
--- a/gnu/packages/c.scm
+++ b/gnu/packages/c.scm
@@ -394,14 +394,9 @@ more, like escaping special characters.")
("automake" ,automake)
("libtool" ,libtool)))
(home-page "https://github.com/rsyslog/libfastjson")
- (synopsis "Fast json library for C ")
+ (synopsis "Fast JSON library for C")
(description
- "libfastjson is a fork from json-c, and is currently under development.
-The aim of this project is not to provide a slightly modified clone of json-c.
-It's aim is to provide: a small library with essential json handling
-functions, sufficiently good json support (not 100% standards compliant), and
-be very fast in processing.")
- (license
- (license:non-copyleft
- "https://github.com/rsyslog/libfastjson/blob/master/COPYING"
- "It is a MIT license."))))
+ "libfastjson is a fork from json-c aiming to provide: a small library
+with essential JSON handling functions, sufficiently good JSON support (not
+100% standards compliant), and very fast processing.")
+ (license license:expat)))
[Message part 5 (text/plain, inline)]
> From 9e0c24cc6e0666581b11b45f831ab49486adbdf8 Mon Sep 17 00:00:00 2001 > From: Katherine Cox-Buday <cox.katherine.e <at> 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’.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.