Fix build for wayland

[thomasruiz]

Follows up #198

Regarding the files wayland-pointer-constraints-unstable-v1-client-protocol.c, wayland-pointer-constraints-unstable-v1-client-protocol.h, wayland-relative-pointer-unstable-v1-client-protocol.c and wayland-relative-pointer-unstable-v1-client-protocol.h, they were generated using the wayland-scanner binary and the protocols defined on https://github.com/wayland-project/wayland-protocols.

$ wayland-scanner client-header < /usr/share/wayland-protocols/unstable/relative-pointer/relative-pointer-unstable-v1.xml > wayland-relative-pointer-unstable-v1-client-protocol.h
$ wayland-scanner client-header < /usr/share/wayland-protocols/unstable/pointer-constraints/pointer-constraints-unstable-v1.xml > wayland-pointer-constraints-unstable-v1-client-protocol.h
$ wayland-scanner code < /usr/share/wayland-protocols/unstable/relative-pointer/relative-pointer-unstable-v1.xml > wayland-relative-pointer-unstable-v1-client-protocol.c
$ wayland-scanner code < /usr/share/wayland-protocols/unstable/pointer-constraints/pointer-constraints-unstable-v1.xml > wayland-pointer-constraints-unstable-v1-client-protocol.c

I am not entirely sure we can add these files to the repo, but at least it builds correctly with no warning.

[tapir]

Thanks for this! Can you explain this PR a little bit.

• Where do the *.c *.h files come from? I don't see them in glfw source.
• You were talking about some commands? Something to do in cli? Can you document?
• What does _GNU_SOURCE macro have to do with it?

[thomasruiz]

These files come from these lines: https://github.com/glfw/glfw/blob/master/src/CMakeLists.txt#L42-L49.

During the compilation, cmake generates the 4 files using the extra cmake module defined here.

The commands I talked about were the wayland-scanner ones. You don't need to run them if the files are included in the source code.

_GNU_SOURCE fixes a problem with mkostemp (used here). There is a #define in the beginning of the file, but it's still showing the warning nevertheless. I suspect it is because the build use #include to compile the .c files.

[tapir]

I see. I'm not on Wayland so I can't test this but since it didn't compile at all before this PR I'm inclined to merge this anyway.
In theory there is nothing wrong including those cmake generated files as they look like generic to me.

[tapir]

As a side note, it would actually be really cool if we could have a wayland-scanner that generated Go code.
Bu that's beside the point of this PR 👍

[dmitshur]

Hey @elmindreda, if I may bug you about this, do you know whether vendoring copies of these wayland-relative-pointer-unstable-v1-client-protocol.h files in our Go package, generated by wayland-scanner on one system, is an approach that makes sense?

Those files are not expected to be available on the user's Wayland system, are they?

I'm asking because I know you're likely more familiar with this, and I have no idea about Wayland, so your expertise would be helpful. Thanks, and no worries if you don't have bandwidth to deal with this.

[elmindreda]

@shurcooL Sorry, I'm really not an expert on Wayland. The expert (and author) wrt these protocols is @jadahl.

Those C files are not packaged anywhere, as far as I know; only the XML files are.

From what I gather from wayland-protocols it seems relatively safe for you to bundle them, as they're using something similar to semantic versioning and neither the protocols nor the scanner have seen recent updates. Generating the files yourselves with wayland-scanner would be safer, though.

[thomasruiz]

@shurcooL go:generate works like a charm!

We juste need to run (in directory v3.2/glfw/) go generate when the protocols change. I added the xml files directly in the repo.

[jadahl]

We should really use the installed protocol files and generate them on the fly. Is there no way to do that using whatever this new build system is?

[thomasruiz]

@jadahl I think we can use pkg-config to find out where the xml files are located, so yeah, we could remove them from the repository.

The final .c and .h files have to be committed though, as there is no way to run wayland-scanner during the build process (and users might not have the binary anyway).

[dmitshur]

Thanks for info @elmindreda.

Including the generated .c and .h files is good, then. Users are not going to be doing the go generate step, only we the developers are, so it needs to be a part of the repo when users do go get.

It's great that this can be made a little more reproducible via go:generate directives.

I have the following change requests:

