This clarifies comments about static linking made in commit a8608b5b67.
1. There are two ways to create a static binary, one is to disable
cgo, the other is to set linker flags. When cgo is disabled,
there is no need to use osusergo build tag.
2. osusergo only needs to be set when linking against glibc.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
TL;DR: there is no way to do this right.
We do know that in some combination of build tags set (or unset),
linker flags, environment variables, and libc implementation,
this package won't work right. In fact, there is one specific
combination:
1. `CGO_ENABLED=1` (or unset)
2. static binary is being built (e.g. `go build` is run with `-extldflags -static`)
3. `go build` links the binary against glibc
4. `osusergo` is not set
This particular combination results in the following legitimate linker warning:
> cgo_lookup_unix.go: warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
If this warning is ignored and the resulting binary is used on a system
with files from a different glibc version (or without those files), it
could result in a segfault.
The commit being reverted tried to guard against such possibility,
but the problem is, we can only use build tags to account for items
1 and 4 from the above list, while items 2 and 3 do not result in
any build tags being set or unset, making this guard excessive.
Remove it.
This reverts commit 023b072288.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
```
pkg/mount/mountinfo_linux.go:93:5: SA4011: ineffective break statement. Did you mean to break out of the outer loop? (staticcheck)
break
^
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also fixed some incorrectly formatted comments
```
pkg/jsonmessage/jsonmessage.go:180:20: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
fmt.Fprintf(out, endl)
^
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This struct now has a properly typed member, so use the properly typed
functions with it.
Also update the vendor directory and hope nothing explodes.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is to ensure that users of the homedir package cannot
compile statically (CGO_ENABLED=0) without also setting the osusergo
build tag.
Signed-off-by: Tibor Vass <tibor@docker.com>
About github.com/opencontainers/runc/libcontainer/user:
According to 195d8d544a
this package has two functions:
- Have a static implementation of user lookup, which is now supported in the
os/user stdlib package with the osusergo build tag, but wasn't at the time.
- Have extra functions that os/user doesn't have, but none of those are used
in homedir.
Since https://github.com/moby/moby/pull/11287, homedir depended directly on
libcontainer's user package for CurrentUser().
This is being replaced with os/user.Current(), because all of our static
binaries are compiled with the osusergo tag, and for dynamic libraries it
is more correct to use libc's implementation than parsing /etc/passwd.
About github.com/docker/docker/pkg/idtools:
Only dependency was from GetStatic() which uses idtools.LookupUID(uid).
The implementation of idtools.LookupUID just calls to
github.com/opencontainers/runc/libcontainer/user.LookupUid or fallbacks
to exec-ing to getent (since https://github.com/moby/moby/pull/27599).
This patch replaces calls to homedir.GetStatic by homedir.Get(), opting out
of supporting nss lookups in static binaries via exec-ing to getent for
the homedir package.
If homedir package users need to support nss lookups, they are advised
to compile dynamically instead.
Signed-off-by: Tibor Vass <tibor@docker.com>
Staticcheck reported:
SA9002: file mode '600' evaluates to 01130; did you mean '0600'? (staticcheck)
But fixing that caused the test to fail:
=== Failed
=== FAIL: pkg/filenotify TestPollerEvent (0.80s)
poller_test.go:75: timeout waiting for event CHMOD
The problem turned out to be that the file was created with `0644`. However,
after umask, the file created actually had `0600` filemode. Running the `os.Chmod`
with `0600` therefore was a no-op, causing the test to fail (because no
CHMOD event would fire).
This patch changes the test to;
- create the file with mode `0600`
- assert that the file has the expected mode
- change the chmod to `0644`
- assert that it has the correct mode, before testing the event.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
pkg/devicemapper/devmapper_wrapper.go:209:206: SA4000: identical expressions on the left and right side of the '==' operator (staticcheck)
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
As pointed out by govet,
> pkg/jsonmessage/jsonmessage_test.go:231:94: nilness: nil dereference in dynamic method call (govet)
> if err := DisplayJSONMessagesStream(reader, data, inFd, false, nil); err == nil && err.Error()[:17] != "invalid character" {
> ^
The nil deref never happened as err was always non-nil, and so the check
for error message text was not performed.
Fix this, and while at it, refactor the code a bit.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fix the following warnings from staticcheck linter:
```
pkg/term/term_linux_test.go:34:2: SA5001: should check returned error before deferring tty.Close() (staticcheck)
defer tty.Close()
^
pkg/term/term_linux_test.go:52:2: SA5001: should check returned error before deferring tty.Close() (staticcheck)
defer tty.Close()
^
pkg/term/term_linux_test.go:67:2: SA5001: should check returned error before deferring tty.Close() (staticcheck)
defer tty.Close()
^
....
```
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
opts/env_test: suppress a linter warning
this one:
> opts/env_test.go:95:4: U1000: field `err` is unused (unused)
> err error
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Format the source according to latest goimports.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
13:06:14 pkg/directory/directory_test.go:182:2: S1032: should use sort.Strings(...) instead of sort.Sort(sort.StringSlice(...)) (gosimple)
13:06:14 sort.Sort(sort.StringSlice(results))
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
13:06:14 pkg/pools/pools.go:75:13: SA6002: argument should be pointer-like to avoid allocations (staticcheck)
13:06:14 bp.pool.Put(b)
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
There is a race condition in pkg/archive when using `cmd.Start` for pigz
and xz where the `*bufio.Reader` could be returned to the pool while the
command is still writing to it, and then picked up and used by a new
command.
The command is wrapped in a `CommandContext` where the process will be
killed when the context is cancelled, however this is not instantaneous,
so there's a brief window while the command is still running but the
`*bufio.Reader` was already returned to the pool.
wrapReadCloser calls `cancel()`, and then `readBuf.Close()` which
eventually returns the buffer to the pool. However, because cmdStream
runs `cmd.Wait` in a go routine that we never wait for to finish, it is
not safe to return the reader to the pool yet. We need to ensure we
wait for `cmd.Wait` to finish!
Signed-off-by: Stephen Benjamin <stephen@redhat.com>
