Re: gnu-patches back log

2017-02-28 Thread Leo Famulari
On Tue, Feb 28, 2017 at 06:25:31AM +, Pjotr Prins wrote:
> Now we have debbugs we can see there is a building back-log:
> 
>   
> https://debbugs.gnu.org/cgi/pkgreport.cgi?package=guix-patches;max-bugs=100;base-order=1;bug-rev=1
> 
> A patch like this one
> 
>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25725
> 
> has been two weeks without comment. I think we should not leave patches 
> without
> feedback longer than one week - even 3 days, to be honest. It is the surest 
> way
> to kill enthusiasm.
> 
> To move forward with Guix and to recognise the effort new submitters
> put in I would like to ask *all* reviewers to pick an outstanding
> patch on a regular basis. If reviewers split the work it should be doable.

We all know that patch review is important. But it's also real work, and
just as hard as writing patches in many cases. I think we all do it when
we find the motivation. 

> Would it be an idea to send out weekly E-mails with patches that had
> no attention to a select list of reviewers? Or maybe to the ML as a
> whole? Basically it would read:

As long as the list of reviewers volunteered for that.

We already get the messages with the patches. I wonder if adding yet
another message to our mail boxes is going to help. At least for me, the
issue is finding the energy to review things, not tools for finding old
patches.

If we are interested in handling submissions more quickly, we could
arrange for package-related changes to be linted and built before they
get sent to the list subscribers. Spending time on a patch series before
learning that the submitter did not even test it reduces my motivation
to review.



Re: gnu-patches back log

2017-02-28 Thread Pjotr Prins
On Tue, Feb 28, 2017 at 12:14:52PM +0100, Hartmut Goebel wrote:
> Am 28.02.2017 um 07:25 schrieb Pjotr Prins:
> > Would it be an idea to send out weekly E-mails with patches that had
> > no attention to a select list of reviewers? Or maybe to the ML as a
> > whole? Basically it would read:
> 
> This might be a good idea.
> 
> Please mind adding links to that mail so one can easily access the
> patches and the "reports". This would make occasional reviewers live
> much easier.

Also, not all reviewers have push rights. Maybe we can add tags for
'PLEASE REVIEW', 'IN REVIEW', 'HELP WANTED', 'LGTM' and 'PLEASE PUSH'.
Without capitals ;)

That way the masters can quickly scan for pathes that need attention.

Pj.



Re: `guix pull` over HTTPS

2017-02-28 Thread Leo Famulari
On Wed, Mar 01, 2017 at 03:36:11AM +0100, Marius Bakke wrote:
> Subject: [PATCH] pull: Default to HTTPS.
> 
> * guix/build/download.scm (tls-wrap): Add CERTIFICATE-DIRECTORY parameter.
> (open-connection-for-uri): Adjust parameters to match.
> (http-fetch): Likewise.
> (url-fetch): Likewise.
> * guix/download.scm (download-to-store): Likewise.
> * guix/scripts/pull.scm (%snapshot-url): Use HTTPS.
> (guix-pull): Verify against the store path of NSS-CERTS.

When I don't have GnuTLS in my environment, it fails like this:

Starting download of /tmp/guix-file.pSCYyI
From https://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz...
;;; Failed to autoload make-session in (gnutls):
;;; ERROR: missing interface for module (gnutls)
ERROR: In procedure module-lookup: Unbound variable: make-session
failed to download "/tmp/guix-file.pSCYyI" from 
"https://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz";
guix pull: error: failed to download up-to-date source, exiting

Also, I think we should only use a default trust store when pulling from
%snapshot-url.


signature.asc
Description: PGP signature


Re: `guix pull` over HTTPS

2017-02-28 Thread Marius Bakke
Marius Bakke  writes:

