code review 8465043: cmd/cgo: record CGO_LDFLAGS env var in _cgo_flags (issue 8465043)

[axw...@gmail.com]

Reviewers: iant,

Message:
Hello ia...@golang.org (cc: golan...@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/


Description:
cmd/cgo: record CGO_LDFLAGS env var in _cgo_flags

cgo stores cgo LDFLAGS in _cgo_flags and _cgo_defun.c.
The _cgo_defun.c records the flags via
"#pragma cgo_ldflag <flag>", which external linking
relies upon for passing libraries (and search paths)
to the host linker.

The go command will allow LDFLAGS for cgo to be passed
through the environment (CGO_LDFLAGS); cgo ignores
this environment variable, and so its value doesn't
make it into the above mentioned files. This CL changes
cgo to record CGO_LDFLAGS also.

Please review this at https://codereview.appspot.com/8465043/

Affected files:
M src/cmd/cgo/main.go


Index: src/cmd/cgo/main.go
===================================================================
--- a/src/cmd/cgo/main.go
+++ b/src/cmd/cgo/main.go
@@ -208,6 +208,11 @@

p := newPackage(args[:i])

+ // Record CGO_LDFLAGS from the environment for external linking.
+ if ldflags := os.Getenv("CGO_LDFLAGS"); ldflags != "" {
+ p.addToFlag("LDFLAGS", strings.Fields(ldflags))
+ }
+
// Need a unique prefix for the global C symbols that
// we use to coordinate between gcc and ourselves.
// We already put _cgo_ at the beginning, so the main

[minux]

Fixes issue 5205.

Do we need to support embedded space in $CGO_LDFLAGS?

[axw...@gmail.com]

On 2013/04/07 16:04:35, minux wrote:
> Fixes issue 5205.

> Do we need to support embedded space in $CGO_LDFLAGS?

Yes, sorry, I should have used splitQuoted. Will fix it this evening
when I get home from work.

https://codereview.appspot.com/8465043/

[axw...@gmail.com]

Hello ia...@golang.org, minu...@gmail.com (cc:
golan...@googlegroups.com),

Please take another look.


https://codereview.appspot.com/8465043/

[minu...@gmail.com]

LGTM.

Please add "Fixes issue 5205." to the CL description.

https://codereview.appspot.com/8465043/

[axw...@gmail.com]

On 2013/04/08 12:44:15, minux wrote:
> LGTM.

> Please add "Fixes issue 5205." to the CL description.

Done. Sorry - I thought your comment would take care of that.
I don't have commit rights, can you please submit it for me?

https://codereview.appspot.com/8465043/

[minux]

On Mon, Apr 8, 2013 at 9:47 PM, <axw...@gmail.com> wrote:
> I don't have commit rights, can you please submit it for me?

Thank you for fixing this. I will take care of submitting this CL.

Will wait some time in case iant (or anyone) has further comments.

[ia...@golang.org]

LGTM

Thanks.


https://codereview.appspot.com/8465043/

[minu...@gmail.com]

*** Submitted as
https://code.google.com/p/go/source/detail?r=38bb920f0beb ***

cmd/cgo: record CGO_LDFLAGS env var in _cgo_flags

cgo stores cgo LDFLAGS in _cgo_flags and _cgo_defun.c.
The _cgo_defun.c records the flags via
"#pragma cgo_ldflag <flag>", which external linking
relies upon for passing libraries (and search paths)
to the host linker.

The go command will allow LDFLAGS for cgo to be passed
through the environment (CGO_LDFLAGS); cgo ignores
this environment variable, and so its value doesn't
make it into the above mentioned files. This CL changes
cgo to record CGO_LDFLAGS also.

Fixes issue 5205.

R=iant, minux.ma
CC=golang-dev
https://codereview.appspot.com/8465043

Committer: Shenghou Ma <minu...@gmail.com>


https://codereview.appspot.com/8465043/