Consider making glfw3 Go package go-gettable by vendoring GLFW inside?

[dmitshur]

@slimsag was able to make the glfw3 Go package go-gettable by including GLFW sources inside. See azul3d-legacy/issues#18 (comment).

If that really works, I think it's a great idea and we should consider doing that. Either as a separate package, or simply modify glfw3 to do it.

Any reasons not to do that?

Advantages:

• go-gettable Go package, much easier to use and install (especially for people trying to go get Go projects that import glfw3).

Disadvntages:

• More work for us, since we'll need to maintain and update a working version of GLFW C source.
• Less configurable? Unable to modify CMAKE parameters?
  • Are there any things that are worth modifying? I never changed anything besides the static build to make it compile, that's it.

[dmitshur]

I tried go getting it on OS X,

~ $ go get -u github.com/slimsag/glfw3
# github.com/slimsag/glfw3
src/github.com/slimsag/glfw3/native_darwin.go:13:35: struct size calculation error off=8 bytesize=0

But I'm sure that's easy to fix.

[slimsag]

Hey! Thanks for supporting this idea! From the issue you linked:

I think that totally deserves consideration to be made into a go-gl package, like glfw3 (or in place of glfw3). It can embed GLFW via git submodules, and users would only have to go-get it, no other dependencies.

One annoying thing is that I don't think go-get will checkout Git submodules, so we actually have to commit the sources not as a submodule (which IMHO is easier to do anyway), basically just:

cd $GOPATH/src/github.com/go-gl/glfw3
git clone https://github.com/glfw/glfw
git add glfw/

Note that the trailing slash in git add glfw/ is critical, if you leave off the trailing slash Git adds the folder as a submodule instead.

More work for us, since we'll need to maintain and update a working version of GLFW C source.

I do agree with this, but I think it can be worth it. Since the way I did it was by actually #including the C sources and using for example #if defined(_GLFW_X11) to guard the sources on platforms that don't need them, then if we want to update the GLFW3 source code, all we have to do each time would be:

cd $GOPATH/src/github.com/go-gl/glfw3
rm -rf glfw/
git clone https://github.com/glfw/glfw
git add glfw/

The removal of the folder is needed because it can't be added as a submodule for go-get.

Less configurable? Unable to modify CMAKE parameters?

