|
|
Descriptionapi: add FreeBSD 10 exceptions
Update issue 7193
Patch Set 1 : diff -r abd51e52924a https://code.google.com/p/go #Patch Set 2 : diff -r abd51e52924a https://code.google.com/p/go #
Total comments: 2
Patch Set 3 : diff -r a3b2bc0b515e https://code.google.com/p/go #MessagesTotal messages: 16
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
Sign in to reply to this message.
Hello golang-codereviews@googlegroups.com, rsc@golang.org, minux.ma@gmail.com (cc: golang-codereviews@googlegroups.com), Please take another look.
Sign in to reply to this message.
ping
Sign in to reply to this message.
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.
Sign in to reply to this message.
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?
Sign in to reply to this message.
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.
Sign in to reply to this message.
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.
Sign in to reply to this message.
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
Sign in to reply to this message.
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.
Sign in to reply to this message.
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
Sign in to reply to this message.
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.
Sign in to reply to this message.
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.
Sign in to reply to this message.
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.
Sign in to reply to this message.
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.
Sign in to reply to this message.
*** 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
Sign in to reply to this message.
|