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

Update to GLFW 3.1.2. #148

Merged
merged 2 commits into from
Oct 17, 2015
Merged

Update to GLFW 3.1.2. #148

merged 2 commits into from
Oct 17, 2015

Conversation

dmitshur
Copy link
Member

See https://github.com/glfw/glfw/releases/tag/3.1.2. Released by @elmindreda 9 hours ago as of this post.

To quote:

This release adds fixes for a large number of bugs that together affect all supported platforms, as well as dynamic loading work that simplifies compilation and linking.

See the version history for a complete list of changes.

Used the following steps:

rm -rf ./glfw/
git clone --depth 1 --branch latest https://github.com/glfw/glfw.git
cd ./glfw/
git rev-parse HEAD > ../GLFW_C_REVISION.txt

# Only keep the needed sources files in ./src, ./include and ./deps.
rm -rf ./.git
rm -rf ./.gitignore
rm -rf ./CMake
rm -rf ./CMakeLists.txt
rm -rf ./src/CMakeLists.txt
rm -rf ./src/*.in
rm -rf ./README.md
rm -rf ./cmake_uninstall.cmake.in
rm -rf ./docs
rm -rf ./examples
rm -rf ./tests

/cc @slimsag

Tested briefly with Conception-go and Hover on OS X, I did not spot any issues.

See https://github.com/glfw/glfw/releases/tag/3.1.2.

Used the following steps:

	rm -rf ./glfw/
	git clone --depth 1 --branch latest https://github.com/glfw/glfw.git
	cd ./glfw/
	git rev-parse HEAD > ../GLFW_C_REVISION.txt

	# Only keep the needed sources files in ./src, ./include and ./deps.
	rm -rf ./.git
	rm -rf ./.gitignore
	rm -rf ./CMake
	rm -rf ./CMakeLists.txt
	rm -rf ./src/CMakeLists.txt
	rm -rf ./src/*.in
	rm -rf ./README.md
	rm -rf ./cmake_uninstall.cmake.in
	rm -rf ./docs
	rm -rf ./examples
	rm -rf ./tests
@pwaller
Copy link
Member

pwaller commented Oct 13, 2015

Hi! I just tried to run this, but I don't see evidence of any files changed in the go package (under /v3.1/glfw) as I expected. Go install does not rebuild the package either. Am I missing something?

@dmitshur
Copy link
Member Author

That's right. No .go files were changed, only the C library was updated. As far as I can tell, it's a patch version update and there are no API changes, so nothing needs updating in .go files.

I also ran into the problem that the go tool couldn't tell that the Go package became stale and needed to be recompiled. I worked around it by doing touch glfw.go and then go install, but you could also do it by removing your $GOPATH/pkg/*/github.com/go-gl/glfw/v3.1/glfw.a file and then doing go install.

@pwaller
Copy link
Member

pwaller commented Oct 13, 2015

Okay, did that, now:

~/.local/src/github.com/go-gl/glfw$ touch v3.1/glfw/glfw.go 
~/.local/src/github.com/go-gl/glfw$ go install -v ./v3.1/glfw/
github.com/go-gl/glfw/v3.1/glfw

# github.com/go-gl/glfw/v3.1/glfw
/usr/bin/ld: $WORK/github.com/go-gl/glfw/v3.1/glfw/_obj/glfw_glx_context.o: undefined reference to symbol 'dlclose@@GLIBC_2.2.5'
/lib/x86_64-linux-gnu/libdl.so.2: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
~/.local/src/github.com/go-gl/glfw$ go version
go version go1.5 linux/amd64

@pwaller
Copy link
Member

pwaller commented Oct 13, 2015

@elmindreda
Copy link

You need -ldl now, but on the upside _GLFW_HAS_GLXGETPROCADDRESSARB and _GLFW_HAS_DLOPEN no longer do anything and can be removed.

@pwaller
Copy link
Member

pwaller commented Oct 13, 2015

Thanks @elmindreda! Can confirm this obvious diff fixes it:

diff --git a/v3.1/glfw/build.go b/v3.1/glfw/build.go
index e3f201c..ce754d1 100644
--- a/v3.1/glfw/build.go
+++ b/v3.1/glfw/build.go
@@ -31,12 +31,12 @@ package glfw
 // Linux Build Tags
 // ----------------
 // GLFW Options:
-#cgo linux,!wayland CFLAGS: -D_GLFW_X11 -D_GLFW_GLX -D_GLFW_HAS_GLXGETPROCADDRESSARB -D_GLFW_HAS_DLOPEN
-#cgo linux,wayland CFLAGS: -D_GLFW_WAYLAND -D_GLFW_EGL -D_GLFW_HAS_DLOPEN
+#cgo linux,!wayland CFLAGS: -D_GLFW_X11 -D_GLFW_GLX
+#cgo linux,wayland CFLAGS: -D_GLFW_WAYLAND -D_GLFW_EGL

 // Linker Options:
