Code review - Issue 57210043: code review 57210043: api: add FreeBSD 10 exceptionshttps://codereview.appspot.com/2014-03-04T10:03:40+00:00rietveld
Message from mikioh.mikioh@gmail.com
2014-02-26T06:55:44+00:00mikiourn:md5:dff5f736c5db21e2f9ef08c9374bf779
Hello golang-codereviews@googlegroups.com (cc: golang-codereviews@googlegroups.com),
I'd like you to review this change to
https://code.google.com/p/go
Message from unknown
2014-02-27T14:10:14+00:00mikiourn:md5:b7f4aba40028980401dedbbe8f85838f
Message from unknown
2014-02-27T15:49:30+00:00mikiourn:md5:9fae5aabfb13c6293ce11c49be925d68
Message from mikioh.mikioh@gmail.com
2014-02-27T15:49:39+00:00mikiourn:md5:b07eda002865d0b81e99cd734404ca83
Hello golang-codereviews@googlegroups.com, rsc@golang.org, minux.ma@gmail.com (cc: golang-codereviews@googlegroups.com),
Please take another look.
Message from mikioh.mikioh@gmail.com
2014-03-02T01:32:36+00:00mikiourn:md5:2683415f80a8c5f08d9bb3913bf4c189
ping
Message from iant@golang.org
2014-03-02T19:16:55+00:00ianturn:md5:010b8fe769ffe4759356c327346fd7d1
My understanding is that only the values that are changing for good reason should be in except.txt. New structs and new values should be in next.txt.
Message from mikioh.mikioh@gmail.com
2014-03-02T21:53:15+00:00mikiourn:md5:0f63e3b9a4a35f718070b97355e342bb
On 2014/03/02 19:16:55, iant wrote:
> My understanding is that only the values that are changing for good reason
> should be in except.txt. New structs and new values should be in next.txt.
Sure, this CL doesn't contain such stuff. Can you please ellaborate, which entry is bad?
Message from mikioh.mikioh@gmail.com
2014-03-03T00:52:11+00:00mikiourn:md5:d5662ef97fd25b94192119614356ad64
On 2014/03/02 19:16:55, iant wrote:
> New structs and new values should be in next.txt.
if you are worrying about freebsd/arm stuff, it's not new. unfortunately go1.1.txt and go1.2.txt contain freebsd/arm stuff generated on 8 or 9? kernel (not sure who made it) w/ old-ABI.
Message from iant@golang.org
2014-03-03T06:48:13+00:00ianturn:md5:9767aff511b0976fe052fd2b3102c04e
I didn't check every value, but there a couple of comments.
https://codereview.appspot.com/57210043/diff/200001/api/except.txt
File api/except.txt (right):
https://codereview.appspot.com/57210043/diff/200001/api/except.txt#newcode339
api/except.txt:339: pkg syscall (freebsd-arm), const BIOCGRTIMEOUT = 1074545262
BIOCGRTIMEOUT is already in go1.1.txt with the same value. Why is it here too? Same for BIOCSRTIMEOUT.
https://codereview.appspot.com/57210043/diff/200001/api/except.txt#newcode346
api/except.txt:346: pkg syscall (freebsd-arm), const SYS_CAP_FCNTLS_GET = 537
SYS_CAP_FCNTLS_GET is already in go1.1.txt with the same value. Why is it here too? Same for other SYS_CAP_FCNTLS values.
Message from mikioh.mikioh@gmail.com
2014-03-03T07:11:18+00:00mikiourn:md5:f05624d4f92877e05ebd647cd68e6cfd
Thanks. I forgot to say that these exceptions are just for applying CL 56770044.
On Mon, Mar 3, 2014 at 3:48 PM, <iant@golang.org> wrote:
> https://codereview.appspot.com/57210043/diff/200001/api/except.txt#newcode339
> api/except.txt:339: pkg syscall (freebsd-arm), const BIOCGRTIMEOUT =
> 1074545262
> BIOCGRTIMEOUT is already in go1.1.txt with the same value. Why is it
> here too? Same for BIOCSRTIMEOUT.
Because those numbers are changed for FreeBSD 10 adaptation. They
consist of IOC number, sizeof IO structure (yup, depends on ABI), and
something.
See https://codereview.appspot.com/56770044/patch/750001/760007
> api/except.txt:346: pkg syscall (freebsd-arm), const SYS_CAP_FCNTLS_GET
> = 537
> SYS_CAP_FCNTLS_GET is already in go1.1.txt with the same value. Why is
> it here too? Same for other SYS_CAP_FCNTLS values.
As we discussed on golang-dev, even on freebsd/arm syscall numbers for
Capsicum are based on 9-STABLE.
See https://codereview.appspot.com/56770044/patch/750001/760010
Message from minux.ma@gmail.com
2014-03-03T08:38:00+00:00minux1urn:md5:40eb12dfe1c39c72f057ff85097fafe3
On Sun, Mar 2, 2014 at 7:52 PM, <mikioh.mikioh@gmail.com> wrote:
> On 2014/03/02 19:16:55, iant wrote:
>> New structs and new values should be in next.txt.
> if you are worrying about freebsd/arm stuff, it's not new. unfortunately
> go1.1.txt and go1.2.txt contain freebsd/arm stuff generated on 8 or 9?
> kernel (not sure who made it) w/ old-ABI.
should be me. When I developed freebsd/arm support, 9 is current.
The syscall stuff is taking up most of except.txt (314 lines of the total 321
lines), which suggests that we probably should just blacklist the whole
package, or re-organize the way it is stored, e.g.
review current syscall changes, remove all syscall lines from go1.txt
and go1.1.txt, then add a new file for just the syscall package; because
some of the except lines are not strictly necessary: they are due to
limitations of cmd/api.
For example:
In go 1, the syscall package on all systems support some const A.
so there is a line like this:
package syscall, const A ideal-int
then we add another system B that doesn't have A. suddenly the A
line is wrong, because B doesn't have A.
So it has to be added to the except.txt but in fact the APIs in Go 1
are still there.
For freebsd/arm, I don't think it's used by a lot of people. we probably
should just upgrade it to EABI. Recently I spent some time update
the NetBSD/ARM builder, and NetBSD is also starting to migrate to
EABI, so the situation is similar to FreeBSD.
Both FreeBSD/ARM and NetBSD/ARM are added in Go 1.1 cycle.
The only difference is that we've never said NetBSD is officially
supported in doc/install.html. I guess I will just remove the NetBSD/ARM
lines from go1.1.txt, and re-add the new ones to go1.3.txt.
I think we might be be able to do something similar for FreeBSD/ARM.
As discussed on the golang-dev thread, we definitely should try our
best to keep the API of FreeBSD 386 and amd64 consistent though.
Message from rsc@golang.org
2014-03-03T16:52:50+00:00rscurn:md5:2eb53fea59410dd750128d0fe782d62f
I would much rather list the differences than add some kind of wildcard for
package syscall. The syscall constants are not supposed to be changing
either. FreeBSD just sucks about backward compatiblity and is forcing us to
do terrible things.
Russ
Message from mikioh.mikioh@gmail.com
2014-03-03T17:34:43+00:00mikiourn:md5:75f8c5db8b49b858a8d93ff66f5b4ac7
I'm fine either way. minux?
On Tue, Mar 4, 2014 at 1:50 AM, Russ Cox <rsc@golang.org> wrote:
> I would much rather list the differences than add some kind of wildcard for
> package syscall. The syscall constants are not supposed to be changing
> either. FreeBSD just sucks about backward compatiblity and is forcing us to
> do terrible things.
>
> Russ
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
Message from minux.ma@gmail.com
2014-03-03T19:30:48+00:00minux1urn:md5:19abc38357bf952c28679d17428e9a77
On Mon, Mar 3, 2014 at 12:34 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
> I'm fine either way. minux?
LGTM. I suggest we remove the cap stuff from syscall for 1.3
so that there is time for FreeBSD to finalize them.
Message from minux.ma@gmail.com
2014-03-03T19:31:26+00:00minux1urn:md5:e04ab5e4ad8dc4dea76cf50980bfc96c
On Mar 3, 2014 11:52 AM, "Russ Cox" <rsc@golang.org> wrote:
> I would much rather list the differences than add some kind of wildcard
for package syscall. The syscall constants are not supposed to be changing
either. FreeBSD just sucks about backward compatiblity and is forcing us to
do terrible things.
ok. i think the except list is slowly growing to be unmaintainable.
i'd also like to propose this change to cmd/api: include some info about
what "all platforms" means in api.txt, to fix the problem i mentioned.
Message from mikioh.mikioh@gmail.com
2014-03-03T22:13:03+00:00mikiourn:md5:3a16a589300ed7d208f96def8bf1cee3
On Tue, Mar 4, 2014 at 4:30 AM, minux <minux.ma@gmail.com> wrote:
> LGTM. I suggest we remove the cap stuff from syscall for 1.3
> so that there is time for FreeBSD to finalize them.
I'm no keen on this, let's leave it as it is. Also I don't say
"syscall for freebsd is finalized", because we're not sure what will
happen in freebsd11.
Message from unknown
2014-03-04T00:26:22+00:00mikiourn:md5:fa94d82a52e30971c9fd0675dc48c742
Message from mikioh.mikioh@gmail.com
2014-03-04T00:26:35+00:00mikiourn:md5:c1d881bc1e0e08ba8a2d828fd404fa5c
*** Submitted as https://code.google.com/p/go/source/detail?r=c12c08ac4dc7 ***
api: add FreeBSD 10 exceptions
Update issue 7193
LGTM=minux.ma
R=golang-codereviews, rsc, minux.ma, iant
CC=golang-codereviews
https://codereview.appspot.com/57210043
Message from gobot@golang.org
2014-03-04T10:03:40+00:00goboturn:md5:e3a694cccf75a4187bb78477f7a553e7
This CL appears to have broken the darwin-amd64-race-cheney builder.