Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Repro for CGO errors on alpine #119

Closed
novas0x2a opened this issue Dec 16, 2016 · 8 comments
Closed

Repro for CGO errors on alpine #119

novas0x2a opened this issue Dec 16, 2016 · 8 comments

Comments

@novas0x2a
Copy link

I have an easy repro (and workaround) for the problem mentioned in #16 (comment) (which is also mentioned in alecthomas/gometalinter#149) re: running errcheck in a CGO_ENABLED build on alpine.

mike@lusus:~> docker run --entrypoint sh -it golang:1.7.4-alpine
/go # ( apk update && apk add git ) &>/dev/null
/go # go get github.com/kisielk/errcheck github.com/alecthomas/gometalinter github.com/stretchr/testify/assert
/go # cd /go/src/github.com/alecthomas/gometalinter/
/go/src/github.com/alecthomas/gometalinter # gometalinter --enable-gc --deadline 40s --exclude vendor --skip vendor --disable-all --enable=errcheck ./...
/usr/local/go/src/net/lookup_unix.go:58:24:warning: error return value not checked (undeclared name: cgoLookupHost) (errcheck)
/usr/local/go/src/net/lookup_unix.go:70:24:warning: error return value not checked (undeclared name: cgoLookupIP) (errcheck)
/usr/local/go/src/net/lookup_unix.go:86:23:warning: error return value not checked (undeclared name: cgoLookupPort) (errcheck)
/usr/local/go/src/net/lookup_unix.go:95:24:warning: error return value not checked (undeclared name: cgoLookupCNAME) (errcheck)
/usr/local/go/src/net/lookup_unix.go:162:23:warning: error return value not checked (undeclared name: cgoLookupPTR) (errcheck)
/go/src/github.com/alecthomas/gometalinter # errcheck ./...
exec: "gcc": executable file not found in $PATH
cgo failed: [go tool cgo -objdir /tmp/net_C448012175 -- -I /tmp/net_C448012175 cgo_linux.go cgo_resnew.go cgo_socknew.go cgo_unix.go]: exit status 1
/usr/local/go/src/net/lookup_unix.go:58:24: undeclared name: cgoLookupHost
/usr/local/go/src/net/lookup_unix.go:70:24: undeclared name: cgoLookupIP
/usr/local/go/src/net/lookup_unix.go:86:23: undeclared name: cgoLookupPort
/usr/local/go/src/net/lookup_unix.go:95:24: undeclared name: cgoLookupCNAME
/usr/local/go/src/net/lookup_unix.go:162:23: undeclared name: cgoLookupPTR
error: failed to check packages: could not type check: couldn't load packages due to errors: net

The error's pretty inscrutable when run via GML but pretty obvious when run through errcheck directly. Now if I install gcc and musl-dev, the error is resolved:

/go/src/github.com/alecthomas/gometalinter # apk add gcc musl-dev &>/dev/null
/go/src/github.com/alecthomas/gometalinter # gometalinter --enable-gc --deadline 40s --exclude vendor --skip vendor --disable-all --enable=errcheck ./...
main.go:162:15:warning: error return value not checked (defer r.Close()) (errcheck)
regressiontests/support.go:48:20:warning: error return value not checked (defer os.RemoveAll(dir)) (errcheck)
regressiontests/support.go:53:17:warning: error return value not checked (defer os.Remove(w.Name())) (errcheck)
/go/src/github.com/alecthomas/gometalinter # errcheck ./... | head -n5
main.go:162:15: defer r.Close()
regressiontests/support.go:48:20:       defer os.RemoveAll(dir)
regressiontests/support.go:53:17:       defer os.Remove(w.Name())
vendor/src/github.com/HewlettPackard/gas/core/issue.go:68:11:   file.Seek(start, 0)
vendor/src/github.com/HewlettPackard/gas/core/issue.go:84:19:   defer file.Close()

