Patchwork go@1.4 Updated patch

login
register
mail settings
Submitter Ludovic Courtès
Date June 12, 2016, 7:22 p.m.
Message ID <877fdukya9.fsf@gnu.org>
Download mbox | patch
Permalink /patch/13005/
State New
Headers show

Comments

Ludovic Courtès - June 12, 2016, 7:22 p.m.
Hi!

It seems we’re almost done!  I was about to commit it with the changes
below on top of the patch you posted (they are mostly cosmetic.)

Since there are parts I don’t fully understand, I thought I’d leave it
up to you.  In particular, could you could at least replace the “why” in
the comments below with explanations of why this is done?

Also, why do we need this “src” output?  IIUC, it contains the source of
some of the unit tests, right?

Thank you for persevering in spite of all the nitpicks!  :-)

Ludo’.
Ludovic Courtès - June 14, 2016, 12:45 p.m.
ludo@gnu.org (Ludovic Courtès) skribis:

> It seems we’re almost done!  I was about to commit it with the changes
> below on top of the patch you posted (they are mostly cosmetic.)
>
> Since there are parts I don’t fully understand, I thought I’d leave it
> up to you.  In particular, could you could at least replace the “why” in
> the comments below with explanations of why this is done?
>
> Also, why do we need this “src” output?  IIUC, it contains the source of
> some of the unit tests, right?

One last thing I forgot to mention: building with --rounds=2 shows that
the build is not reproducible (some differences in a few .a files.)

If you prefer, we can address it in a later patch, though.

Thanks,
Ludo’.
Matthew Jordan - June 26, 2016, 10:17 p.m.
Good Day,

> One last thing I forgot to mention: building with --rounds=2 shows that
> the build is not reproducible (some differences in a few .a files.)
>
> If you prefer, we can address it in a later patch, though.

Yes I would prefer address this issue in a later patch.

Respectuflly,
Ludovic Courtès - June 27, 2016, 12:13 p.m.
Matthew Jordan <matthewjordandevops@yandex.com> skribis:

>> One last thing I forgot to mention: building with --rounds=2 shows that
>> the build is not reproducible (some differences in a few .a files.)
>>
>> If you prefer, we can address it in a later patch, though.
>
> Yes I would prefer address this issue in a later patch.

Sure, let’s keep that in mind and address it later (but not too late
;-)).

I also noticed the unusual file system layout in the main output:

--8<---------------cut here---------------start------------->8---
$ ls $(./pre-inst-env guix build go)
/gnu/store/05al7hhwng4a5hs1ix2i4agqzm9cs4gy-go-1.4.3-doc:
share

/gnu/store/6f6ns5c7gfr79pl190r9xjyx6rbx1vv1-go-1.4.3-tests:
share

/gnu/store/873c46bpppjssni07xk1j8r10ky4ybwz-go-1.4.3:
bin  favicon.ico  include  lib	misc  pkg  src
--8<---------------cut here---------------end--------------->8---

It would be nice to move misc/, pkg/, src/, and favicon.ico to
conventional directories (lib/ or share/ I guess.)

Thanks,
Ludo’.
Matthew Jordan - June 28, 2016, 12:02 a.m.
> I also noticed the unusual file system layout in the main output:
>
> --8<---------------cut here---------------start------------->8---
> $ ls $(./pre-inst-env guix build go)
> /gnu/store/05al7hhwng4a5hs1ix2i4agqzm9cs4gy-go-1.4.3-doc:
> share
>
> /gnu/store/6f6ns5c7gfr79pl190r9xjyx6rbx1vv1-go-1.4.3-tests:
> share
>
> /gnu/store/873c46bpppjssni07xk1j8r10ky4ybwz-go-1.4.3:
> bin  favicon.ico  include  lib	misc  pkg  src
> --8<---------------cut here---------------end--------------->8---
>
> It would be nice to move misc/, pkg/, src/, and favicon.ico to
> conventional directories (lib/ or share/ I guess.)

Okay will look into that next, before submitting the next golang version.

Patch

diff --git a/gnu/packages/golang.scm b/gnu/packages/golang.scm
index 5652bbd..dbb8283 100644
--- a/gnu/packages/golang.scm
+++ b/gnu/packages/golang.scm
@@ -1,5 +1,5 @@ 
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2015, 2016 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2016 Matthew Jordan <matthewjordandevops@yandex.com>
 ;;; Copyright © 2016 Andy Wingo <wingo@igalia.com>
 ;;;
@@ -23,11 +23,10 @@ 
   #:use-module (guix utils)
   #:use-module (guix download)
   #:use-module (guix packages)
-  #:use-module (guix build utils)
   #:use-module (guix build-system gnu)
   #:use-module (gnu packages admin)
   #:use-module (gnu packages gcc)