> Marius Bakke  writes:
>
>> @@ -224,8 +225,11 @@ contained therein."
>>(with-error-handling
>>  (let* ((opts  (parse-options))
>> (store (open-connection))
>> +   (certs (string-append (package-output store nss-certs)
>> + "/etc/ssl/certs"))
>
> Note: This only works if you have nss-certs in the store already. Not
> sure how to convert this into a gexp.

Wait, this is false. For some reason I assumed package-output just
computed the store path, but it is in fact added to the store.

The attached patch adds a #:certificate-directory parameter and passes
it from (guix-pull) all the way down to (tls-wrap). Feedback wanted!



signature.asc
Description: PGP signature
>From a27448b259b1d2061faabe3c17433e1c660e60c9 Mon Sep 17 00:00:00 2001
From: Marius Bakke 
Date: Tue, 28 Feb 2017 22:34:29 +0100
Subject: [PATCH] pull: Default to HTTPS.

* guix/build/download.scm (tls-wrap): Add CERTIFICATE-DIRECTORY parameter.
(open-connection-for-uri): Adjust parameters to match.
(http-fetch): Likewise.
(url-fetch): Likewise.
* guix/download.scm (download-to-store): Likewise.
* guix/scripts/pull.scm (%snapshot-url): Use HTTPS.
(guix-pull): Verify against the store path of NSS-CERTS.
---
 guix/build/download.scm | 40 
 guix/download.scm   | 10 +++---
 guix/scripts/pull.scm   |  8 ++--
 3 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/guix/build/download.scm b/guix/build/download.scm
index 203338b52..2a06a 100644
--- a/guix/build/download.scm
+++ b/guix/build/download.scm
@@ -340,15 +340,20 @@ way."
 (set-exception-printer! 'tls-certificate-error
 print-tls-certificate-error)
 
-(define* (tls-wrap port server #:key (verify-certificate? #t))
+(define* (tls-wrap port server #:key (verify-certificate? #t)
+   (certificate-directory #f))
   "Return PORT wrapped in a TLS connection to SERVER.  SERVER must be a DNS
-host name without trailing dot."
+host name without trailing dot.  If CERTIFICATE-DIRECTORY is set, x509
+certificates will be verified against those found in the specified path
+instead of the default."
   (define (log level str)
 (format (current-error-port)
 "gnutls: [~a|~a] ~a" (getpid) level str))
 
   (let ((session  (make-session connection-end/client))
-(ca-certs (%x509-certificate-directory)))
+(ca-certs (if (string? certificate-directory)
+  certificate-directory
+  (%x509-certificate-directory
 
 ;; Some servers such as 'cloud.github.com' require the client to support
 ;; the 'SERVER NAME' extension.  However, 'set-session-server-name!' is
@@ -459,10 +464,12 @@ ETIMEDOUT error is raised."
 (define* (open-connection-for-uri uri
   #:key
   timeout
-  (verify-certificate? #t))
+  (verify-certificate? #t)
+  (certificate-directory #f))
   "Like 'open-socket-for-uri', but also handle HTTPS connections.  The
 resulting port must be closed with 'close-connection'.  When
-VERIFY-CERTIFICATE? is true, verify HTTPS server certificates."
+VERIFY-CERTIFICATE? is true, verify HTTPS server certificates;
+optionally against those found in CERTIFICATE-DIRECTORY."
   (define https?
 (eq? 'https (uri-scheme uri)))
 
@@ -490,7 +497,8 @@ VERIFY-CERTIFICATE? is true, verify HTTPS server certificates."
 
(if https?
(tls-wrap s (uri-host uri)
- #:verify-certificate? verify-certificate?)
+ #:verify-certificate? verify-certificate?
+ #:certificate-directory certificate-directory)
s)
 
 (define (close-connection port)
@@ -675,11 +683,13 @@ Return the resulting target URI."
 #:query(uri-queryref)
 #:fragment (uri-fragment ref)
 
-(define* (http-fetch uri file #:key timeout (verify-certificate? #t))
+(define* (http-fetch uri file #:key timeout (verify-certificate? #t)
+ (certificate-directory #f))
   "Fetch data from URI and write it to FILE; when TIMEOUT is true, bail out if
 the connection could not be established in less than TIMEOUT seconds.  Return
 FILE on success.  When VERIFY-CERTIFICATE? is true, verify HTTPS
-certificates; otherwise simply ignore them."
+certificates, optionally against those found in CERTIFICATE-DIRECTORY;
+otherwise simply ignore them."
 
   (define post-2.0.7?
 (or (> (string->number (major-version)) 2)
@@ -709,7 +719,9 @@ certificates; otherwise simply ignore them."
  (open-connection-for-uri uri
   #:timeout timeout
   #:verify-certificate?
-  verify-certificate?))
+   

Re: [PATCH] pull: Use HTTPS by default.

2017-02-28 Thread Leo Famulari
On Tue, Feb 28, 2017 at 05:39:02PM +0100, Marius Bakke wrote:
> * guix/scripts/pull.scm (%snapshot-url): Use HTTPS.
> (%options): Add "--insecure" option.

I hope that we can make it reliable enough that we don't need a special
option. In that case, users would instead pass the option
'--url=http://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz'


signature.asc
Description: PGP signature


Re: export variable CONFFLAGS

2017-02-28 Thread rennes

Hi,

On 2017-02-28 05:26, Danny Milosavljevic wrote:

Hi,

On Mon, 27 Feb 2017 23:45:36 -0600
ren...@openmailbox.org wrote:


Hello,
I am trying to export the variable 'CONFFLAGS' for 'apr' package on
GNU/Hurd as follows:

(arguments
  #:make-flags ("CONFFLAGS+=apr_cv_struct_ipmreq=no"
 (string-append "PREFIX=" %output))


This way does not work!


Why "+=" ? Just use "=".


I think it's cumulative, but I'll try as you mention.
Thanks



Re: `guix pull` over HTTPS

2017-02-28 Thread Leo Famulari
On Wed, Mar 01, 2017 at 12:05:57AM +0100, Marius Bakke wrote:
> The ISRG trust chain is supported by NSS since 3.26[0] and Firefox 50.
> 
> [0] https://bugzilla.mozilla.org/show_bug.cgi?id=1204656
> 
> As long as the ISRG chain works with all software in Guix, I don't see a
> reason to include the IdenTrust root and intermediates. I think we
> should not include the retired intermediate certificates however.
> 
> This tiny chain works for all cases I've tried:
> 
> cat isrgrootx1.pem letsencryptauthorityx3.pem letsencryptauthorityx4.pem > 
> le-certs.pem

I've updated the le-certs Git repo to include this bundle (although I
put the root certificate at the end of the list), and a separate bundle
for the IdenTrust-signed chain.

Let's use the ISRG chain.

> > 2) I'd like at least two other Guix developers to try recreating the
> > repository "from scratch", and to send signed email to this thread
> > saying that they were able to successfully recreate this custom
> > certificate store.
> 
> I downloaded each certificate independently and ran the provided 'sed'
> command and ended up with the same SHA256 hashes, which are:
> 
> 139a5e4a4e0fa505378c72c5f700934ce8333f4e6b1b508886c4b0eb14f4be99  
> dstrootx3.pem
> 3e6cf961c196c63a39bd99e5e34ff42c83669e3d7bcc2e4a0f9c7c7df40d0d7e  
> isrgrootx1.pem
> 3acb088b1372351a29c23ba51d28442a22e810224f8d009d8e026c470f463d74  le-certs.pem
> f8a8316dcc1f813774e7d7e2f85d7069d8b387c98a81b6073ef9f415be62410e  
> letsencryptauthorityx1.pem
> 3f67c48667781f7a7221320ee5b76c353aa4e0f4b2ed24a8a41113e6638f9724  
> letsencryptauthorityx2.pem
> 735a28bd5d93161769dd3a5d1d6337f24d1f2662cfe355930c1cffc38cac6a7d  
> letsencryptauthorityx3.pem
> 04f703429322d699af9e4d47e558cb696378fa20073700c9309263c448626d00  
> letsencryptauthorityx4.pem
> 6c0a324bb803e9d66b8986ea2085bb9d6bdfe33f5c04a03a3f7024f4aa8e7a2d  
> lets-encrypt-x1-cross-signed.pem
> b5791649cc21518a9757d7e1809bc47c5e60edc45c9dddaaf6c060cbe03bcb1d  
> lets-encrypt-x2-cross-signed.pem
> e446c5e9dbef9d09ac9f7027c034602492437a05ff6c40011d7235fca639c79a  
> lets-encrypt-x3-cross-signed.pem
> f524491d9c2966c01ecec75c7803c7169ff46bc5cfd44c396d418cd7053d8015  
> lets-encrypt-x4-cross-signed.pem

Thanks, that matches what I have. One more person, please :)


signature.asc
Description: PGP signature


Re: [PATCH 6/8] vm: Improve readability of run-vm.sh generation.

2017-02-28 Thread Danny Milosavljevic
Applied as 26a076ed693167dd4d501a880933a75da894a57f to master.



Re: `guix pull` over HTTPS

2017-02-28 Thread Marius Bakke
Leo Famulari  writes:

> On Tue, Feb 28, 2017 at 03:59:42PM +0100, Marius Bakke wrote:
>> For some reason setting SSL_CERT_FILE to "le-certs.pem" does not work
>> for `guix download`, but having just the one file in SSL_CERT_DIR does.
>> That's good enough for me! Could you make this into a Guix package? 
>
> I plan to make a package once these issues are resolved:
>
> 1) Which "trust path" should we use? The one using ISRG (the "native"
> Let's Encrypt root certificate authority), or the one that is
> cross-signed by IdenTrust? Or should we keep it as-is, where both are
> included? This is my first time creating a custom set of certificates,
> so I don't know all the issues.
>
> They recommend that server operators used the cross-signed trust chain
> because the ISRG trust chain is not yet widely deployed in web browsers,
> but that's not an issue for this use case.

The ISRG trust chain is supported by NSS since 3.26[0] and Firefox 50.

[0] https://bugzilla.mozilla.org/show_bug.cgi?id=1204656

As long as the ISRG chain works with all software in Guix, I don't see a
reason to include the IdenTrust root and intermediates. I think we
should not include the retired intermediate certificates however.

This tiny chain works for all cases I've tried:

cat isrgrootx1.pem letsencryptauthorityx3.pem letsencryptauthorityx4.pem > 
le-certs.pem

> 2) I'd like at least two other Guix developers to try recreating the
> repository "from scratch", and to send signed email to this thread
> saying that they were able to successfully recreate this custom
> certificate store.

I downloaded each certificate independently and ran the provided 'sed'
command and ended up with the same SHA256 hashes, which are:

139a5e4a4e0fa505378c72c5f700934ce8333f4e6b1b508886c4b0eb14f4be99  dstrootx3.pem
3e6cf961c196c63a39bd99e5e34ff42c83669e3d7bcc2e4a0f9c7c7df40d0d7e  isrgrootx1.pem
3acb088b1372351a29c23ba51d28442a22e810224f8d009d8e026c470f463d74  le-certs.pem
f8a8316dcc1f813774e7d7e2f85d7069d8b387c98a81b6073ef9f415be62410e  
letsencryptauthorityx1.pem
3f67c48667781f7a7221320ee5b76c353aa4e0f4b2ed24a8a41113e6638f9724  
letsencryptauthorityx2.pem
735a28bd5d93161769dd3a5d1d6337f24d1f2662cfe355930c1cffc38cac6a7d  
letsencryptauthorityx3.pem
04f703429322d699af9e4d47e558cb696378fa20073700c9309263c448626d00  
letsencryptauthorityx4.pem
6c0a324bb803e9d66b8986ea2085bb9d6bdfe33f5c04a03a3f7024f4aa8e7a2d  
lets-encrypt-x1-cross-signed.pem
b5791649cc21518a9757d7e1809bc47c5e60edc45c9dddaaf6c060cbe03bcb1d  
lets-encrypt-x2-cross-signed.pem
e446c5e9dbef9d09ac9f7027c034602492437a05ff6c40011d7235fca639c79a  
lets-encrypt-x3-cross-signed.pem
f524491d9c2966c01ecec75c7803c7169ff46bc5cfd44c396d418cd7053d8015  
lets-encrypt-x4-cross-signed.pem


signature.asc
Description: PGP signature


[PATCH 2/2] services: Move configuration functions that shouldn't be factorized.

2017-02-28 Thread Clément Lassieur
* gnu/services/configuration.scm (serialize-field, serialize-string)
  (serialize-space-separated-string-list, space-separated-string-list?)
  (serialize-file-name, file-name?, serialize-boolean): Move these functions...
* gnu/services/cups.scm: ...to this file.
* gnu/services/kerberos.scm: ...to this file.

Configuration syntaxes are very specific to services.  Some services may have
the same configuration syntax, but none of them is common enough to be
abstracted in configuration.scm.
---
 gnu/services/configuration.scm | 40 
 gnu/services/cups.scm  | 32 
 gnu/services/kerberos.scm  | 15 +++
 3 files changed, 47 insertions(+), 40 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index a98db64fa..2ad3a637a 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -39,14 +39,6 @@
 define-configuration
 validate-configuration
 generate-documentation
-serialize-field
-serialize-string
-serialize-name
-serialize-space-separated-string-list
-space-separated-string-list?
-serialize-file-name
-file-name?
-serialize-boolean
 serialize-package))
 
 ;;; Commentary:
@@ -140,41 +132,9 @@
#,(id #'stem #'stem #'-fields))
conf
 
-(define (uglify-field-name field-name)
-  (let ((str (symbol->string field-name)))
-(string-concatenate
- (map string-titlecase
-  (string-split (if (string-suffix? "?" str)
-(substring str 0 (1- (string-length str)))
-str)
-#\-)
-
-(define (serialize-field field-name val)
-  (format #t "~a ~a\n" (uglify-field-name field-name) val))
-
 (define (serialize-package field-name val)
   #f)
 
-(define (serialize-string field-name val)
-  (serialize-field field-name val))
-
-(define (space-separated-string-list? val)
-  (and (list? val)
-   (and-map (lambda (x)
-  (and (string? x) (not (string-index x #\space
-val)))
-(define (serialize-space-separated-string-list field-name val)
-  (serialize-field field-name (string-join val " ")))
-
-(define (file-name? val)
-  (and (string? val)
-   (string-prefix? "/" val)))
-(define (serialize-file-name field-name val)
-  (serialize-string field-name val))
-
-(define (serialize-boolean field-name val)
-  (serialize-string field-name (if val "yes" "no")))
-
 ;; A little helper to make it easier to document all those fields.
 (define (generate-documentation documentation documentation-name)
   (define (str x) (object->string x))
diff --git a/gnu/services/cups.scm b/gnu/services/cups.scm
index 70b858479..70a71eff0 100644
--- a/gnu/services/cups.scm
+++ b/gnu/services/cups.scm
@@ -57,6 +57,21 @@
  (home-directory "/var/empty")
  (shell (file-append shadow "/sbin/nologin")
 
+(define (uglify-field-name field-name)
+  (let ((str (symbol->string field-name)))
+(string-concatenate
+ (map string-titlecase
+  (string-split (if (string-suffix? "?" str)
+(substring str 0 (1- (string-length str)))
+str)
+#\-)
+
+(define (serialize-field field-name val)
+  (format #t "~a ~a\n" (uglify-field-name field-name) val))
+
+(define (serialize-string field-name val)
+  (serialize-field field-name val))
+
 (define (multiline-string-list? val)
   (and (list? val)
(and-map (lambda (x)
@@ -65,11 +80,28 @@
 (define (serialize-multiline-string-list field-name val)
   (for-each (lambda (str) (serialize-field field-name str)) val))
 
+(define (space-separated-string-list? val)
+  (and (list? val)
+   (and-map (lambda (x)
+  (and (string? x) (not (string-index x #\space
+val)))
+(define (serialize-space-separated-string-list field-name val)
+  (serialize-field field-name (string-join val " ")))
+
 (define (space-separated-symbol-list? val)
   (and (list? val) (and-map symbol? val)))
 (define (serialize-space-separated-symbol-list field-name val)
   (serialize-field field-name (string-join (map symbol->string val) " ")))
 
+(define (file-name? val)
+  (and (string? val)
+   (string-prefix? "/" val)))
+(define (serialize-file-name field-name val)
+  (serialize-string field-name val))
+
+(define (serialize-boolean field-name val)
+  (serialize-string field-name (if val "yes" "no")))
+
 (define (non-negative-integer? val)
   (and (exact-integer? val) (not (negative? val
 (define (serialize-non-negative-integer field-name val)
diff --git a/gnu/services/kerberos.scm b/gnu/services/kerberos.scm
index cb33a7c53..f09f47893 100644
--- a/gnu/services/kerberos.scm
+++ b/gnu/services/kerberos.scm
@@ -96,6 +96,12 @@ trail

[PATCH 0/2] Services configuration syntaxes patches.

2017-02-28 Thread Clément Lassieur
There was a bug in the dovecot service introduced by a refactoring of syntaxes
functions.  I don't think those functions should be put in a common file,
because syntaxes are often specific to services, as I explained in one of the
commit messages.

Therefore I did two commits.  The first fixes the bug.  The other removes
functions that should not be abstracted from configuration.scm, and put them
back into their specific services.

The second commit should not have any actual effect, but I checked that
services depending on configuration.scm were still working and:
  - Dovecot works (because of the first commit).
  - Prosody works (no changes).
  - Cups works.
  - I don't know how to test Kerberos, but its configuration file didn't move.
  - OpenVPN does not build when put in config.scm, because of a tls issue, so
I could not check (but I didn't need to modify it).

WDYT?

Clément Lassieur (2):
  services: dovecot: Reimplement proper configuration functions.
  services: Move configuration functions that shouldn't be factorized.

 gnu/services/configuration.scm | 40 
 gnu/services/cups.scm  | 32 
 gnu/services/kerberos.scm  | 15 +++
 gnu/services/mail.scm  | 30 ++
 4 files changed, 77 insertions(+), 40 deletions(-)

-- 
2.12.0




[PATCH 1/2] services: dovecot: Reimplement proper configuration functions.

2017-02-28 Thread Clément Lassieur
* gnu/services/mail.scm (uglify-field-name, serialize-field, serialize-string)
  (space-separated-string-list?, serialize-space-separated-string-list)
  (file-name?, serialize-file-name, serialize-boolean): Add them.

These functions were inadvertently changed while being factorized in
gnu/service/configuration.scm.
---
 gnu/services/mail.scm | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index 30b1672d3..8b75134a7 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -62,6 +62,27 @@
 ;;;
 ;;; Code:
 
+(define (uglify-field-name field-name)
+  (let ((str (symbol->string field-name)))
+(string-join (string-split (if (string-suffix? "?" str)
+   (substring str 0 (1- (string-length str)))
+   str)
+   #\-)
+ "_")))
+
+(define (serialize-field field-name val)
+  (format #t "~a=~a\n" (uglify-field-name field-name) val))
+
+(define (serialize-string field-name val)
+  (serialize-field field-name val))
+
+(define (space-separated-string-list? val)
+  (and (list? val)
+   (and-map (lambda (x)
+  (and (string? x) (not (string-index x #\space
+val)))
+(define (serialize-space-separated-string-list field-name val)
+  (serialize-field field-name (string-join val " ")))
 
 (define (comma-separated-string-list? val)
   (and (list? val)
@@ -71,6 +92,12 @@
 (define (serialize-comma-separated-string-list field-name val)
   (serialize-field field-name (string-join val ",")))
 
+(define (file-name? val)
+  (and (string? val)
+   (string-prefix? "/" val)))
+(define (serialize-file-name field-name val)
+  (serialize-string field-name val))
+
 (define (colon-separated-file-name-list? val)
   (and (list? val)
;; Trailing slashes not needed and not
@@ -78,6 +105,9 @@
 (define (serialize-colon-separated-file-name-list field-name val)
   (serialize-field field-name (string-join val ":")))
 
+(define (serialize-boolean field-name val)
+  (serialize-string field-name (if val "yes" "no")))
+
 (define (non-negative-integer? val)
   (and (exact-integer? val) (not (negative? val
 (define (serialize-non-negative-integer field-name val)
-- 
2.12.0




Re: [PATCH 5/8] vm: Remove hard coded kernel file name.

2017-02-28 Thread Danny Milosavljevic
Applied as 43fe431cce107bd311a68dea59ac0f672ac13615 to master.



Re: `guix pull` over HTTPS

2017-02-28 Thread Marius Bakke
Marius Bakke  writes:

> @@ -224,8 +225,11 @@ contained therein."
>(with-error-handling
>  (let* ((opts  (parse-options))
> (store (open-connection))
> +   (certs (string-append (package-output store nss-certs)
> + "/etc/ssl/certs"))

Note: This only works if you have nss-certs in the store already. Not
sure how to convert this into a gexp.

> (url   (assoc-ref opts 'tarball-url)))
> -  (let ((tarball (download-to-store store url "guix-latest.tar.gz")))
> +  (let ((tarball (download-to-store store url "guix-latest.tar.gz"
> +#:verify-certificate? certs)))
>  (unless tarball
>(leave (_ "failed to download up-to-date source, exiting\n")))
>  (parameterize ((%guile-for-build
> -- 
> 2.12.0


signature.asc
Description: PGP signature


Re: `guix pull` over HTTPS

2017-02-28 Thread Marius Bakke
Marius Bakke  writes:

>>> I want to bundle a 'le-certs' package with GNU Guix, and change `guix
>>> pull` to know to use the le-certs bundle when pulling from
>>> %snapshot-url. For other URLs, users will have to take care of it
>>> themselves. 
>>
>> This sounds like a better approach. Also, I did not see this email
>> before sending the patch! If you package it up, I can look into
>> realizing the package in `guix pull` directly.
>
> I gave this a go using "nss-certs", but can't figure out how to set
> SSL_CERT_DIR (or GUIX_TLS_CERTIFICATE_DIRECTORY) in `guix pull`. The
> naive approach of setting the variable before calling
> "download-to-store" does not work because %x509-certificate-directory
> has already been evaluated.
>
> I wonder what's the best approach here. Parameterizing this and
> propagating it all the way down to (tls-wrap) similar to
> #:verify-certificate? could work, but seems awkward. Any suggestions?

I made it work with the attached hack. It breaks all conventions by
allowing #:verify-certificate? to be a search path for certificates.

If it wasn't for the implied boolean nature of "#:verify-certificate?" I
would be happy with this solution. But I think setting the
GUIX_TLS_CERTIFICATE_DIRECTORY environment variable before pulling in
(guix download) would be better.



signature.asc
Description: PGP signature
>From 800051909362b5817bbb386029edf14ffd8269a8 Mon Sep 17 00:00:00 2001
From: Marius Bakke 
Date: Tue, 28 Feb 2017 22:34:29 +0100
Subject: [PATCH] pull: Default to HTTPS.

* guix/build/download.scm (tls-wrap): Allow #:verify-certificate? to be a
  search string for certificates.
* guix/scripts/pull.scm (%snapshot-url): Use HTTPS.
(guix-pull): Verify against the store path of NSS-CERTS.
---
 guix/build/download.scm | 7 +--
 guix/scripts/pull.scm   | 8 ++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/guix/build/download.scm b/guix/build/download.scm
index 203338b52..88da1776f 100644
--- a/guix/build/download.scm
+++ b/guix/build/download.scm
@@ -342,13 +342,16 @@ way."
 
 (define* (tls-wrap port server #:key (verify-certificate? #t))
   "Return PORT wrapped in a TLS connection to SERVER.  SERVER must be a DNS
-host name without trailing dot."
+host name without trailing dot.  If VERIFY-CERTIFICATE? is a string, it is
+assumed to be the search path for TLS certificates passed to gnutls."
   (define (log level str)
 (format (current-error-port)
 "gnutls: [~a|~a] ~a" (getpid) level str))
 
   (let ((session  (make-session connection-end/client))
-(ca-certs (%x509-certificate-directory)))
+(ca-certs (if (string? verify-certificate?)
+  verify-certificate?
+  (%x509-certificate-directory
 
 ;; Some servers such as 'cloud.github.com' require the client to support
 ;; the 'SERVER NAME' extension.  However, 'set-session-server-name!' is
diff --git a/guix/scripts/pull.scm b/guix/scripts/pull.scm
index a4824e4fd..402332192 100644
--- a/guix/scripts/pull.scm
+++ b/guix/scripts/pull.scm
@@ -30,6 +30,7 @@
   #:use-module ((guix build utils)
 #:select (with-directory-excursion delete-file-recursively))
   #:use-module (gnu packages base)
+  #:use-module ((gnu packages certs) #:select (nss-certs))
   #:use-module (gnu packages guile)
   #:use-module ((gnu packages bootstrap)
 #:select (%bootstrap-guile))
@@ -45,7 +46,7 @@
 
 (define %snapshot-url
   ;; "http://hydra.gnu.org/job/guix/master/tarball/latest/download";
-  "http://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz";
+  "https://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz";
   )
 
 (define-syntax-rule (with-environment-variable variable value body ...)
@@ -224,8 +225,11 @@ contained therein."
   (with-error-handling
 (let* ((opts  (parse-options))
(store (open-connection))
+   (certs (string-append (package-output store nss-certs)
+ "/etc/ssl/certs"))
(url   (assoc-ref opts 'tarball-url)))
-  (let ((tarball (download-to-store store url "guix-latest.tar.gz")))
+  (let ((tarball (download-to-store store url "guix-latest.tar.gz"
+#:verify-certificate? certs)))
 (unless tarball
   (leave (_ "failed to download up-to-date source, exiting\n")))
 (parameterize ((%guile-for-build
-- 
2.12.0



Re: `guix pull` over HTTPS

2017-02-28 Thread Marius Bakke

>> I want to bundle a 'le-certs' package with GNU Guix, and change `guix
>> pull` to know to use the le-certs bundle when pulling from
>> %snapshot-url. For other URLs, users will have to take care of it
>> themselves. 
>
> This sounds like a better approach. Also, I did not see this email
> before sending the patch! If you package it up, I can look into
> realizing the package in `guix pull` directly.

I gave this a go using "nss-certs", but can't figure out how to set
SSL_CERT_DIR (or GUIX_TLS_CERTIFICATE_DIRECTORY) in `guix pull`. The
naive approach of setting the variable before calling
"download-to-store" does not work because %x509-certificate-directory
has already been evaluated.

I wonder what's the best approach here. Parameterizing this and
propagating it all the way down to (tls-wrap) similar to
#:verify-certificate? could work, but seems awkward. Any suggestions?


signature.asc
Description: PGP signature


Re: `guix pull` over HTTPS

2017-02-28 Thread Marius Bakke
Leo Famulari  writes:

> On Tue, Feb 28, 2017 at 03:59:42PM +0100, Marius Bakke wrote:
>> For some reason setting SSL_CERT_FILE to "le-certs.pem" does not work
>> for `guix download`, but having just the one file in SSL_CERT_DIR does.
>> That's good enough for me! Could you make this into a Guix package? 
>
> I plan to make a package once these issues are resolved:
>
> 1) Which "trust path" should we use? The one using ISRG (the "native"
> Let's Encrypt root certificate authority), or the one that is
> cross-signed by IdenTrust? Or should we keep it as-is, where both are
> included? This is my first time creating a custom set of certificates,
> so I don't know all the issues.
>
> They recommend that server operators used the cross-signed trust chain
> because the ISRG trust chain is not yet widely deployed in web browsers,
> but that's not an issue for this use case.

I don't fully understand the differences here, but will do some reading.

> 2) I'd like at least two other Guix developers to try recreating the
> repository "from scratch", and to send signed email to this thread
> saying that they were able to successfully recreate this custom
> certificate store.

I will do this later.

>> I wonder what happens if we simply switch %snapshot-url to HTTPS in
>> `guix/scripts/pull.scm`. How many users do not have SSL_CERT_DIR
>> configured? I think it would be sufficient to mention in the manual to
>> install one of "nss-certs" or "le-certs" before running `guix pull` for
>> the first time. How does that sound?
>
> I think it's too much of a regression if users have to fiddle with
> environment variables for `guix pull` to work reliably. People are
> constantly asking for help with environment variables in the #guix chat
> room.
>
> I want to bundle a 'le-certs' package with GNU Guix, and change `guix
> pull` to know to use the le-certs bundle when pulling from
> %snapshot-url. For other URLs, users will have to take care of it
> themselves. 

This sounds like a better approach. Also, I did not see this email
before sending the patch! If you package it up, I can look into
realizing the package in `guix pull` directly.

> This should preserve the existing user experience of `guix pull`, which
> is that the default invocation "just works", at least in terms of
> downloading the source code. It could fail anyways if their clock is way
> off... any other ideas about how it could fail?

Not off the top of my head. But see the patch for a fallback
"--insecure" option if all else fails.

Thanks a lot for taking this on!


signature.asc
Description: PGP signature


[PATCH] pull: Use HTTPS by default.

2017-02-28 Thread Marius Bakke
* guix/scripts/pull.scm (%snapshot-url): Use HTTPS.
(%options): Add "--insecure" option.
(show-help): Mention it.
(guix-pull): Pass #:verify-certificate to DOWNLOAD-TO-STORE.
---
 guix/scripts/pull.scm | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/pull.scm b/guix/scripts/pull.scm
index a4824e4fd..b1724f13c 100644
--- a/guix/scripts/pull.scm
+++ b/guix/scripts/pull.scm
@@ -45,7 +45,7 @@
 
 (define %snapshot-url
   ;; "http://hydra.gnu.org/job/guix/master/tarball/latest/download";
-  "http://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz";
+  "https://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz";
   )
 
 (define-syntax-rule (with-environment-variable variable value body ...)
@@ -78,6 +78,8 @@ Download and deploy the latest version of Guix.\n"))
   (display (_ "
   --url=URL  download the Guix tarball from URL"))
   (display (_ "
+  --insecure do not perform validation of TLS certificates"))
+  (display (_ "
   --bootstrapuse the bootstrap Guile to build the new Guix"))
   (newline)
   (display (_ "
@@ -96,6 +98,9 @@ Download and deploy the latest version of Guix.\n"))
 (lambda (opt name arg result)
   (alist-cons 'tarball-url arg
   (alist-delete 'tarball-url result
+(option '("insecure") #f #f
+(lambda (opt name arg result)
+  (alist-cons 'insecure? #t result)))
 (option '("bootstrap") #f #f
 (lambda (opt name arg result)
   (alist-cons 'bootstrap? #t result)))
@@ -225,7 +230,9 @@ contained therein."
 (let* ((opts  (parse-options))
(store (open-connection))
(url   (assoc-ref opts 'tarball-url)))
-  (let ((tarball (download-to-store store url "guix-latest.tar.gz")))
+  (let ((tarball (download-to-store store url "guix-latest.tar.gz"
+#:verify-certificate?
+(not (assoc-ref opts 'insecure?)
 (unless tarball
   (leave (_ "failed to download up-to-date source, exiting\n")))
 (parameterize ((%guile-for-build
-- 
2.12.0




Re: `guix pull` over HTTPS

2017-02-28 Thread Leo Famulari
On Tue, Feb 28, 2017 at 03:59:42PM +0100, Marius Bakke wrote:
> For some reason setting SSL_CERT_FILE to "le-certs.pem" does not work
> for `guix download`, but having just the one file in SSL_CERT_DIR does.
> That's good enough for me! Could you make this into a Guix package? 

I plan to make a package once these issues are resolved:

1) Which "trust path" should we use? The one using ISRG (the "native"
Let's Encrypt root certificate authority), or the one that is
cross-signed by IdenTrust? Or should we keep it as-is, where both are
included? This is my first time creating a custom set of certificates,
so I don't know all the issues.

They recommend that server operators used the cross-signed trust chain
because the ISRG trust chain is not yet widely deployed in web browsers,
but that's not an issue for this use case.

2) I'd like at least two other Guix developers to try recreating the
repository "from scratch", and to send signed email to this thread
saying that they were able to successfully recreate this custom
certificate store.

> I wonder what happens if we simply switch %snapshot-url to HTTPS in
> `guix/scripts/pull.scm`. How many users do not have SSL_CERT_DIR
> configured? I think it would be sufficient to mention in the manual to
> install one of "nss-certs" or "le-certs" before running `guix pull` for
> the first time. How does that sound?

I think it's too much of a regression if users have to fiddle with
environment variables for `guix pull` to work reliably. People are
constantly asking for help with environment variables in the #guix chat
room.

I want to bundle a 'le-certs' package with GNU Guix, and change `guix
pull` to know to use the le-certs bundle when pulling from
%snapshot-url. For other URLs, users will have to take care of it
themselves. 

This should preserve the existing user experience of `guix pull`, which
is that the default invocation "just works", at least in terms of
downloading the source code. It could fail anyways if their clock is way
off... any other ideas about how it could fail?

> $ CURL_CA_BUNDLE=/tmp/le-certs/le-certs.pem curl -sv https://nrk.no > 
> /dev/null
> * Rebuilt URL to: https://nrk.no/
> *   Trying 160.68.205.231...
> * TCP_NODELAY set
> * Connected to nrk.no (160.68.205.231) port 443 (#0)
> * found 10 certificates in /tmp/le-certs/le-certs.pem
> * ALPN, offering http/1.1
> * SSL connection using TLS1.2 / ECDHE_RSA_AES_256_GCM_SHA384
> * server certificate verification failed. CAfile: /tmp/le-certs/le-certs.pem 
> CRLfile: none
> * Closing connection 0
> 
> $ CURL_CA_BUNDLE=/tmp/le-certs/le-certs.pem curl -sv https://gnu.org > 
> /dev/null
> * Rebuilt URL to: https://gnu.org/
> *   Trying 208.118.235.148...
> * TCP_NODELAY set
> * Connected to gnu.org (208.118.235.148) port 443 (#0)
> * found 10 certificates in /tmp/le-certs/le-certs.pem
> * ALPN, offering http/1.1
> * SSL connection using TLS1.2 / ECDHE_RSA_AES_128_GCM_SHA256
> *server certificate verification OK
> *server certificate status verification SKIPPED
> *common name: gnu.org (matched)
> *server certificate expiration date OK
> *server certificate activation date OK
> *certificate public key: RSA
> *certificate version: #3
> *subject: CN=gnu.org
> *start date: Wed, 15 Feb 2017 10:01:00 GMT
> *expire date: Tue, 16 May 2017 10:01:00 GMT
> *issuer: C=US,O=Let's Encrypt,CN=Let's Encrypt Authority X3
> *compression: NULL
> 
> $ GIT_SSL_CAINFO="" git clone --depth=1 
> https://git.savannah.gnu.org/git/guix.git
> Cloning into 'guix'...
> fatal: unable to access 'https://git.savannah.gnu.org/git/guix.git/': Problem 
> with the SSL CA cert(path? access rights?)
> 
> $ GIT_SSL_CAINFO=/tmp/le-certs/le-certs.pem git clone --depth=1 
> https://git.savannah.gnu.org/git/guix.git
> Cloning into 'guix'...
> remote: Counting objects: 1409, done.

Excellent :)


signature.asc
Description: PGP signature


Re: `guix pull` over HTTPS

2017-02-28 Thread Marius Bakke
Leo Famulari  writes:

> On Sat, Feb 11, 2017 at 03:28:52PM +0100, Ludovic Courtès wrote:
>> Marius Bakke  skribis:
>> > I think having a separate 'le-certs' package that can verify the Lets
>> > Encrypt chain sounds like the easiest option. Presumably new
>> > intermediates etc will be known well in advance.
>> 
>> That sounds more reasonable to me.  Do you know what it would take to
>> get the whole LE chain in such a package?  Would you like to give it a
>> try?
>
> I tried it. The next intermediate (also called the "backup") is already
> known.
>
> I've made it available here:
>
> https://github.com/lfam/le-certs
>
> You can try it out:
>
> $ echo | openssl s_client -CAfile /tmp/le-certs/le-certs.pem -CApath 
> /tmp/le-certs -connect git.savannah.gnu.org:443
>
> Your feedback is requested!

Wow, this is cool!

$ SSL_CERT_FILE="" SSL_CERT_DIR=""  guix pull 
--url=https://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz 
   
Starting download of /tmp/guix-file.7U65Ts
From https://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz...
ERROR: X.509 certificate of 'git.savannah.gnu.org' could not be verified:
  signer-not-found
  invalid

SSL_CERT_FILE="" SSL_CERT_DIR="/tmp/le-certs/"  guix pull 
--url=https://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz
Starting download of /tmp/guix-file.wOblWP
From https://git.savannah.gnu.org/cgit/guix.git/snapshot/master.tar.gz...
 ….tar.gz   1.0MiB/s 00:11 | 11.1MiB transferred
unpacking '/gnu/store/p0gbr83a4g9qlk59vvxkw8gvrv1z8cnw-guix-latest.tar.gz'...

For some reason setting SSL_CERT_FILE to "le-certs.pem" does not work
for `guix download`, but having just the one file in SSL_CERT_DIR does.
That's good enough for me! Could you make this into a Guix package? 

I wonder what happens if we simply switch %snapshot-url to HTTPS in
`guix/scripts/pull.scm`. How many users do not have SSL_CERT_DIR
configured? I think it would be sufficient to mention in the manual to
install one of "nss-certs" or "le-certs" before running `guix pull` for
the first time. How does that sound?

These certs are valid until at least 2020, so using a Guix release
snapshot of this package should work for a long time.

Some other tests:

$ CURL_CA_BUNDLE=/tmp/le-certs/le-certs.pem curl -sv https://nrk.no > /dev/null
* Rebuilt URL to: https://nrk.no/
*   Trying 160.68.205.231...
* TCP_NODELAY set
* Connected to nrk.no (160.68.205.231) port 443 (#0)
* found 10 certificates in /tmp/le-certs/le-certs.pem
* ALPN, offering http/1.1
* SSL connection using TLS1.2 / ECDHE_RSA_AES_256_GCM_SHA384
* server certificate verification failed. CAfile: /tmp/le-certs/le-certs.pem 
CRLfile: none
* Closing connection 0

$ CURL_CA_BUNDLE=/tmp/le-certs/le-certs.pem curl -sv https://gnu.org > /dev/null
* Rebuilt URL to: https://gnu.org/
*   Trying 208.118.235.148...
* TCP_NODELAY set
* Connected to gnu.org (208.118.235.148) port 443 (#0)
* found 10 certificates in /tmp/le-certs/le-certs.pem
* ALPN, offering http/1.1
* SSL connection using TLS1.2 / ECDHE_RSA_AES_128_GCM_SHA256
*server certificate verification OK
*server certificate status verification SKIPPED
*common name: gnu.org (matched)
*server certificate expiration date OK
*server certificate activation date OK
*certificate public key: RSA
*certificate version: #3
*subject: CN=gnu.org
*start date: Wed, 15 Feb 2017 10:01:00 GMT
*expire date: Tue, 16 May 2017 10:01:00 GMT
*issuer: C=US,O=Let's Encrypt,CN=Let's Encrypt Authority X3
*compression: NULL

$ GIT_SSL_CAINFO="" git clone --depth=1 
https://git.savannah.gnu.org/git/guix.git
Cloning into 'guix'...
fatal: unable to access 'https://git.savannah.gnu.org/git/guix.git/': Problem 
with the SSL CA cert(path? access rights?)

$ GIT_SSL_CAINFO=/tmp/le-certs/le-certs.pem git clone --depth=1 
https://git.savannah.gnu.org/git/guix.git
Cloning into 'guix'...
remote: Counting objects: 1409, done.



signature.asc
Description: PGP signature


Re: [PATCH 0/5] gnu/packages/aux-files

2017-02-28 Thread Alex Kost
Ricardo Wurmus (2017-02-27 16:07 +0100) wrote:

> Alex Kost  writes:
>
>>> IIUC Ludovic is AFK now, so I would like to push these patches in a week
>>> or so, if there will be no comments.
>>
>> Since there were no comments, I applied this patchset to master.
>
> I missed this patch set the first time around and only just went over
> it.  It looks good to me.  Thanks!

Thanks, I decided that I would just go on with it in a hope it wouldn't
break things :-)

-- 
Alex



Re: export variable CONFFLAGS

2017-02-28 Thread Danny Milosavljevic
Hi,

On Mon, 27 Feb 2017 23:45:36 -0600
ren...@openmailbox.org wrote:

> Hello,
> I am trying to export the variable 'CONFFLAGS' for 'apr' package on 
> GNU/Hurd as follows:
> 
> (arguments
>   #:make-flags ("CONFFLAGS+=apr_cv_struct_ipmreq=no"
>  (string-append "PREFIX=" %output))
> 
> 
> This way does not work!

Why "+=" ? Just use "=".



Re: gnu-patches back log

2017-02-28 Thread Hartmut Goebel
Am 28.02.2017 um 07:25 schrieb Pjotr Prins:
> Would it be an idea to send out weekly E-mails with patches that had
> no attention to a select list of reviewers? Or maybe to the ML as a
> whole? Basically it would read:

This might be a good idea.

Please mind adding links to that mail so one can easily access the
patches and the "reports". This would make occasional reviewers live
much easier.



-- 
Regards
Hartmut Goebel

| Hartmut Goebel  | h.goe...@crazy-compilers.com   |
| www.crazy-compilers.com | compilers which you thought are impossible |




Re: Key 69096DFDD7028BEDACC5884BC5E051C79C0BECDB "Key has been compromised"

2017-02-28 Thread David Craven
Hi Leo

> Can you clarify whether the key was deleted inadvertently, or if you
> think it was actually "compromised". To me, key compromise means you
> believe that the private key could have been copied by a 3rd party.

Key revocation certificates are generated before something happens.

It is possible, but unlikely that someone obtained my key file. There is
a strong password on it however, and I'm not aware of anyone using my
key. The signature should be valid for all before the revocation date and
for none after.

David



Re: PHP on core-updates

2017-02-28 Thread Julien Lepiller
On Mon, 27 Feb 2017 16:09:58 -0500
Leo Famulari  wrote:

> On Mon, Feb 27, 2017 at 11:24:42AM +0100, julien lepiller wrote:
> > I think php-for-gd would break with the new version (it would fail
> > to apply the patches), so I'd like to get rid of it, and use our gd
> > package instead.  
> 
> Okay, sounds good!
> 
> > I found an issue with gd that was discovered by php, reported to gd
> > and fixed in their repo. I've extracted a patch from the gd repo
> > and would like to add it to our package. I found a way to "fix" the
> > newly failing tests and reported them upstream. I've attached two
> > patches I'd like to see in core-updates. I haven't tested them yet,
> > though, but comments would be highly appreciated ;)  
> 
> It's expected that contributors test their submissions before sending
> them, or put a note in the message subject like "WIP". Please let us
> know the result of your test :)

It was going to take some time to build all dependencies, but I
wanted feedback quickly. Ok for the "WIP" thing, I'll think about that
next time. I've tried to build php and failed because it depends on glib
that fails to build on core-updates because one test hangs.

> 
> > From 6ee0afac1c72c8e92dcd0384090ead62d5e0cf8a Mon Sep 17 00:00:00
> > 2001 From: Julien Lepiller 
> > Date: Mon, 27 Feb 2017 11:09:11 +0100
> > Subject: [PATCH 1/2] gnu: gd: Fix an issue with XBM decoding.
> > 
> > * gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch: New
> > file.
> > * gnu/local.scm (dist_patch_DATA): Add it.
> > * gnu/packages/gd.scm (gd)[source]: Use it.  
> 
> [...]
> 
> > diff --git
> > a/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch
> > b/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch new
> > file mode 100644 index 000..bfaa4ff --- /dev/null
> > +++ b/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch
> > @@ -0,0 +1,114 @@
> > +From 082c5444838ea0d84f9fb6441aefdb44d78d9bba Mon Sep 17 00:00:00
> > 2001 +From: "Christoph M. Becker" 
> > +Date: Fri, 20 Jan 2017 22:48:20 +0100
> > +Subject: [PATCH] Fix #109: XBM reading fails with printed error
> > +
> > +When calculating the number of required bytes of an XBM image, we
> > have +to take the line padding into account.
> > +
> > +This patch has been taken from the gd repository and binary files
> > have been +removed from the patch because our patch procedure
> > doesn't support that format. +---
> > + src/gd_xbm.c |   2 +-
> > + tests/xbm/CMakeLists.txt |   1 +
> > + tests/xbm/Makemodule.am  |   5 -
> > + tests/xbm/github_bug_109.c   |  35
> > +++
> > + tests/xbm/github_bug_109.xbm |   5 +
> > + 5 files changed, 47 insertions(+), 2 deletions(-)
> > + create mode 100644 tests/xbm/github_bug_109.c
> > + create mode 100644 tests/xbm/github_bug_109.xbm
> > + create mode 100644 tests/xbm/github_bug_109_exp.png  
> 
> We should always include links to the upstream bug report (if one
> exists) and source of the 3rd-party patch in the patch file that is
> being added (in this case, in
> 'gd-php-73968-Fix-109-XBM-reading.patch'). This makes it more likely
> that people will review it. I'm sure you can imagine that it's
> exhausting to hunt down the origin and reasoning behind every
> 3rd-party patch.

The php bug report is this one: https://bugs.php.net/bug.php?id=73968.

> 
> I guess these are some relevant bug reports (I did not find the PHP
> report): https://bitbucket.org/libgd/gd-libgd/issues/109
> https://github.com/libgd/libgd/issues/109
> 
> And the source of the patch is this:
> https://github.com/libgd/libgd/commit/082c5444838ea0d84f9fb6441aefdb44d78d9bba
> 
> Do you know if this change have a security impact? Is XBM still used
> widely?

It was not marked as a security bug in php or gd, so I don't think so.
That patch is required for a test not to fail in php though. If you
don't like having patches around, I could also disable the test in php.