cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs #26221

steeve · 2018-07-04T17:54:34Z

In Android's NDK16, jobject is now declared as:

#ifdef __cplusplus
class _jobject {};
typedef _jobject*       jobject;
#else /* not __cplusplus */
typedef void*           jobject;
#endif

This makes the jobject to uintptr check fail because it expects the
following definition:

struct _jobject;
typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes #26213

gopherbot · 2018-07-04T17:55:59Z

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-09T16:54:28Z

Message from Keith Randall:

Patch Set 1: Code-Review+2

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-09T17:33:26Z

Message from Ian Lance Taylor:

Patch Set 1:

(4 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T11:57:08Z

Message from Gerrit Bot:

Uploaded patch set 3: New patch set was added with same tree, parent, and commit message as Patch Set 2.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T13:31:56Z

Message from Ian Lance Taylor:

Patch Set 3:

(5 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T14:48:32Z

Message from Steeve Morin:

Patch Set 3:

(5 comments)

Woops, had some unpublished drafts.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T15:00:45Z

Message from Steeve Morin:

Patch Set 3:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T17:01:48Z

Message from Gerrit Bot:

Uploaded patch set 5: New patch set was added with same tree, parent, and commit message as Patch Set 4.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T18:30:30Z

Message from Ian Lance Taylor:

Patch Set 5:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T18:34:42Z

Message from Steeve Morin:

Patch Set 5:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T18:43:59Z

Message from Gerrit Bot:

Uploaded patch set 7: New patch set was added with same tree, parent, and commit message as Patch Set 6.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T19:59:35Z

Message from Ian Lance Taylor:

Patch Set 7: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T19:59:54Z

Message from Gobot Gobot:

Patch Set 7:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=6cce751a

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T20:06:56Z

Message from Gobot Gobot:

Patch Set 7:

Build is still in progress...
This change failed on openbsd-amd64-62:
See https://storage.googleapis.com/go-build-log/6cce751a/openbsd-amd64-62_099d345b.log

Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T20:07:24Z

Message from Gobot Gobot:

Patch Set 7: TryBot-Result-1

1 of 19 TryBots failed:
Failed on openbsd-amd64-62: https://storage.googleapis.com/go-build-log/6cce751a/openbsd-amd64-62_099d345b.log

Consult https://build.golang.org/ to see whether they are new failures.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-11T20:33:54Z

Message from Ian Lance Taylor:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T00:00:12Z

Message from Steeve Morin:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T01:11:25Z

Message from Ian Lance Taylor:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T10:51:39Z

Message from Gerrit User 9540:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T11:04:00Z

Message from Gerrit User 12446:

Uploaded patch set 9: New patch set was added with same tree, parent, and commit message as Patch Set 8.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T19:47:55Z

Message from Gerrit User 5206:

Patch Set 9: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T19:48:10Z

Message from Gerrit User 5976:

Patch Set 9:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=ab1da42b

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T19:54:58Z

Message from Gerrit User 5976:

Patch Set 9:

Build is still in progress...
This change failed on openbsd-amd64-62:
See https://storage.googleapis.com/go-build-log/ab1da42b/openbsd-amd64-62_32192d62.log

Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T19:56:14Z

Message from Gerrit User 5976:

Patch Set 9: TryBot-Result-1

1 of 19 TryBots failed:
Failed on openbsd-amd64-62: https://storage.googleapis.com/go-build-log/ab1da42b/openbsd-amd64-62_32192d62.log

Consult https://build.golang.org/ to see whether they are new failures.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T20:01:09Z

Message from Gerrit User 5206:

Patch Set 9:

The trybot failure seems to indicate a problem with the C++ portion of the test. Is the test running, and passing, on your system?

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T22:28:53Z

Message from Gerrit User 9540:

Patch Set 9:

Patch Set 9:

The trybot failure seems to indicate a problem with the C++ portion of the test. Is the test running, and passing, on your system?

It is. What is even weirder is that this is precisely the case this CL is trying to solve. Is OpenBSD the only system failing or the first one ?

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T22:29:33Z

Message from Gerrit User 5206:

Patch Set 9:

The trybot failure seems to indicate a problem with the C++ portion of the test. Is the test running, and passing, on your system?

It is. What is even weirder is that this is precisely the case this CL is trying to solve. Is OpenBSD the only system failing or the first one ?

OpenBSD is the only system failing, but I don't know which of the trybots have a C++ compiler installed. It is possible that of the trybots that have a C++ compiler installed (if any) that OpenBSD is the only one that uses clang rather than g++.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T22:30:44Z

Message from Gerrit User 9540:

Patch Set 9:

Patch Set 9:

Patch Set 9:

The trybot failure seems to indicate a problem with the C++ portion of the test. Is the test running, and passing, on your system?

It is. What is even weirder is that this is precisely the case this CL is trying to solve. Is OpenBSD the only system failing or the first one ?

Just to be sure I've re-run the test suite, on darwin_amd64:

../misc/cgo/test

PASS
scatter = 0x4141fe0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.107s
PASS
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test/issue26213/cxx 0.006s
PASS
scatter = 0x10025a0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.091s
PASS
scatter = 0x4141fe0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.095s
PASS
scatter = 0x4141fe0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.121s

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-12T22:30:45Z

Message from Gerrit User 9540:

Patch Set 9:

Patch Set 9:

Patch Set 9:

Patch Set 9:

The trybot failure seems to indicate a problem with the C++ portion of the test. Is the test running, and passing, on your system?

It is. What is even weirder is that this is precisely the case this CL is trying to solve. Is OpenBSD the only system failing or the first one ?

Just to be sure I've re-run the test suite, on darwin_amd64:

../misc/cgo/test

PASS
scatter = 0x4141fe0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.107s
PASS
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test/issue26213/cxx 0.006s
PASS
scatter = 0x10025a0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.091s
PASS
scatter = 0x4141fe0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.095s
PASS
scatter = 0x4141fe0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.121s

I'm going to try on Linux.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-17T13:03:50Z

Message from Gerrit User 7435:

Patch Set 9:

Patch Set 9:

Patch Set 9:

Patch Set 9:

Patch Set 9:

The trybot failure seems to indicate a problem with the C++ portion of the test. Is the test running, and passing, on your system?

It is. What is even weirder is that this is precisely the case this CL is trying to solve. Is OpenBSD the only system failing or the first one ?

Just to be sure I've re-run the test suite, on darwin_amd64:

../misc/cgo/test

PASS
scatter = 0x4141fe0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.107s
PASS
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test/issue26213/cxx 0.006s
PASS
scatter = 0x10025a0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.091s
PASS
scatter = 0x4141fe0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.095s
PASS
scatter = 0x4141fe0
hello from C
sqrt is: 0
ok _/Users/steeve/go/src/github.com/znly/go/misc/cgo/test 1.121s

I'm going to try on Linux.

Ping. It would be great to have this fix in time for the next beta release.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-17T14:01:19Z

Message from Gerrit User 5206:

Patch Set 9:

Let's make a version of this CL without the C++ test, so that we get it in without having to worry about the testing issues. Then we can handle the C++ test separately. Steeve, do you want to do that? Thanks.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-17T15:45:56Z

Message from Gerrit User 9540:

Patch Set 9:

Patch Set 9:

Let's make a version of this CL without the C++ test, so that we get it in without having to worry about the testing issues. Then we can handle the C++ test separately. Steeve, do you want to do that? Thanks.

Sounds good to me. I'll submit a CL without the C++ tests.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

In Android's NDK16, jobject is now declared as: #ifdef __cplusplus class _jobject {}; typedef _jobject* jobject; #else /* not __cplusplus */ typedef void* jobject; #endif This makes the jobject to uintptr check fail because it expects the following definition: struct _jobject; typedef struct _jobject *jobject; Update the type check to handle that new type definition in both C and C++ modes. Fixes #26213 Signed-off-by: Steeve Morin <steeve.morin@gmail.com>

gopherbot · 2018-07-17T16:24:49Z

Message from Gerrit User 5206:

Patch Set 10: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-17T16:24:50Z

Message from Gerrit User 5976:

Patch Set 10:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=529af54d

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-17T16:32:30Z

Message from Gerrit User 5976:

Patch Set 10: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-17T16:51:54Z

Message from Gerrit User 5206:

Patch Set 10:

Thanks!

Please don’t reply on this GitHub thread. Visit golang.org/cl/122217.
After addressing review feedback, remember to publish your drafts!

In Android's NDK16, jobject is now declared as: #ifdef __cplusplus class _jobject {}; typedef _jobject* jobject; #else /* not __cplusplus */ typedef void* jobject; #endif This makes the jobject to uintptr check fail because it expects the following definition: struct _jobject; typedef struct _jobject *jobject; Update the type check to handle that new type definition in both C and C++ modes. Fixes #26213 Change-Id: Ic36d4a5176526998d2d5e4e404f8943961141f7a GitHub-Last-Rev: 42037c3 GitHub-Pull-Request: #26221 Reviewed-on: https://go-review.googlesource.com/122217 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>

gopherbot · 2018-07-17T16:57:13Z

This PR is being closed because golang.org/cl/122217 has been merged.

googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Jul 4, 2018

steeve mentioned this pull request Jul 4, 2018

cmd/cgo: jobject is not mapped to uintptr if building with the Android NDK #26213

Closed

gopherbot force-pushed the master branch from 12ed0dd to 9300d22 Compare July 11, 2018 20:25

gopherbot closed this Jul 17, 2018

steeve deleted the fix.jobject branch October 2, 2018 10:36

cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs #26221

cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs #26221

Conversation

steeve commented Jul 4, 2018 • edited

gopherbot commented Jul 4, 2018

gopherbot commented Jul 9, 2018

gopherbot commented Jul 9, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 11, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

gopherbot commented Jul 12, 2018

../misc/cgo/test

gopherbot commented Jul 12, 2018

../misc/cgo/test

gopherbot commented Jul 17, 2018

../misc/cgo/test

gopherbot commented Jul 17, 2018

gopherbot commented Jul 17, 2018

gopherbot commented Jul 17, 2018

gopherbot commented Jul 17, 2018

gopherbot commented Jul 17, 2018

gopherbot commented Jul 17, 2018

gopherbot commented Jul 17, 2018

steeve commented Jul 4, 2018 •

edited