• Add a new section to https://github.com/go-gl/glfw/wiki/Development, at the bottom, and document the steps a developer would need to run to regenerate the .c/.h files when XML source files change.

  It can be a short section. As I understand, it's just a matter of installing/having the wayland-scanner binary, then running go generate on the package. But it's a good place for you to write where one can install wayland-scanner binary from, and where latest XML files can be found.

• Decide if you're going to include the XML files or if they'll be fetched from somewhere else.

  If you do keep them, move them to a wayland subdirectory, I think that'd be cleaner.

I have not tested this (because I don't actively use Linux). But at a high level, this looks good. Thanks for your work!

[thomasruiz]

@shurcooL There you go: https://github.com/go-gl/glfw/wiki/Development#generating-wayland-sourceheader-files

To keep the repo clean, it is probably best to regenerate the files from the up-to-date repositories.

Can you try the small bash script added in the wiki and let me know if it work on a different machine than mine?

[dmitshur]

To keep the repo clean, it is probably best to regenerate the files from the up-to-date repositories.

Sounds good.

There you go: https://github.com/go-gl/glfw/wiki/Development#generating-wayland-sourceheader-files

Can you try the small bash script added in the wiki and let me know if it work on a different machine than mine?

Great, thanks!

Does it need to be done on a Linux machine? I tried on macOS and got:

./autogen.sh: line 7: autoreconf: command not found

If it's meant to be done on Linux only, we should mention that in the wiki section.

[dmitshur]

Let's not add a .gitignore file, it's not needed.

Instead, we can add a rm pointer-constraints-unstable-v1.xml relative-pointer-unstable-v1.xml line at the end of https://github.com/go-gl/glfw/wiki/Development#generating-wayland-sourceheader-files and the repo will be clean.

[dmitshur]

This still needs to be addressed.

[dmitshur]

Tried it in a fresh Ubuntu VM with Go 1.8.3 and v3.2/glfw, got the same error:

I'm guessing some package is needed for autoreconf. We should document that to make the instructions reproducible.

[dmitshur]

I did sudo apt-get install autoconf which installed the missing autoreconf, but then:

Installing sudo apt-get install libtool fixed the above problem.

Afterwards, the only missing step was adding the wayland-scanner to PATH:

export PATH="$PATH:/tmp/wayland"

Otherwise it wasn't finding wayland-scanner binary, obviously.

With all that, I was able to rebuild the wayland .c/.h files and they pretty much matched the ones in this PR generated by you, except some comments were different because I generated with a newer version of wayland-scanner.

So, overall, I confirm that with the minor tweaks above, it works.

However, one big question I have is this. It seems there's an APT package that offers a binary version of wayland-scanner. Is there a reason you opted to build it from source rather than just doing sudo apt-get install libwayland-bin?

[dmitshur]

Friendly ping @thomasruiz. Are you still interesting in completing this? If so, did you see my question above? When you answer it, we can proceed.

[thomasruiz]

@shurcooL I was sure I answered that already. Good catches for the missing steps to compile wayland-scanner.

Yes, there is an APT package for it on debian and ubuntu, but I am not sure whether it exists on other distributions, but now I'm thinking about it, it's probably best to tell in the docs to try and install the package provided if it exists.

[dmitshur]

Okay, thanks.

Here is my suggestion. I think we should change the wiki page to list instructions for installing wayland-scanner from packages, and say "or you can install from source if you prefer."

The only reason I went about building wayland-scanner from source is because I thought it was necessary for this PR to function correctly. If that's not the case, I think recommending installing a wayland-scanner via a package is better. Installing it from source is harder (it's not as easy as go get -u), and we're glfw developers, not wayland-scanner developers, so it seems appropriate. Also, ubuntu/debian are pretty common. Also, I don't think we even need to maintain its installation instructions from source - we can let people figure that out on their own (or, just point to this thread for more info.)

[dmitshur]

I want to help communicate what still needs to be done to get this into mergeable state. See inline comment.

[dmitshur]

This still needs to be addressed.

[pwaller]

Hi! What's the status of this? Does #212 supersede it? Can I help with testing? It's not totally clear to me what is outstanding, or how I might test it.

If this is merged now as-is, does it regress anything?

[tapir]

I think #212 supersedes this one. I would also like to have some updates on how to test it as now I'm in wayland and I have proper setup to test it. Shall we continue on #212? @r0l1

[r0l1]

I'll update #212 with some more information. Let's continue there.

[tapir]

I'm closing this one then thanks