Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make gokogiri compile with go 1.6 #93

Closed
wants to merge 4 commits into from
Closed

Make gokogiri compile with go 1.6 #93

wants to merge 4 commits into from

Conversation

nightlyone
Copy link

In Go 1.6 it is basically forbidden to pass a Go pointer to Go functions that are used as callbacks from C.

Fix this by funneling those pointers through global variables.

Fixes #92

Ingo Oeser added 4 commits March 17, 2016 13:26
So we only check X == Y where
  * X is the version we are compiled against
  * Y is the version we are dynamically linked against on startup
by passing the callback argument (a Go pointer) via global variable.
This allows only one SetContent to happen at any time in the program,
but works for now.
by passing the WriteBuffer in a global variable
since callbacks in Go1.6 ar not allowed to take Go pointers
@nightlyone
Copy link
Author

Ping, and feedback on this?

@jbowtie
Copy link
Contributor

jbowtie commented Mar 30, 2016

Looks like a reasonable approach to me, but I haven't yet had a chance to test it.

@andreastt
Copy link

I can confirm that this resolves the same issue I was having with gokogiri on FreeBSD as well as Linux.

@jbowtie
Copy link
Contributor

jbowtie commented Apr 28, 2016

I've incorporated this into my fork, and it works fine on both 1.5 and 1.6 as far as I can tell.

@andreastt
Copy link

I should add that it’s possible to work around the runtime checks in Go 1.6 by using the GODEBUG=cgocheck=0 environmental variable.

@nightlyone
Copy link
Author

Any feedback on this from you @cin210 or is the fork from @jbowtie now the official one and this one is orphaned?

afeld added a commit to opencontrol/fedramp-templater that referenced this pull request May 27, 2016
@nightlyone
Copy link
Author

OK, will close it, since everyone interested in a maintained package uses the fork from @jbowtie anyway 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants