Golang programs keeping references [gnu: go: Update default to 1.11.]

Golang programs keeping references [gnu: go: Update default to 1.11.]

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On Mon, Nov 12, 2018 at 04:32:46AM -0500, Pierre Neidhardt wrote:
> commit 9a65a052016572b61e3c4247fcdf9e0478656f71
> Author: Pierre Neidhardt <mail@ambrevar.xyz>
> Date:   Sun Nov 11 22:02:18 2018 +0100
> 
>     gnu: go: Update default to 1.11.
>     
>     * gnu/packages/golang.scm (go): Update default to 1.11.

I noticed that since this change, Go programs (that is, command-line
executables) keep references to their inputs:

$ guix gc --references $(./pre-inst-env guix build --no-grafts kurly)
/gnu/store/2b2md66fbzyspsmd5dj6zkj9hilac40r-tzdata-2018e
/gnu/store/4iwksvq53rlzphfp3xvp63ihlw226c0n-go-github-com-aki237-nscjar-0.0.0-0.e2df936
/gnu/store/5rxdjbk8h0bh1hbaan8y8ib13va2bcmw-net-base-5.3
/gnu/store/ahvdlp6y44qj6kx63rmx1sq8r61x3zc2-go-github-com-alsm-ioprogress-0.0.0-0.063c372
/gnu/store/f8yps0l8p371jgzh6cki0z5n2kgfjiwy-go-github-com-urfave-cli-1.19.1-0.934abfb
/gnu/store/l4lr0f5cjd0nbsaaf8b5dmcw1a1yypr3-glibc-2.27
/gnu/store/pp0bakrbyv9xmp1kyv2114l19s11b74z-gcc-6.4.0-lib

Previously, they did not:

$ guix gc --references $(guix build --no-grafts kurly)
/gnu/store/2b2md66fbzyspsmd5dj6zkj9hilac40r-tzdata-2018e
/gnu/store/5rxdjbk8h0bh1hbaan8y8ib13va2bcmw-net-base-5.3
/gnu/store/l4lr0f5cjd0nbsaaf8b5dmcw1a1yypr3-glibc-2.27
/gnu/store/pp0bakrbyv9xmp1kyv2114l19s11b74z-gcc-6.4.0-lib

Is this expected? I thought that Go programs were always statically
linked.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

Yes, it should be statically linked.  Isn't it the same issue as we recently had
with go-1.11 itself?

The fix was

