code review 4859043: build: allow builds without cgo

build: allow builds without cgo

[rsc, 2011-08-10 22:58:23]

Hello golang-dev (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg

[bradfitz, 2011-08-10 23:01:13]

LGTM

Nice!  And the Makefiles are simpler now too.


On Wed, Aug 10, 2011 at 3:58 PM, <rsc@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg
>
>
> Description:
> build: allow builds without cgo
>
> Please review this at
http://codereview.appspot.com/**4859043/<http://codereview.appspot.com/4859043/>
>
> Affected files:
>  M src/Make.inc
>  M src/pkg/net/Makefile
>  M src/pkg/os/user/Makefile
>  M src/pkg/os/user/lookup_unix.go
>  M src/pkg/os/user/user.go
>  M src/pkg/os/user/user_test.go
>  M src/pkg/runtime/cgo/Makefile
>  M src/run.bash
>
>
> Index: src/Make.inc
> ==============================**==============================**=======
> --- a/src/Make.inc
> +++ b/src/Make.inc
> @@ -121,6 +121,21 @@
>  HOST_LDFLAGS=$(HOST_EXTRA_**LDFLAGS)
>  PWD=$(shell pwd)
>
> +# Decide whether use of cgo is okay.
> +ifeq ($(CGO_ENABLED),)
> +# Default on...
> +CGO_ENABLED:=1
> +ifeq ($(GOARCH),arm)  # ... but not on ARM
> +CGO_ENABLED:=0
> +endif
> +ifeq ($(GOOS),plan9)  # ... and not on Plan 9
> +CGO_ENABLED:=0
> +endif
> +ifeq ($(GOOS),openbsd)  # ... and not on OpenBSD
> +CGO_ENABLED:=0
> +endif
> +endif
> +
>  # Make environment more standard.
>  LANG:=
>  LC_ALL:=C
> @@ -130,11 +145,12 @@
>  export LANG LC_ALL LC_CTYPE GREP_OPTIONS GREP_COLORS
>
>  go-env:
> -       @echo export GOARCH=$(GOARCH)
> -       @echo export GOOS=$(GOOS)
> -       @echo export GOHOSTARCH=$(GOHOSTARCH)
> -       @echo export GOHOSTOS=$(GOHOSTOS)
> -       @echo export O=$O
> +       @echo export GOARCH="$(GOARCH)"
> +       @echo export GOOS="$(GOOS)"
> +       @echo export GOHOSTARCH="$(GOHOSTARCH)"
> +       @echo export GOHOSTOS="$(GOHOSTOS)"
> +       @echo export CGO_ENABLED="$(CGO_ENABLED)"
> +       @echo export O="$O"
>        @echo export AS="$(AS)"
>        @echo export CC="$(CC)"
>        @echo export GC="$(GC)"
> Index: src/pkg/net/Makefile
> ==============================**==============================**=======
> --- a/src/pkg/net/Makefile
> +++ b/src/pkg/net/Makefile
> @@ -36,9 +36,13 @@
>        sendfile_stub.go\
>        sock_bsd.go\
>
> +ifeq ($(CGO_ENABLED),1)
>  CGOFILES_freebsd=\
>        cgo_bsd.go\
> -       cgo_unix.go\
> +       cgo_unix.go
> +else
> +GOFILES_freebsd+=cgo_stub.go
> +endif
>
>  GOFILES_darwin=\
>        dnsclient_unix.go\
> @@ -53,9 +57,13 @@
>        sendfile_stub.go\
>        sock_bsd.go\
>
> +ifeq ($(CGO_ENABLED),1)
>  CGOFILES_darwin=\
>        cgo_bsd.go\
> -       cgo_unix.go\
> +       cgo_unix.go
> +else
> +GOFILES_darwin+=cgo_stub.go
> +endif
>
>  GOFILES_linux=\
>        dnsclient_unix.go\
> @@ -69,6 +77,14 @@
>        sendfile_linux.go\
>        sock_linux.go\
>
> +ifeq ($(CGO_ENABLED),1)
> +CGOFILES_linux=\
> +       cgo_linux.go\
> +       cgo_unix.go
> +else
> +GOFILES_linux+=cgo_stub.go
> +endif
> +
>  GOFILES_openbsd=\
>        dnsclient.go\
>        dnsconfig.go\
> @@ -86,15 +102,6 @@
>        lookup_unix.go\
>        sendfile_stub.go\
>
> -ifeq ($(GOARCH),arm)
> -# ARM has no cgo, so use the stubs.
> -GOFILES_linux+=cgo_stub.go
> -else
> -CGOFILES_linux=\
> -       cgo_linux.go\
> -       cgo_unix.go
> -endif
> -
>  GOFILES_windows=\
>        file_windows.go\
>        interface_windows.go\
> Index: src/pkg/os/user/Makefile
> ==============================**==============================**=======
> --- a/src/pkg/os/user/Makefile
> +++ b/src/pkg/os/user/Makefile
> @@ -8,7 +8,7 @@
>  GOFILES=\
>        user.go\
>
> -ifneq ($(GOARCH),arm)
> +ifeq ($(CGO_ENABLED),1)
>  CGOFILES_linux=\
>        lookup_unix.go
>  CGOFILES_freebsd=\
> Index: src/pkg/os/user/lookup_unix.go
> ==============================**==============================**=======
> --- a/src/pkg/os/user/lookup_unix.**go
> +++ b/src/pkg/os/user/lookup_unix.**go
> @@ -25,6 +25,10 @@
>  */
>  import "C"
>
> +func init() {
> +       implemented = true
> +}
> +
>  // Lookup looks up a user by username. If the user cannot be found,
>  // the returned error is of type UnknownUserError.
>  func Lookup(username string) (*User, os.Error) {
> Index: src/pkg/os/user/user.go
> ==============================**==============================**=======
> --- a/src/pkg/os/user/user.go
> +++ b/src/pkg/os/user/user.go
> @@ -9,6 +9,8 @@
>        "strconv"
>  )
>
> +var implemented = false  // set to true by lookup_unix.go's init
> +
>  // User represents a user account.
>  type User struct {
>        Uid      int // user id
> Index: src/pkg/os/user/user_test.go
> ==============================**==============================**=======
> --- a/src/pkg/os/user/user_test.go
> +++ b/src/pkg/os/user/user_test.go
> @@ -13,8 +13,8 @@
>  )
>
>  func skip(t *testing.T) bool {
> -       if runtime.GOARCH == "arm" {
> -               t.Logf("user: cgo not implemented on arm; skipping tests")
> +       if !implemented {
> +               t.Logf("user: not implemented; skipping tests")
>                return true
>        }
>
> Index: src/pkg/runtime/cgo/Makefile
> ==============================**==============================**=======
> --- a/src/pkg/runtime/cgo/Makefile
> +++ b/src/pkg/runtime/cgo/Makefile
> @@ -4,29 +4,12 @@
>
>  include ../../../Make.inc
>
> -ENABLED:=1
> -
> -ifeq ($(GOARCH),arm)
> -ENABLED:=0
> -endif
> -
> -ifeq ($(GOOS),plan9)
> -ENABLED:=0
> -endif
> -ifeq ($(GOOS),openbsd)
> -ENABLED:=0
> -endif
> -
> -ifeq ($(DISABLE_CGO),1)
> -ENABLED:=0
> -endif
> -
>  TARG=runtime/cgo
>
>  GOFILES=\
>        cgo.go\
>
> -ifeq ($(ENABLED),1)
> +ifeq ($(CGO_ENABLED),1)
>
>  # Unwarranted chumminess with Make.pkg's cgo rules.
>  # Do not try this at home.
> @@ -59,7 +42,7 @@
>
>  include ../../../Make.pkg
>
> -ifeq ($(ENABLED),1)
> +ifeq ($(CGO_ENABLED),1)
>  _cgo_defun.c:
>        echo >$@
>
> Index: src/run.bash
> ==============================**==============================**=======
> --- a/src/run.bash
> +++ b/src/run.bash
> @@ -49,20 +49,20 @@
>  time gomake test
>  ) || exit $?
>
> -[ "$GOARCH" == arm ] ||
> +[ "$CGO_ENABLED" != 1 ] ||
>  [ "$GOHOSTOS" == windows ] ||
>  (xcd ../misc/cgo/stdio
>  gomake clean
>  ./test.bash
>  ) || exit $?
>
> -[ "$GOARCH" == arm ] ||
> +[ "$CGO_ENABLED" != 1 ] ||
>  (xcd ../misc/cgo/life
>  gomake clean
>  ./test.bash
>  ) || exit $?
>
> -[ "$GOARCH" == arm ] ||
> +[ "$CGO_ENABLED" != 1 ] ||
>  [ "$GOHOSTOS" == windows ] ||
>  (xcd ../misc/cgo/test
>  gomake clean
>
>
>

[dsymonds, 2011-08-10 23:18:04]

Why use += in src/pkg/net/Makefile?

[fhs, 2011-08-10 23:26:57]

http://codereview.appspot.com/4859043/diff/6001/src/pkg/net/Makefile
File src/pkg/net/Makefile (right):

http://codereview.appspot.com/4859043/diff/6001/src/pkg/net/Makefile#newcode98
src/pkg/net/Makefile:98: cgo_stub.go\
Maybe you missed this one?

[dsymonds, 2011-08-10 23:51:46]

http://codereview.appspot.com/4859043/diff/6001/src/pkg/net/Makefile
File src/pkg/net/Makefile (right):

http://codereview.appspot.com/4859043/diff/6001/src/pkg/net/Makefile#newcode44
src/pkg/net/Makefile:44: GOFILES_freebsd+=cgo_stub.go
sorry, to clarify, you assign to CGOFILES_freebsd in one branch of the ifeq, but
append to it in the other branch. Same goes for the other OSes.

[rsc, 2011-08-11 01:34:33]

http://codereview.appspot.com/4859043/diff/6001/src/pkg/net/Makefile
File src/pkg/net/Makefile (right):

http://codereview.appspot.com/4859043/diff/6001/src/pkg/net/Makefile#newcode44
src/pkg/net/Makefile:44: GOFILES_freebsd+=cgo_stub.go
On 2011/08/10 23:51:46, dsymonds wrote:
> sorry, to clarify, you assign to CGOFILES_freebsd in one branch of the ifeq,
but
> append to it in the other branch. Same goes for the other OSes.

They are different variables.  One needs to be set; the other needs a +=.

http://codereview.appspot.com/4859043/diff/6001/src/pkg/net/Makefile#newcode98
src/pkg/net/Makefile:98: cgo_stub.go\
On 2011/08/10 23:26:57, fhs wrote:
> Maybe you missed this one?

No, I'm not trying to change anything,
just preserve the current Makefiles.
The OpenBSD port has not gotten here yet.

[rsc, 2011-08-11 01:36:53]

*** Submitted as http://code.google.com/p/go/source/detail?r=cb26571a1d3f ***

build: allow builds without cgo

R=bradfitz, dsymonds, fshahriar
CC=golang-dev
http://codereview.appspot.com/4859043

[dsymonds, 2011-08-11 01:37:08]

LGTM

http://codereview.appspot.com/4859043/diff/6001/src/pkg/net/Makefile
File src/pkg/net/Makefile (right):

http://codereview.appspot.com/4859043/diff/6001/src/pkg/net/Makefile#newcode44
src/pkg/net/Makefile:44: GOFILES_freebsd+=cgo_stub.go
On 2011/08/11 01:34:33, rsc wrote:
> On 2011/08/10 23:51:46, dsymonds wrote:
> > sorry, to clarify, you assign to CGOFILES_freebsd in one branch of the ifeq,
> but
> > append to it in the other branch. Same goes for the other OSes.
> 
> They are different variables.  One needs to be set; the other needs a +=.

Oh, I missed that missing "C".