code review 60840049: lib9: enable on Plan 9

lib9: enable on Plan 9

This change depends on CL 57170052.

[0intro, 2014-02-08 23:02:16]

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

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

[0intro, 2014-02-09 18:22:15]

The file src/plan9/libc.h is mostly an extract from /sys/include/libc.h on
Plan9. I've removed the fmt and utf parts, which are replaced by Go
include/fmt.h and include/utf.h. Do you know what are the licencing issues here?

[rsc, 2014-02-10 16:13:15]

Ignoring the licensing issue (which I think is a problem as well), I don't want
to have to keep a copy of Plan 9's libc.h in sync with the code in the Go repo
(or vice versa). It is like copying glibc's stdio.h into the tree for building
on Linux.

Surely there is a better way.

[0intro, 2014-02-10 20:58:24]

> Surely there is a better way.

Perhaps we could "#define dofmt p9dofmt" every structure, enum
and function from utf and fmt before including /sys/include/libc.h.

Do you have a better solution?

-- 
David du Colombier

[0intro, 2014-02-11 22:36:35]

> Surely there is a better way.

What about calling a script to generate a stripped libc.h at build time?

sed -e '/rune routines/,/^\/\*/d' -e '/print routines/,/^\/\*/d'
/sys/include/libc.h

-- 
David du Colombier

[0intro, 2014-02-12 07:31:03]

Hello brad@fitzpat.com, rsc@golang.org (cc:
golang-codereviews@googlegroups.com),

Please take another look.

[rsc, 2014-02-13 14:53:36]

LGTM

If this works, great. I don't fully understand why it's needed, but it's good
enough.

[0intro, 2014-02-13 16:01:09]

> If this works, great. I don't fully understand why it's needed, but it's
> good enough.

Brad suggested this change because I've noticed some verbs were
interpreted differently between Go lib9/fmt and Plan 9 libc/fmt.

For example, the fixedbugs/issue4085a.go test fails because gc returns
".5" instead of "0.5" on the "constant truncated to integer" error.
This error prints the value using the %F verb (cmd/gc/const.c:370)
which use the %g verb in this case (cmd/gc/mparith1.c:586).
However, the implementation of the %g verb in Go lib9
(src/lib9/fmt/fltfmt.c:/^__efgfmt) differ from the Plan 9 libc
(/sys/src/src/libc/fmt/fltfmt.c:/^_efgfmt).

I've proposed CL 60990044 to fix the test, but Brad suggested to use
Go fmt instead.

# go run run.go -- fixedbugs/issue4085a.go

issue4085a.go:13: no match for `constant 0.5 truncated to
integer|non-integer len argument` in:
issue4085a.go:13: constant .5 truncated to integer
Unmatched Errors:
issue4085a.go:13: constant .5 truncated to integer

FAIL fixedbugs/issue4085a.go 0.103s

-- 
David du Colombier

[rsc, 2014-02-13 16:04:59]

sounds good.

[0intro, 2014-02-13 16:08:32]

> sounds good.

Thanks. I'll will fix the remaining details, then I will submit CL
57170052 and CL 60840049.

-- 
David du Colombier

[0intro, 2014-02-13 18:35:28]

Please take another look. I've updated libc.h.

[rsc, 2014-02-13 18:38:00]

LGTM

https://codereview.appspot.com/60840049/diff/500001/include/fmt.h
File include/fmt.h (right):

https://codereview.appspot.com/60840049/diff/500001/include/fmt.h#newcode20
include/fmt.h:20: #ifndef PLAN9
please make an empty include/plan9/stdarg.h.
then the tree will be free of #ifdef/#ifndef PLAN9.
thanks.

[0intro, 2014-02-13 18:42:16]

> please make an empty include/plan9/stdarg.h.
> then the tree will be free of #ifdef/#ifndef PLAN9.
> thanks.

Should I do the same with errno.h on the CL 57170052?

-- 
David du Colombier

[rsc, 2014-02-13 18:42:56]

sure

[0intro, 2014-02-13 19:06:58]

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

lib9: enable on Plan 9

This change depends on CL 57170052.

LGTM=rsc
R=rsc, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/60840049

[gobot, 2014-02-14 14:54:00]

This CL appears to have broken the windows-386-ec2 builder.