So, the source of the error is actually that CGO is configured but kinda broken, and the fix is to install the compiler and the libc headers. I'm not actually sure if there's anything errcheck (or GML) can do about this, but I figured I'd open this to at least provide google fuel for the next poor soul who runs into the problem like I did.
@novas0x2a novas0x2a changed the title Confirmed repro for CGO errors Confirmed repro for CGO errors on alpine Dec 16, 2016
@novas0x2a novas0x2a changed the title Confirmed repro for CGO errors on alpine Repro for CGO errors on alpine Dec 16, 2016
@kisielk
Copy link
Owner

kisielk commented Dec 18, 2016

I think it's because go/loader expects to load a compiled version of the package and compiles it if it's not available. It seems like the installation of Go you have doesn't have a cgo-enabled net package and so it wants to compile it, which requires gcc. At least that's my guess based on the log output.

@kisielk
Copy link
Owner

kisielk commented Dec 18, 2016

I actually think this is a separate problem from #16

@novas0x2a
Copy link
Author

Ah, just to clarify:

  1. I don't think it's the same problem as Fails if anything uses cgo #16, but it is the same problem as the one in the comment on 16 by @amacneil which was the only reference I found to the same problem I ran into. (My issue here is a direct response to @zimmski's request for a repro).
  2. I think your analysis is correct, I just wanted to raise the issue because this is actually the official go alpine image, so more people are likely to run into the same problem (and I wanted to give them a google reference to find). It's unlikely to be fixed in the official image, because it's very common for alpine images to lack gcc.
  3. I don't think this is a bug in errcheck, but I figured I'd raise it here in case you could think of a better way to handle it.

@kisielk
Copy link
Owner

kisielk commented Dec 18, 2016

Right, I agree with all that. I don't think there's a better way to fix it. Really if you are developing anything with CGO code, you should have a copy of gcc available.

@leighmcculloch
Copy link

leighmcculloch commented May 12, 2017 •
edited
Loading

