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.

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

cmd/cgo: correctly record pkg-config CFLAGS and LDFLAGS in package archives #5224

Closed
pebbe opened this issue Apr 5, 2013 · 20 comments

Comments

@pebbe
Copy link
Contributor

commented Apr 5, 2013

go version devel +b32ab1e2e6df Fri Apr 05 21:24:07 2013 +0200 linux/amd64

Install some package that uses cgo with something like this in it:

    #cgo pkg-config: some_c_library

The package will install without error.

Next, try to build a Go program that imports that package. It will fail with 'undefined
reference' errors.

Build the program with -ldflags="-linkmode internal" and it builds without
error, and works fine.

Remove the pkg-config directive from the package, replace it with this:

    #cgo CFLAGS: -I/path/to/some_c_library/header/files
    #cgo LDFLAGS: -L/path/to/some_c_library/library/files -lsome_c_library

Then install it, and then, programs importing the package can be build in the normal
way, without having to add the -ldflags option.
@alberts

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2013

Comment 1:

the ZeroMQ wrapper might be a good test case. Discussion:
https://groups.google.com/d/topic/golang-nuts/st0tegAes4Q/discussion
@kisielk

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2013

Comment 2:

I'm bisecting the repo with the llgo build mentioned in that thread.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2013

Comment 3:

A self-contained test case would be really helpful.  From the description this shouldn't
be too hard.

Labels changed: added priority-soon, go1.1, removed priority-triage.

@pebbe

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2013

Comment 4:

This demonstrates the problem.
On Linux:
Download and install ZeroMQ version 3 from: http://www.zeromq.org/intro:get-the-software
Then, run these commands:
    go get github.com/pebbe/zmq3
    cd $GOPATH/src/github.com/pebbe/zmq3/examples
    go build version.go     # this will fail
But:
    go build -ldflags="-linkmode internal" version.go   # this will succeed
If you already have ZeroMQ version2 installed, and don't want to install version 3, than
you can use this package:
    github.com/pebbe/zmq2
@kisielk

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2013

Comment 5:

The error was introduced in 030625a923ca
Not sure if I can produce a reduced testcase, I just encountered this when trying to
build llgo.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2013

Comment 6:

I'm surprised that is the revision that introduces the failure.
Can you attach the error messages to this issue?
@kisielk

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2013

Comment 7:

They all look like what I reported in this thread:
https://groups.google.com/forum/m/#!topic/llgo-dev/Efhcn94mwNU
On Friday, April 5, 2013, wrote:
@remyoudompheng

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2013

Comment 8:

please run go build with the -ldflags -v option on addition to -x and other options, and
post the exact verbatim output.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2013

Comment 9:

Same as issue #5205?
@kisielk

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2013

Comment 10:

Here's the full log generated with, running with the revision in question:
export CGO_LDFLAGS="-L/usr/lib/llvm-3.3/lib -lpthread -lffi -lrt -ldl -lm
-Wl,-L/usr/lib/llvm-3.3/lib -lLLVM-3.3"
export CGO_CFLAGS="-I/usr/lib/llvm-3.3/include -DNDEBUG -D_GNU_SOURCE
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -g -O2
-fomit-frame-pointer -fPIC"
go get -ldflags -v github.com/axw/llgo

Attachments:

  1. llgo.log (120842 bytes)
@minux

This comment has been minimized.

Copy link
Member

commented Apr 6, 2013

Comment 11:

Status changed to Accepted.

@kisielk

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2013

Comment 12:

I applied https://golang.org/cl/8465043 and it appears to have fixed the issue
@pebbe

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2013

Comment 13:

Appearances are deceiving. The issue remains.
@axw

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2013

Comment 14:

I think Kamil is referring to the llgo issue. It's different to this issue. Sorry, I
mentioned this in issue #5205, but not here.
@axw

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2013

Comment 15:

Seems that the cgo command has a problem with the platform filter between #cgo and
pkg-filter (e.g. #cgo !windows pkg-config).
@axw

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2013

Comment 16:

Specifically, the problem is that it doesn't recognise the bang. To be fair, the
description at http://golang.org/cmd/cgo/ does not permit this.
Internal linkmode works quite differently: cmd/go parses the pkg-config line, and
interprets the platform filter using standard go/build tag matching (which allows bangs,
multiple tags, etc.); cmd/go then links the cgo library, and calls "cgo -dynimport" to
determine the required shared object names.
A simple (I think?) fix would be to update CGO_LDFLAGS when calling cmd/cgo from cmd/go,
and just remove the pkg-config parsing code from cmd/cgo altogether. I don't know if
there are any good reasons to not do this - someone on the core dev team care to comment?
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2013

Comment 17:

I think it would be fine to have the go tool do all the pkg-config parsing.
@axw

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2013

Comment 18:

Peter: I've just submitted a CL for review, 8610044. I have verified against zmq2. If
you could clpatch and verify against zmq3, that'd be handy.
@pebbe

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2013

Comment 19:

Tested with zmq3. It's working.
@robpike

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2013

Comment 20:

This issue was closed by revision d06be39.

Status changed to Fixed.

@pebbe pebbe added fixed labels Apr 11, 2013

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015

@rsc rsc removed the go1.1 label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
10 participants
You can’t perform that action at this time.