New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 796320 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Remove system dependencies from Linux Go binaries

Project Member Reported by d...@chromium.org, Dec 19 2017

Issue description

From: https://groups.google.com/a/chromium.org/d/msg/chromium-dev/RLwRMF1BkPM/pTb_m5-FCgAJ

natanael.copa@ recommended building Linux Go binaries with:
$ go build -a -ldflags '-extldflags "-static"' go.chromium.org/luci/cipd/client/cmd/cipd

I haven't looked into it, but if this is something that is effective, not too expensive, and trivial to add to the "build.py" script, perhaps we should add it for Linux builds: https://chromium.googlesource.com/infra/infra/+/7a3650e64866c8f83a53b11e632829e7b0eea923/build/build.py#435

This would presumably make them more universal, allowing Linux binaries and bootstrap to work on Alpine Linux.
 
Hm, interesting. I'm going to try it and see how much bloat it adds.
I think we pull in dynamic libraries through C dependencies:

$ go build .
$ ldd ./cipd
	linux-vdso.so.1 =>  (0x00007ffd0a904000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f8c75761000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f8c75398000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f8c7597f000)

$ CGO_ENABLED=0 go build .
$ ldd ./cipd
	not a dynamic executable

We already quite heavily rely on non-cgo builds when cross-compiling. I think it would be OK to disable cgo even when not cross-compiling. It would make all our binaries statically linked.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/1599153bc5487c96bf66c74c187d67fd3ea5a3f1

commit 1599153bc5487c96bf66c74c187d67fd3ea5a3f1
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Dec 19 22:28:35 2017

Disable cgo on Linux and Windows.

There are multiple reasons for this:
 * Makes all binaries statically linked (no dependency on dynamic libc).
 * Makes cross-compiled and non-cross-compiled binaries be more similar, so
   they behave uniformly across platforms.
 * Makes sure dependencies with cgo don't work locally for developers, as
   they won't really work when building code on the bots. We don't maintain
   C toolchain on bots that build Go code currently (especially true for
   Windows bots).

Unfortunately, some third party libraries (e.g github.com/shirou/gopsutil)
have poor support for non-cgo build on OSX, so we keep it enabled on OSX,
assuming there's not a wide variety of OSX platforms out there and it's fine
to have dynamically linked binaries.

R=iannucci@chromium.org
BUG= 796320 

Change-Id: Ie947e716375cb4af3f17169bd30823de2470b9f8
Reviewed-on: https://chromium-review.googlesource.com/834992
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/1599153bc5487c96bf66c74c187d67fd3ea5a3f1/go/bootstrap.py

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/ac945475528762bd6e91a865919990a5a393503a

commit ac945475528762bd6e91a865919990a5a393503a
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Dec 19 23:12:16 2017

Revert "Disable cgo on Linux and Windows."

This reverts commit 1599153bc5487c96bf66c74c187d67fd3ea5a3f1.

Reason for revert: 'go test -race' needs cgo :(

Original change's description:
> Disable cgo on Linux and Windows.
> 
> There are multiple reasons for this:
>  * Makes all binaries statically linked (no dependency on dynamic libc).
>  * Makes cross-compiled and non-cross-compiled binaries be more similar, so
>    they behave uniformly across platforms.
>  * Makes sure dependencies with cgo don't work locally for developers, as
>    they won't really work when building code on the bots. We don't maintain
>    C toolchain on bots that build Go code currently (especially true for
>    Windows bots).
> 
> Unfortunately, some third party libraries (e.g github.com/shirou/gopsutil)
> have poor support for non-cgo build on OSX, so we keep it enabled on OSX,
> assuming there's not a wide variety of OSX platforms out there and it's fine
> to have dynamically linked binaries.
> 
> R=​iannucci@chromium.org
> BUG= 796320 
> 
> Change-Id: Ie947e716375cb4af3f17169bd30823de2470b9f8
> Reviewed-on: https://chromium-review.googlesource.com/834992
> Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
> Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

TBR=iannucci@chromium.org,vadimsh@chromium.org

Change-Id: I5614fc4809ae025503136109c6eb8ac58d59e415
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  796320 
Reviewed-on: https://chromium-review.googlesource.com/834671
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/ac945475528762bd6e91a865919990a5a393503a/go/bootstrap.py

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/2288f0cb47eea8b5430882867099586afcee2368

commit 2288f0cb47eea8b5430882867099586afcee2368
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Dec 20 04:38:05 2017

Selectively build some CIPD packages with CGO_ENABLED=0.

This makes them statically linked.

We don't build all packages that way because our CI system uses cgo-enabled
binaries by default, and testing a cgo-enabled variant of a package but
releasing a non-cgo one (by default) is not very smart (though we do it
currently when cross-compiling, unfortunately).

Instead owners of the package should decide whether its OK to release non-cgo
variant or not by modifying corresponding package definition YAML.

R=iannucci@chromium.org
BUG= 796320 

Change-Id: Ie6fbb5bf77fa1d387eee3fbc8e6ec0d4c1391d4d
Reviewed-on: https://chromium-review.googlesource.com/835871
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/2288f0cb47eea8b5430882867099586afcee2368/build/README.md
[modify] https://crrev.com/2288f0cb47eea8b5430882867099586afcee2368/build/packages/cipd_client.yaml
[modify] https://crrev.com/2288f0cb47eea8b5430882867099586afcee2368/build/build.py
[modify] https://crrev.com/2288f0cb47eea8b5430882867099586afcee2368/build/packages/vpython-native.yaml
[modify] https://crrev.com/2288f0cb47eea8b5430882867099586afcee2368/build/packages/led.yaml
[modify] https://crrev.com/2288f0cb47eea8b5430882867099586afcee2368/build/packages/vpython.yaml

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/9b5cb92fa457c8f61a058c6584c3bec233088c3b

commit 9b5cb92fa457c8f61a058c6584c3bec233088c3b
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Dec 20 19:41:02 2017

Update cipd client version.

Notable changes here:
 * 'cipd' binary is now statically linked and doesn't depend of libc.so.
 * New 'cipd instances <pkg>' subcommand that list recently uploaded instances
   of a package.
 * cipd now properly removes empty directory trees if they are left after
   uninstalling some files.

R=iannucci@chromium.org
BUG= 796320 ,  789750 

Change-Id: Iebc5655640defbbef4a5fb1850aeb852746e5ef3
Reviewed-on: https://chromium-review.googlesource.com/835657
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/9b5cb92fa457c8f61a058c6584c3bec233088c3b/cipd_client_version

Comment 7 by mar...@chromium.org, Dec 30 2017

Cc: -vadimsh@chromium.org
Labels: -Type-Bug Type-Feature
Owner: vadimsh@chromium.org
Status: Assigned (was: Untriaged)
Assigning to Vadim since he worked on it.
Status: Fixed (was: Assigned)
All Go binaries in depot_tools (cipd and vpython specifically) are statically linked now.

Sign in to add a comment