-#cgo linux,!wayland LDFLAGS: -lGL -lX11 -lXrandr -lXxf86vm -lXi -lXcursor -lm -lXinerama
-#cgo linux,wayland LDFLAGS: -lGL -lX11 -lXrandr -lXxf86vm -lXi -lXcursor -lm -lXinerama
+#cgo linux,!wayland LDFLAGS: -lGL -lX11 -lXrandr -lXxf86vm -lXi -lXcursor -lm -lXinerama -ldl
+#cgo linux,wayland LDFLAGS: -lGL -lX11 -lXrandr -lXxf86vm -lXi -lXcursor -lm -lXinerama -ldl


 // FreeBSD Build Tags

@dmitshur
Copy link
Member Author

Thanks, I've confirmed it in a Linux VM as well, and updated the PR.

@tapir
Copy link
Member

tapir commented Oct 14, 2015

Build and go-gl/examples are OK on Windows 7 64-bit with MSYS 2 and mingw. 👍

@tapir
Copy link
Member

tapir commented Oct 14, 2015

Maybe I should open a new issue for this but doesn't Go support git submodules now?
Although it's trivial to update GLFW and has the advantage of being able to remove unnecessary files, I think GLFW as a submodule makes more sense.

@dmitshur
Copy link
Member Author

Yes, feel free to open an issue and we can think about that separately.

I'd have to look into it, because as far as I know right now, git submodules are not as friendly/reliable as a single git repo.

For the update steps, I didn't have to re-invent them, they are taken from previously established update steps at https://github.com/go-gl/glfw/wiki/Development. Updating takes a few minutes at most and isn't painful at all (not counting potential work to update Go package APIs if needed, etc.). So I'm open to improvements but I don't think it's a painful area. GLFW updates come out rarely and only developers need to do this step.

@dmitshur
Copy link
Member Author

Also, with a submodule, you couldn't remove unneeded files, could you?

@pwaller
Copy link
Member

pwaller commented Oct 14, 2015

We use submodules and go successfully within my organization, and I think the proposal to use it here seems reasonable. There are a few gotchas, particularly if the submodules are moved around or deleted, and particularly if using an old version of git. I don't have much experience of how well go get works though.

With respect to removing unneeded files, there are a few possibilities: 1) symlink the needed files somewhere, 2) maintain a fork which deletes unneeded files (should be relatively cheap so long as one has the right mental model, since merges should be straightforward). 3) what is the problem with having a few extra files? Why not just keep them?

The main question in my mind is how big the whole glfw repository is. If it's <10MiB then no problem. If it's >100MiB then it might represent a bit more of a burden to obtain it and we might want to think before acting.

@dmitshur
Copy link
Member Author

Let's please move that discussion to a separate issue.

I've addressed the issue that @pwaller found and confirmed it fixes Linux build in a VM. Can I merge this PR or is there further feedback?

@tapir
Copy link
Member

tapir commented Oct 17, 2015

LGTM
On 17 Oct 2015 07:04, "Dmitri Shuralyov" notifications@github.com wrote:

Let's please move that discussion to a separate issue.

I've addressed the issue that @pwaller https://github.com/pwaller found
and confirmed it fixes Linux build in a VM. Can I merge this PR or is there
other feedback?


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

@dmitshur dmitshur merged commit be87be3 into master Oct 17, 2015
@dmitshur dmitshur deleted the update-GLFW-3.1.2 branch October 17, 2015 08:41
@dmitshur
Copy link
Member Author

dmitshur commented Nov 2, 2015

Hey guys, I've noticed Travis builds are now failing for this package. I think it's likely a regression after this PR. But it's also possible something changed about Travis Linux environment.

See https://travis-ci.org/go-gl/examples/builds/83050258.

The build errors are:

# github.com/go-gl/glfw/v3.1/glfw
/tmp/go-build452009267/github.com/go-gl/glfw/v3.1/glfw/_obj/glfw_posix_time.o: In function `getRawTime':
../glfw/v3.1/glfw/glfw/src/posix_time.c:42: undefined reference to `clock_gettime'
/tmp/go-build452009267/github.com/go-gl/glfw/v3.1/glfw/_obj/glfw_posix_time.o: In function `_glfwInitTimer':
../glfw/v3.1/glfw/glfw/src/posix_time.c:67: undefined reference to `clock_gettime'
collect2: ld returned 1 exit status

I've googled that error and the suggested fix seems to be to link against the librt.so "Real Time" shared library via -lrt linker flag.

Right now we don't have -lrt flag in our build options. Is it needed? Or is something else the problem.

I'll add a Travis build to this repo so this is easier to debug and catch in the future.

@dmitshur
Copy link
Member Author

dmitshur commented Nov 2, 2015

I'll add a Travis build to this repo so this is easier to debug and catch in the future.

Done so in #149.

The idea of adding -lrt to Linux build options seems to have allowed glfw to build successfully in Travis environment.

I'm not sure why none of us caught it when trying on Linux (but not in Travis) earlier. But it seems like a reasonable change, and it fixes Travis build (which means it will fix Travis builds for all projects that import this Go package). Please review that PR, and if it looks good, let's merge it.

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.

4 participants