--8<---------------cut here---------------start------------->8---
           ;; Prevent installation of the build cache, which contains store
           ;; references to most of the tools used to build Go and would
           ;; unnecessarily increase the size of Go's closure if it was
           ;; installed.
           ;; TODO This should be moved into the 'install' phase when Go 1.9 is
           ;; removed.
           (add-before 'install 'delete-extraneous-files
             (lambda _
               (delete-file-recursively "../pkg/obj")
               #t))
--8<---------------cut here---------------end--------------->8---

We should try that.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

On Mon, Nov 12, 2018 at 06:38:53PM +0100, Pierre Neidhardt wrote:
> Yes, it should be statically linked.  Isn't it the same issue as we recently had
> with go-1.11 itself?
> 
> The fix was
> 
> --8<---------------cut here---------------start------------->8---
>            ;; Prevent installation of the build cache, which contains store
>            ;; references to most of the tools used to build Go and would
>            ;; unnecessarily increase the size of Go's closure if it was
>            ;; installed.
>            ;; TODO This should be moved into the 'install' phase when Go 1.9 is
>            ;; removed.
>            (add-before 'install 'delete-extraneous-files
>              (lambda _
>                (delete-file-recursively "../pkg/obj")
>                #t))
> --8<---------------cut here---------------end--------------->8---
> 
> We should try that.

I can try it now.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]

On Mon, Nov 12, 2018 at 06:38:53PM +0100, Pierre Neidhardt wrote:
> Yes, it should be statically linked.  Isn't it the same issue as we recently had
> with go-1.11 itself?
> 
> The fix was
> 
> --8<---------------cut here---------------start------------->8---
>            ;; Prevent installation of the build cache, which contains store
>            ;; references to most of the tools used to build Go and would
>            ;; unnecessarily increase the size of Go's closure if it was
>            ;; installed.
>            ;; TODO This should be moved into the 'install' phase when Go 1.9 is
>            ;; removed.
>            (add-before 'install 'delete-extraneous-files
>              (lambda _
>                (delete-file-recursively "../pkg/obj")
>                #t))

Unfortunately, in this case, the references are within the executable
binaries, so it's not the same issue.

We could rewrite the references like we do for the errant compiler
references. This will be really expensive though — if there is no other
way, then it's time to finally move the Boyer-Moore implementation in
(guix grafts) to its own module so we can use it easily.

But ideally we could prevent these references from being created in the
first place.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 240 bytes --]

On Mon, Nov 12, 2018 at 12:48:20PM -0500, Leo Famulari wrote:
> way, then it's time to finally move the Boyer-Moore implementation in
> (guix grafts) to its own module so we can use it easily.

Mistake — it's in (guix build grafts).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]

I've investigated the possible solutions to avoid including the paths into the
binaries.

I've found this:

https://github.com/golang/go/issues/16860

It's still unresolved and only planned for Go 1.13.

In the meantime, I've played with the -trimpath option to see if I could get
something out of it.

I've added this to Go's (build) function:

--8<---------------cut here---------------start------------->8---
              "-asmflags=all=-trimpath=/gnu/store"
              "-gcflags=all=-trimpath=/gnu/store"
--8<---------------cut here---------------end--------------->8---

The resulting binary is indeed trimmed, but that's not enough: it seems that
Guix detects the remaining part of the path as a store item and includes it in
the list of requisites.  Is this really how Guix works?  It does not need the
full path?

Go supports only one call to -trimpath, so we can't even set this to the Go
inputs.

Regarding Boyer-Moore over the binary, we would have to apply the changes for
all recursive Go libraries.  Now is there a reliable way to detect what's a Go
library and what is not?  We don't want to patch non-Go libraries, right?
(Let's not forget that there is CGo...)

If we can't think of a way to detect a Go library, Boyer-Moore does not seem to
be a solution either.  And we might have to wait for Go 1.13...

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Ludovic Courtès]

Hello!

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> I've investigated the possible solutions to avoid including the paths into the
> binaries.
>
> I've found this:
>
> https://github.com/golang/go/issues/16860
>
> It's still unresolved and only planned for Go 1.13.
>
> In the meantime, I've played with the -trimpath option to see if I could get
> something out of it.
>
> I've added this to Go's (build) function:
>
>               "-asmflags=all=-trimpath=/gnu/store"
>               "-gcflags=all=-trimpath=/gnu/store"
>
> The resulting binary is indeed trimmed, but that's not enough: it seems that
> Guix detects the remaining part of the path as a store item and includes it in
> the list of requisites.  Is this really how Guix works?  It does not need the
> full path?

No, the scanner just looks for the hash part of the store file name (see
‘scanForReferences’ in nix/libstore/references.cc).

How much of a problem is this though?  Currently the set of references
looks reasonable:

--8<---------------cut here---------------start------------->8---
$ guix size go-ipfs
store item                                                       total    self
/gnu/store/rm3cfgzd8iywjmsdcxf6gnwswm5za50x-go-ipfs-0.4.18         203.4   102.2  50.2%
/gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28              37.8    36.3  17.8%
/gnu/store/ypiv8dj4lkvsnm82s639h18l87frrh5g-gcc-6.5.0-lib           69.0    31.2  15.3%
/gnu/store/4sqps8dczv3g7rwbdibfz6rf5jlk7w90-gcc-5.5.0-lib           68.0    30.2  14.8%
/gnu/store/8g2wi0i5fgp0ylz99mckhprh25p1zgiv-tzdata-2018e             2.0     2.0   1.0%
/gnu/store/zzakf905mzla4csi1dn9qpcwmgbxj29b-bash-static-4.4.23       1.5     1.5   0.8%
/gnu/store/sj8m05bfj2902h67c4qkmvnzg2pjdgsv-net-base-5.3             0.0     0.0   0.0%
total: 203.4 MiB
--8<---------------cut here---------------end--------------->8---

Well OK we could do with a single gcc:lib :-), but other than that it
looks good.

Thoughts?

Ludo’.

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]


Hi Ludo!

OK, thanks for clarifying scanForReferences.

go-ipfs is fine indeed, but that's because you've just looked at the lucky
package!  go-ipfs vendors all the Go dependencies, which is why there is no
extra input there and go-ipfs closure size is optimal.

Now try with gx, demlo or syncthing: all of them had zero Go inputs in their
closure with go-1.9.  Now they have hundreds with go-1.11 :(

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Ludovic Courtès]

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> go-ipfs is fine indeed, but that's because you've just looked at the lucky
> package!  go-ipfs vendors all the Go dependencies, which is why there is no
> extra input there and go-ipfs closure size is optimal.

Oh, we should really work towards unbundling things from ‘go-ipfs’.  Any
idea how difficult that would be?

> Now try with gx, demlo or syncthing: all of them had zero Go inputs in their
> closure with go-1.9.  Now they have hundreds with go-1.11 :(

“guix size syncthing”, for instance, returns mostly tiny Go packages,
which looks good to me, no?

Anyway, if you think there’s a problem, could you email details to
bug-guix@gnu.org so we keep track of it there?

Thanks!

Ludo’.

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]


> Oh, we should really work towards unbundling things from ‘go-ipfs’.  Any
> idea how difficult that would be?

go-ipfs has more than 100 deps.  So unless we work on a recursive importer, it's
going to be a lot of work.

Harder: those dependencies can only be retrieved with gx (or IPFS itself, but
then we have a  bootstrapping issue).  Hence my former work on the gx
downloader.  I got stuck at an SSL issue.

> “guix size syncthing”, for instance, returns mostly tiny Go packages,
> which looks good to me, no?

Funny, for me "guix size syncthing" returns no Go package at all... Err... Am I
doing something wrong?

The issue is there for Demlo and Kurly though.  It's not a big deal, but it
could be cleaner.

I'll open a bug.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Ludovic Courtès]

Hello,

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

>> Oh, we should really work towards unbundling things from ‘go-ipfs’.  Any
>> idea how difficult that would be?
>
> go-ipfs has more than 100 deps.  So unless we work on a recursive importer, it's
> going to be a lot of work.
>
> Harder: those dependencies can only be retrieved with gx (or IPFS itself, but
> then we have a  bootstrapping issue).  Hence my former work on the gx
> downloader.  I got stuck at an SSL issue.

Oh OK.  In general, we always unbundle software (for better
transparency, handling of security updates, etc.).  So it can’t be a
long-term plan to keep everything bundled in ‘go-ipfs’ (well, unless we
don’t add any new Go packages, in which case bundling in this specific
package becomes “invisible” :-)).

>> “guix size syncthing”, for instance, returns mostly tiny Go packages,
>> which looks good to me, no?
>
> Funny, for me "guix size syncthing" returns no Go package at all... Err... Am I
> doing something wrong?

Here’s what I see:

--8<---------------cut here---------------start------------->8---
$ guix describe
Generation 46   Dec 14 2018 15:56:53    (current)
  guix adb158b
    repository URL: https://git.savannah.gnu.org/git/guix.git
    branch: master
    commit: adb158b7396cbdcda347fa298978408e531a03fd
$ guix size syncthing
substitute: updating substitutes from 'https://berlin.guixsd.org'... 100.0%
substitute: updating substitutes from 'https://mirror.hydra.gnu.org'... 100.0%
store item                                                       total    self
/gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28              37.8    36.3  41.4%
/gnu/store/ypiv8dj4lkvsnm82s639h18l87frrh5g-gcc-6.5.0-lib           69.0    31.2  35.6%
/gnu/store/nfikc8vj2whjl7012dgaj30m7y2lpv9d-syncthing-0.14.54       87.7    16.6  19.0%
/gnu/store/8g2wi0i5fgp0ylz99mckhprh25p1zgiv-tzdata-2018e             2.0     2.0   2.3%
/gnu/store/zzakf905mzla4csi1dn9qpcwmgbxj29b-bash-static-4.4.23       1.5     1.5   1.7%
/gnu/store/sj8m05bfj2902h67c4qkmvnzg2pjdgsv-net-base-5.3             0.0     0.0   0.0%
total: 87.7 MiB
--8<---------------cut here---------------end--------------->8---

So hmm, you’re right!  I’m sure I saw go packages somewhere, dunno…

Cheers,
Ludo’.

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]


