code review 14604047: build: do not use the host's libbio on Plan 9

build: do not use the host's libbio on Plan 9

[ality, 2013-10-11 17:10:05]

Hello jas@corpus-callosum.com, lucio.dere@gmail.com (cc:
golang-dev@googlegroups.com),

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

[lucio, 2013-10-12 04:39:03]

Sorry to be a pain, but could you submit the Bison fixes separately?
I think it would make the history simpler to follow.

I also don't much like the change to bseek(), but I don't think it's
the only place where we condone the use of "#if !defined", so I'd
rather not be the lone stick in the mud.

Lucio.


On 10/11/13, ality@pbrane.org <ality@pbrane.org> wrote:
> Reviewers: jas, lucio,
>
> Message:
> Hello jas@corpus-callosum.com, lucio.dere@gmail.com (cc:
> golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> build: do not use the host's libbio on Plan 9
>
> Please review this at https://codereview.appspot.com/14604047/
>
> Affected files (+18, -25 lines):
>    M include/bio.h
>    A include/plan9/bio.h
>    M src/cmd/dist/build.c
>    M src/libbio/bgetrune.c
>    M src/libbio/bprint.c
>    M src/libbio/bputrune.c
>    M src/libbio/bseek.c
>
>
> Index: include/bio.h
> ===================================================================
> --- a/include/bio.h
> +++ b/include/bio.h
> @@ -33,7 +33,9 @@
>   AUTOLIB(bio)
>   #endif
>
> +#ifndef PLAN9
>   #include <fcntl.h>	/* for O_RDONLY, O_WRONLY */
> +#endif
>
>   typedef	struct	Biobuf	Biobuf;
>
> Index: include/plan9/bio.h
> ===================================================================
> new file mode 100644
> --- /dev/null
> +++ b/include/plan9/bio.h
> @@ -0,0 +1,8 @@
> +// Copyright 2013 The Go Authors. All rights reserved.
> +// Use of this source code is governed by a BSD-style
> +// license that can be found in the LICENSE file.
> +
> +#include "../bio.h"
> +
> +#define fmtcharstod charstod
> +#define lseek seek
> Index: src/cmd/dist/build.c
> ===================================================================
> --- a/src/cmd/dist/build.c
> +++ b/src/cmd/dist/build.c
> @@ -613,7 +613,7 @@
>   	Buf b, b1, path;
>   	Vec compile, files, link, go, missing, clean, lib, extra;
>   	Time ttarg, t;
> -	int i, j, k, n, doclean, targ, usecpp;
> +	int i, j, k, n, doclean, targ;
>
>   	if(vflag) {
>   		if(!streq(goos, gohostos) || !streq(goarch, gohostarch))
> @@ -763,8 +763,6 @@
>   				bsubst(&b1, "$GOARCH", goarch);
>   				p = bstr(&b1);
>   				if(hassuffix(p, ".a")) {
> -					if(streq(gohostos, "plan9") && hassuffix(p, "libbio.a"))
> -						continue;
>   					vadd(&lib, bpathf(&b, "%s", p));
>   					continue;
>   				}
> @@ -916,20 +914,6 @@
>   		goto nobuild;
>   	}
>
> -	// The files generated by GNU Bison use macros that aren't
> -	// supported by the Plan 9 compilers so we have to use the
> -	// external preprocessor when compiling.
> -	usecpp = 0;
> -	if(streq(gohostos, "plan9")) {
> -		for(i=0; i<files.len; i++) {
> -			p = files.p[i];
> -			if(hassuffix(p, "y.tab.c") || hassuffix(p, "y.tab.h")){
> -				usecpp = 1;
> -				break;
> -			}
> -		}
> -	}
> -
>   	// Compile the files.
>   	for(i=0; i<files.len; i++) {
>   		if(!hassuffix(files.p[i], ".c") && !hassuffix(files.p[i], ".s"))
> @@ -941,9 +925,10 @@
>   			// C library or tool.
>   			if(streq(gohostos, "plan9")) {
>   				vadd(&compile, bprintf(&b, "%sc", gohostchar));
> -				vadd(&compile, "-FTVw");
> -				if(usecpp)
> -					vadd(&compile, "-Bp+");
> +				vadd(&compile, "-FTVwp");
> +				vadd(&compile, "-D__STDC__=1");
> +				vadd(&compile, "-D__SIZE_TYPE__=ulong"); // for GNU Bison
> +				vadd(&compile, "-DPLAN9");
>   				vadd(&compile, bpathf(&b, "-I%s/include/plan9", goroot));
>   				vadd(&compile, bpathf(&b, "-I%s/include/plan9/%s", goroot,
> gohostarch));
>   				// Work around Plan 9 C compiler's handling of #include with .. path.
> @@ -1150,8 +1135,6 @@
>   			if(!contains(name, "plan9"))
>   				return 0;
>   		}
> -		if(streq(dir, "libbio"))
> -			return 0;
>   	}
>   	
>   	// Check file name for GOOS or GOARCH.
> Index: src/libbio/bgetrune.c
> ===================================================================
> --- a/src/libbio/bgetrune.c
> +++ b/src/libbio/bgetrune.c
> @@ -26,7 +26,6 @@
>   #include	<u.h>
>   #include	<libc.h>
>   #include	<bio.h>
> -#include	<utf.h>
>
>   long
>   Bgetrune(Biobuf *bp)
> Index: src/libbio/bprint.c
> ===================================================================
> --- a/src/libbio/bprint.c
> +++ b/src/libbio/bprint.c
> @@ -67,7 +67,9 @@
>   	Fmt f;
>   	
>   	memset(&f, 0, sizeof f);
> +#ifndef PLAN9
>   	fmtlocaleinit(&f, nil, nil, nil);
> +#endif
>   	f.stop = bp->ebuf;
>   	f.to = (char*)f.stop + bp->ocount;
>   	f.flush = bflush;
> Index: src/libbio/bputrune.c
> ===================================================================
> --- a/src/libbio/bputrune.c
> +++ b/src/libbio/bputrune.c
> @@ -26,7 +26,6 @@
>   #include	<u.h>
>   #include	<libc.h>
>   #include	<bio.h>
> -#include	<utf.h>
>
>   int
>   Bputrune(Biobuf *bp, long c)
> Index: src/libbio/bseek.c
> ===================================================================
> --- a/src/libbio/bseek.c
> +++ b/src/libbio/bseek.c
> @@ -33,7 +33,7 @@
>   	vlong n, d;
>   	int bufsz;
>
> -#ifndef _WIN32
> +#if !defined(_WIN32) && !defined(PLAN9)
>   	if(sizeof(offset) != sizeof(off_t)) {
>   		fprint(2, "Bseek: libbio compiled with %d-byte offset\n",
> sizeof(off_t));
>   		abort();
>
>
>

[jas, 2013-10-14 20:48:59]

Doesn't the non-Plan 9 version require <utf.h>?

I'm not pleased with more #ifdefs, but I guess that's the world we live in now.

[ality, 2013-10-15 03:27:13]

jas@corpus-callosum.com once said:
> Doesn't the non-Plan 9 version require <utf.h>?

It does, but not in libbio. The libc.h for lib9 includes it.

> I'm not pleased with more #ifdefs, but I guess that's the
> world we live in now.

The inclusion of fcntl.h in bio.h turned out to be entirely
unnecessary since it's already include by the u.h for lib9.

I've removed it.

The only ifdef addition is now around fmtlocaleinit. If you
know of a clean way to get around that one, I'm all ears.

Cheers,
  Anthony

[rsc, 2013-12-09 23:39:28]

LGTM

[rsc, 2013-12-09 23:40:01]

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

build: do not use the host's libbio on Plan 9

R=jas, lucio.dere, rsc
CC=golang-dev
https://codereview.appspot.com/14604047

Committer: Russ Cox <rsc@golang.org>