I'm seeing this happen on Ubuntu after installing go from the official download page. My installation process is here and is simply a wget of go, then go get of errcheck. I'm attempting to run errcheck on a project (https://github.com/lionelbarrow/braintree-go) that does not use CGO.

Running errcheck, this is what I get:

$ errcheck ./...                                                                                                                                                                             
exec: "gcc": executable file not found in $PATH
cgo failed: [go tool cgo -objdir /tmp/net_C253105090 -- -I /tmp/net_C253105090 cgo_linux.go cgo_resnew.go cgo_socknew.go cgo_unix.go]: exit status 1
/usr/local/go/src/net/lookup_unix.go:54:24: undeclared name: cgoLookupHost
/usr/local/go/src/net/lookup_unix.go:69:24: undeclared name: cgoLookupIP
/usr/local/go/src/net/lookup_unix.go:81:23: undeclared name: cgoLookupPort
/usr/local/go/src/net/lookup_unix.go:97:24: undeclared name: cgoLookupCNAME
/usr/local/go/src/net/lookup_unix.go:164:23: undeclared name: cgoLookupPTR
error: failed to check packages: could not type check: couldn't load packages due to errors: net

But if I explicitly disable CGO it works as expected.

$ CGO_ENABLED=0 errcheck ./...
braintree.go:106:23:    defer resp.Body.Close()
...

@roadrunner
Copy link

@leighmcculloch, same here for alpine images

when not using cgo at all, it's sad to explicitly tell disable cgo

@leighmcculloch
Copy link

I'm now using Go1.9 on Ubuntu with revision b1445a9 of errcheck and I no longer have this issue.

fabianschwarzfritz pushed a commit to Peripli/service-broker-proxy-k8s that referenced this issue May 3, 2018
Add additional docker image for gometalinter.
7269991 
Currently the gometalinter does not work correctly with the alpine
docker image, as it scans too many directory.
See github issues:
 - kisielk/errcheck#119
 - alecthomas/gometalinter#149

As a solution, we add an additional layer with a debian image to the
multi-staged docker image. In this image, the gometalinter works
correctly.
@kisielk
Copy link
Owner

kisielk commented May 17, 2018

Closing since it seems this was resolved.

@kisielk kisielk closed this as completed May 17, 2018
georgifarashev pushed a commit to Peripli/service-broker-proxy-k8s that referenced this issue Jun 26, 2018
@georgifarashev
First version of service-broker-proxy-k8s (#17)
be0fa41 
* working demo for ServiceBroker CRUD Operations

* remove unused code

* Fix formatting and lint errors

* Revert "remove unused code"

This reverts commit a01455b.

* Add Andreas example; periodically ping the service manager;

 - Add Andreas example to to a new file 'servicecatalog', which provides
 methods to list/register/unregister service brokers form the k8s
 serivce catalog.
 - Add function to periodically ping the service manager.
 - Formatting.

* Start broker proxy in main.go

* Fix constructor, check for error when setting env variable

* Use broker lib

* First working draft of k8s

* Use enablement service-manager

* Pin service-catalog to version v0.1.13

* Fix log messages in k8s client

* Proxy not needed anymore; Remove secrets/config of the k8s deployment

* Downstrip to minimal setup

* use in cluster configuration and strip down Dockerfile for friday demo.
Adding clusterrole and permissions to access the API server/servicecatalog resources.

* fix wrong service declaration - no endpoints assign regarding wrong selector

* Fix linter errors

 - Use 'envProxy' instead of 'env_proxy'
 - Add short documentation to exported methods to
 create/update/list/delete brokers in kubernetes
 - Remove not needed exports.

* Use helm for isntallation of service-broker-proxy

* Add vendor/ to SCM

Not ignoring the vendor folder eases the initial development onboarding and can
guarantee reliable builds.

* Update dependencies and pin service-broker-proxy lib to fixed version

 - Set dependencies to version of dem from last week.
 - Pin version of service-broker-proxy library do working version of
 last week.

* Fix port in service definition in helm chart

* Use gcr docker repository as the default in the helm chart

* Improve installation instructions

* Add additional docker image for gometalinter.

Currently the gometalinter does not work correctly with the alpine
docker image, as it scans too many directory.
See github issues:
 - kisielk/errcheck#119
 - alecthomas/gometalinter#149

As a solution, we add an additional layer with a debian image to the
multi-staged docker image. In this image, the gometalinter works
correctly.

* Update service-broker-proxy library.

Update service-broker-proxy library to the most recent version.
Adopt client.go to use the newest library version.

* Fix client go type error

* Improve in-code documentation.

* Improve build script for development.

Use helm to install the service-broker-proxy-k8s.

* Tests: implement tests for successfull service-broker operation.

Implement tests for successful service broker operations: create/delete/update/get/sync

* File system mock obsolete in client_internal_test

* Add go dep dependencies for ginkgo and gomega.

* Use better fake urls in unit test.

* Improve logging and log levels.

 - Use warn log-level when an error with the service-catalog occurs and return
 the error to the service-broker-proxy framework.
 - Use fatal log level when the creation of a k8s client occurs.

* Test output prints more information in docker build.

* Test service-broker-creation when catalog returns an error.

* Test service-broker-deletion when catalog returns an error.

* Test getting service-brokers when catalog returns an error.

* Test service-broker-delete when catalog returns an error.

* Test syncing service-brokers when catalog returns an error.

* Print broker guids/names with log messages

* Print go test coverage during build

* Print test coverage

Added a script to print the test coverage information with the following
outputs:
 - html report
 - stdout

* Use .html filending for coverage report

* Return error when clusterconfig cannot be loaded.

* More coverage details during Docker build

* Improve test coverage

Review with @AlexRieder:
 - use warn log-level when error with service-catalog occurs
 - Improve test for service-broker-creation
 - Add test to check getBrokers with empty list of brokers

* README badge: Add go report and add branch to coveralls

* No error handling, as log.Fatal exists the program

* Log error and return error when configuration cannot be loaded

* Template values for service-manager-instance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@novas0x2a @roadrunner @kisielk @leighmcculloch