-  #:use-module ((gnu packages base) #:prefix system:)
+  #:use-module (gnu packages base)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages pcre)
@@ -46,18 +45,17 @@ 
   (package
     (name "go")
     (version "1.4.3")
-    (source
-     (origin
-       (method url-fetch)
-       (uri (string-append "https://storage.googleapis.com/golang/"
-                           name version ".src.tar.gz"))
-       (sha256
-        (base32
-         "0na9yqilzpvq0bjndbibfp07wr796gf252y471cip10bbdqgqiwr"))))
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "https://storage.googleapis.com/golang/"
+                                  name version ".src.tar.gz"))
+              (sha256
+               (base32
+                "0na9yqilzpvq0bjndbibfp07wr796gf252y471cip10bbdqgqiwr"))))
     (build-system gnu-build-system)
     (outputs '("out"
                "doc"
-               "src"))
+               "src"))                            ;What's this?
     (arguments
      `(#:modules ((ice-9 match)
                   (guix build gnu-build-system)
@@ -68,30 +66,27 @@ 
          (delete 'configure)
          (add-after 'patch-generated-file-shebangs 'chdir
            (lambda _ (chdir "src")))
-         (add-before 'build 'prebuild
+         (add-before 'build 'pre-build
            (lambda* (#:key inputs outputs #:allow-other-keys)
-             (let* ((gcclib (string-append (assoc-ref inputs "gcc:lib") "/lib"))
-                    (ld (string-append
-                         (assoc-ref inputs "glibc") "/lib"))
+             (let* ((gcclib (string-append (assoc-ref inputs "gcc:lib")
+                                           "/lib"))
+                    (ld (string-append (assoc-ref inputs "libc") "/lib"))
                     (loader (car (find-files ld "^ld-linux.+")))
                     (net-base (assoc-ref inputs "net-base"))
                     (tzdata-path
                      (string-append (assoc-ref inputs "tzdata") "/share/zoneinfo"))
                     (output (assoc-ref outputs "out")))
 
-               ;; Removing net/ tests
-               (for-each
-                (lambda (srcfile)
-                  (let ((srcfile (string-append "net/" srcfile)))
-                    (delete-file srcfile)))
-                '("multicast_test.go" "parse_test.go" "port_test.go"))
+               ;; Remove net/ tests.  Why?
+               (for-each delete-file
+                         '("net/multicast_test.go" "net/parse_test.go"
+                           "net/port_test.go"))
 
-               ;; Add libgcc to runpath
+               ;; Add libgcc to the RUNPATH.
                (substitute* "cmd/go/build.go"
                  (("cgoldflags := \\[\\]string\\{\\}")
                   (string-append "cgoldflags := []string{"
-                                 "\"-rpath=" gcclib "\""
-                                 "}"))
+                                 "\"-rpath=" gcclib "\"" "}"))
                  (("ldflags := buildLdflags")
                   (string-append
                    "ldflags := buildLdflags\n"
@@ -102,8 +97,8 @@ 
                  (("/usr/bin") (getcwd))
                  (("/bin/pwd") (which "pwd")))
 
-               ;; Disable failing tests
-               (map
+               ;; Disable failing tests. (XXX: Why?)
+               (for-each
                 (match-lambda
                   ((file regex)
                    (substitute* file
@@ -127,9 +122,9 @@ 
                  (("/etc/protocols") (string-append net-base "/etc/protocols")))
                (substitute* "time/zoneinfo_unix.go"
                  (("/usr/share/zoneinfo/") tzdata-path))
-               (substitute*
-                   (find-files "cmd" "asm.c")
-                 (("/lib/ld-linux.*\\.so\\.[0-9]") loader)))))
+               (substitute* (find-files "cmd" "asm.c")
+                 (("/lib/ld-linux.*\\.so\\.[0-9]") loader))
+               #t)))
 
          (replace 'build
            (lambda* (#:key inputs outputs #:allow-other-keys)
@@ -144,11 +139,12 @@ 
          (replace 'install
            (lambda* (#:key outputs inputs #:allow-other-keys)
              (let* ((output (assoc-ref outputs "out"))
-                    (doc_out (assoc-ref outputs "doc"))
+                    (doc-out (assoc-ref outputs "doc"))
                     (bash (string-append (assoc-ref inputs "bash") "bin/bash"))
-                    (docs (string-append doc_out "/share/doc/" ,name "-" ,version))
+                    (docs (string-append doc-out "/share/doc/" ,name "-" ,version))
                     (src (string-append
-                            (assoc-ref outputs "src") "/share/" ,name "-" ,version)))
+                          (assoc-ref outputs "src") "/share/"
+                          ,name "-" ,version)))
                (mkdir-p src)
                (copy-recursively "../test" (string-append src "/test"))
                (delete-file-recursively "../test")
@@ -158,27 +154,24 @@ 
                (copy-recursively "../doc" (string-append docs "/doc"))
                (delete-file-recursively "../doc")
 
-               (for-each
-                (lambda (file)
-                  (let* ((filein (string-append "../" file))
-                         (fileout (string-append docs "/" file)))
-                  (copy-file filein fileout)
-                  (delete-file filein)))
-                '("README" "CONTRIBUTORS" "AUTHORS" "PATENTS"
-                  "LICENSE" "VERSION" "robots.txt"))
-               (copy-recursively "../" output)))))))
+               (for-each (lambda (file)
+                           (let ((file (string-append "../" file)))
+                             (install-file file docs)
+                             (delete-file file)))
+                         '("README" "CONTRIBUTORS" "AUTHORS" "PATENTS"
+                           "LICENSE" "VERSION" "robots.txt"))
+               (copy-recursively "../" output)
+               #t))))))
     (inputs
-     `(("tzdata" ,system:tzdata)
-       ("pcre" ,pcre)))
+     `(("tzdata" ,tzdata)
+       ("pcre" ,pcre)
+       ("gcc:lib" ,gcc "lib")))
     (native-inputs
      `(("pkg-config" ,%pkg-config)
-       ("which" ,system:which)
-       ("gcc:out" ,gcc-4.9 "out")
+       ("which" ,which)
        ("net-base" ,net-base)
        ("perl" ,perl)))
-    (propagated-inputs
-     `(("gcc:lib" ,gcc-4.9 "lib")
-       ("glibc" ,system:glibc)))
+
     (home-page "https://golang.org/")
     (synopsis "Compiled, statically typed language developed by Google")
     (description "Go, also commonly referred to as golang, is a programming