> So hmm, you’re right!  I’m sure I saw go packages somewhere, dunno…

Just looked at the source: after my Go 1.11 update, Syncthing was updated to use
vendored dependencies, just like go-ipfs.  This is why it does not contains any
go ref.  From the package declaration:

              ;; Since the update to Go 1.11, Go programs have been keeping
              ;; spurious references to all their dependencies:
              ;; <https://bugs.gnu.org/33620>.
              ;; For Syncthing, this increases the size of the 'out' closure
              ;; from 87.6 MiB to 253.5 MiB.  So, we use the bundled
              ;; dependencies until the bug is resolved.
              
So yup, "guix size" is not happy with Go...

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1401 bytes --]

On Sat, Dec 15, 2018 at 07:19:49PM +0100, Pierre Neidhardt wrote:
> 
> > So hmm, you’re right!  I’m sure I saw go packages somewhere, dunno…
> 
> Just looked at the source: after my Go 1.11 update, Syncthing was updated to use
> vendored dependencies, just like go-ipfs.  This is why it does not contains any
> go ref.  From the package declaration:
> 
>               ;; Since the update to Go 1.11, Go programs have been keeping
>               ;; spurious references to all their dependencies:
>               ;; <https://bugs.gnu.org/33620>.
>               ;; For Syncthing, this increases the size of the 'out' closure
>               ;; from 87.6 MiB to 253.5 MiB.  So, we use the bundled
>               ;; dependencies until the bug is resolved.
>               
> So yup, "guix size" is not happy with Go...