Actually -- We can make build tags for any CMAKE configurable feature that we want (e.g. tell people to compile their application with go install -tags "wayland" my/pkg and a wayland client will be built instead of an X11 one, you can see how I did this here: https://github.com/slimsag/glfw3/blob/master/build.go)

Although -- the test will be finding which flags we want on what os/arch's so that we can add them and make sure they work -- for instance the linker options for -tags "wayland" probably are incorrect, I haven't tested them yet.

Are there any things that are worth modifying? I never changed anything besides the static build to make it compile, that's it.

This is one reason why I think we should do it, a lot of people don't configure anything. If someone did need a configuration option, they could add an issue and we could add it, or they could even modify the build.go file themselves by just looking over GLFW's CMAKE file to see what it defines for the option they want.

[slimsag]

I tried go getting it on OS X,

Sigh I really need to get myself some Apple. I can't verify anything just yet, but it looks like the true issue at hand could very well be a bug with Go itself as these all seem similar:

https://golang.org/issue/6472
https://golang.org/issue/6368
https://golang.org/issue/6368
https://golang.org/issue/3505

But I think that perhaps there could be a different issue at hand. Could you dump the output of go get -u -v -x github.com/slimsag/glfw3 for me here?

[dmitshur]

Could you dump the output of go get -u -v -x github.com/slimsag/glfw3 for me here?

Nah, I don't wanna make you try to debug things blindly, that wouldn't be nice. Let me look into this. It might just be an issue with independent of embedding GLFW source into glfw3. Which revision of GLFW did you use?

[dmitshur]

In any case, it seems likely to be a cgo bug.

~ $ go get -u -v -x github.com/slimsag/glfw3
github.com/slimsag/glfw3 (download)
cd /tmp/try-2/src/github.com/slimsag/glfw3
git symbolic-ref HEAD
cd /tmp/try-2/src/github.com/slimsag/glfw3
git pull --ff-only
cd /tmp/try-2/src/github.com/slimsag/glfw3
git show-ref
cd /tmp/try-2/src/github.com/slimsag/glfw3
git checkout master
WORK=/var/folders/tw/kgz4v2kn4n7d7ryg5k_z3dk40000gn/T/go-build094467281
github.com/slimsag/glfw3
mkdir -p $WORK/github.com/slimsag/glfw3/_obj/
mkdir -p $WORK/github.com/slimsag/
cd /tmp/try-2/src/github.com/slimsag/glfw3
CGO_LDFLAGS="-g" "-O2" "-framework" "Cocoa" "-framework" "OpenGL" "-framework" "IOKit" "-framework" "CoreVideo" /usr/local/go/pkg/tool/darwin_amd64/cgo -objdir $WORK/github.com/slimsag/glfw3/_obj/ -- -I $WORK/github.com/slimsag/glfw3/_obj/ -D_GLFW_USE_OPENGL -D_GLFW_COCOA -D_GLFW_NSGL -x objective-c -x objective-c -x objective-c -x objective-c -x objective-c -x objective-c build.go clipboard.go cocoaclipboard_darwin.go cocoainit_darwin.go cocoamonitor_darwin.go cocoawindow_darwin.go context.go error.go glfw.go input.go iokitjoystick_darwin.go monitor.go native_darwin.go nsglcontext_darwin.go time.go util.go window.go
# github.com/slimsag/glfw3
/private/tmp/try-2/src/github.com/slimsag/glfw3/native_darwin.go:13:35: struct size calculation error off=8 bytesize=0

I can try to debug this.

[dmitshur]

The following change makes github.com/slimsag/glfw3 compile and work (tested) on OS X!

diff --git a/native_darwin.go b/native_darwin.go
index aee8b98..126b3a5 100644
--- a/native_darwin.go
+++ b/native_darwin.go
@@ -6,10 +6,10 @@ package glfw3
 //#include "glfw/include/GLFW/glfw3native.h"
 import "C"

-func (w *Window) GetCocoaWindow() C.id {
+/*func (w *Window) GetCocoaWindow() C.id {
    return C.glfwGetCocoaWindow(w.data)
-}
+}*/

-func (w *Window) GetNSGLContext() C.id {
+/*func (w *Window) GetNSGLContext() C.id {
    return C.glfwGetNSGLContext(w.data)
-}
+}*/

It is possibly a bug in cgo related to more recent Objective-C support (or an issue with the code/usage).

Wow. I still can't believe this actually works. This approach will allow glfw3 (or a version thereof) to have its own release schedule AND be go-gettable. With that, my Conception-go project becomes 100% go-gettable. :D

[slimsag]

The following change....

I've gone ahead and added that fix for now, and I am going to submit a pull request where we can do further discussion etc.

Which revision of GLFW did you use?

Very good point. We can track it in the changelog so other people know it too.

In slimsag@fe4551c I make mention of it in the changelog, but it's the wrong revision! I am sorry I messed up! So in slimsag@0b95038 I updated the GLFW sources to the revision actually listed in the changelog. Sorry for any confusion.

Could you test again on OS X and ensure that the new GLFW sources compiler there OK?

[dmitshur]

Could you test again on OS X and ensure that the new GLFW sources compiler there OK?

Confirmed to be working as of posting this.

[slimsag]

I think this can be closed now that #84 is merged.

[tapir]

@slimsag

In build.go I think we lack _-lwinmm_ for Windows target

Also in cmake config below options are set to _ON_ by default. I don't know how cmake works so maybe we should define these also?

GLFW_USE_CHDIR
GLFW_USE_MENUBAR
GLFW_USE_RETINA

Edit: Can you also elaborate how wayland tag works. I don't think go has such an identifier?
Edit2: Line 5 and 6 can be merged into a single line like this: #cgo 386 amd64 CFLAGS: -D_GLFW_USE_OPENGL

[tapir]

Let's continue about the issue mentioned in #8 here.

• I've upgraded MingW to latest: _gcc 4.8.1_, 32 bit
• Using Go version 1.3.1, 32 bit
• I'm on Windows 7 64 bit

Note that, the go get works without issues, it's only when I compile another code referencing to the devel_glfw3.1 that I get below error.

Quick google search shows that we may lack #include <string.h> somewhere.

[slimsag]

Sigh. I think it's a bug in Go:

• https://code.google.com/p/go/issues/detail?id=7555

I've ran into a few issues like this before -- I think it is because Go does not use gcc for the final link on Windows yet.

EDIT: Also, Mozilla has had some trouble with it too:

• Windows Build: _get_output_format(0): not defined mozilla-services/heka#439
• Problem in building on Windows mozilla-services/heka#443

[slimsag]

I've pushed a change to my repository that defines both symbols -- It's a bit of a hack but I think it's okay:

• Defining strdup is commonly accepted among-st C developers when using compilers that don't support it -- this is all we do.
• Defining _get_output_format is more hacky -- bit I think it's OK because from reading MSDN documentation it really just controls the printing of floats to stdout via printf and friends.
• We check if both symbols are defined before defining them ourselves using #ifdef so if/once the bug in Go is fixed -- it will continue working properly.

Can you test the changes I made and see if it works well for you @tapir ?

[tapir]

@slimsag
It works, thanks

[slimsag]

Sorry -- I was wrong.

• _get_output_format is not #define'd by compilers the same way that strdup is.
• _get_output_format missing isn't a Go issue, but a mingw32 one.
• MinGW-W64 does defined _get_output_format.

This causes the package to not compiler with mingw-w64 -- see for instance someone reporting a bug on my mirror of the package: azul3d-legacy/native-glfw#1

I've made a workaround and verified that the package now builds on windows/amd64 with both mingw32 and mingw-w64 compilers successfully, I will submit a PR shortly.

[pwaller]

I was on holiday and getting married, so I have to give a retroactive 👍 to this!

[pwaller]

Oh, and am I missing something or can this issue be closed?

[slimsag]

@pwaller glad you enjoy it =)

am I missing something or can this issue be closed?

Well, it's in the devel_glfw3.1 branch so maybe it's only closed once it's in master? I'm new to the team so.. @shurcooL / @tapir what do we do in these cases?

[pwaller]

Ah yes, I forgot/got confused by the existence of the devel branch.

[dmitshur]

Most of the currently open issues are already fixed in devel branch, and the plan was to have them get closed when the 3.1 PR is merged into master (it has a bunch of "fixes this and that" clauses).

However, given that it's taking 3.1 a very long time to be released (it may very well be another year or more), and there are new legitimate issues with the latest devel branch coming up, it might be easier just to close the fixed issues and use issue tracker for issues in devel branch? Otherwise it's becoming harder to tell what issues are open and which ones are already fixed...

[tapir]

Makes sense. It's getting harder to find your way around in the issue list.
On 3 Sep 2014 11:24, "Dmitri Shuralyov" notifications@github.com wrote:

Most of the currently open issues are already fixed in devel branch, and
the plan was to have them get closed when the 3.1 PR is merged into master.

However, given that it's taking 3.1 a very long time to be released (it
may very well be another year or more), and there are new legitimate issues
with the latest devel branch coming up, it might be easier just to close
the fixed issues and use issue tracker for issues in devel branch?

—
Reply to this email directly or view it on GitHub
#82 (comment).

[pwaller]

👍