That's right. I've previously audited Syncthing's dependencies and I'm
sure they are all free software. Additionally, the Syncthing project is
committed to shipping free software.

Because the closure size had grown way too big, and we don't have a plan
for fixing the issue with Go 1.11, I decided to just use the bundled
dependencies.

Hopefully we can switch it back at some point. I haven't investigated
this particular issue very much, so I don't know how feasible it will
be. I do know that it is basically not supported by the Go community.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

On Sat, Dec 15, 2018 at 07:19:49PM +0100, Pierre Neidhardt wrote:
> 
> > So hmm, you’re right!  I’m sure I saw go packages somewhere, dunno…
> 
> Just looked at the source: after my Go 1.11 update, Syncthing was updated to use
> vendored dependencies, just like go-ipfs.  This is why it does not contains any
> go ref.  From the package declaration:
> 
>               ;; Since the update to Go 1.11, Go programs have been keeping
>               ;; spurious references to all their dependencies:
>               ;; <https://bugs.gnu.org/33620>.
>               ;; For Syncthing, this increases the size of the 'out' closure
>               ;; from 87.6 MiB to 253.5 MiB.  So, we use the bundled
>               ;; dependencies until the bug is resolved.
>               
> So yup, "guix size" is not happy with Go...

Also, can we please keep this discussion in the bug report thread?

https://bugs.gnu.org/33620

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Golang programs keeping references [gnu: go: Update default to 1.11.]

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 126 bytes --]

In the GitHub issue I linked above, they say the fix is planned for Go 1.13.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]