go-gl / glfw

Go bindings for GLFW 3
BSD 3-Clause "New" or "Revised" License
1.58k stars 181 forks source link

v3.3/glfw: Warnings when building using -tags wayland #359

Open Jacalz opened 2 years ago

Jacalz commented 2 years ago

New warnings:

This is now the warning message we get after https://github.com/go-gl/glfw/pull/399 has been merged to fix Fedora 40 now treating implicit declarations as an error due to their stricter security:

# github.com/go-gl/glfw/v3.3/glfw
In file included from ../../c_glfw_lin.go:10:
../../glfw/src/wl_window.c:29:9: warning: "_GNU_SOURCE" redefined
   29 | #define _GNU_SOURCE
      |         ^~~~~~~~~~~
<command-line>: note: this is the location of the previous definition

This should be fixed so we get no warnings. Removing _GNU_SOURCE being defined will make the build fail on Fedora 40.

Old warnings:

I am getting these errors when building for Wayland. Opening the issue here in case there are issues on our end like last time.

# github.com/go-gl/glfw/v3.3/glfw
In file included from ../../vendor/github.com/go-gl/glfw/v3.3/glfw/c_glfw_lin.go:9:
../../vendor/github.com/go-gl/glfw/v3.3/glfw/glfw/src/wl_window.c: In function ‘createTmpfileCloexec’:
../../vendor/github.com/go-gl/glfw/v3.3/glfw/glfw/src/wl_window.c:115:10: warning: implicit declaration of function ‘mkostemp’; did you mean ‘mkstemp’? [-Wimplicit-function-declaration]
  115 |     fd = mkostemp(tmpname, O_CLOEXEC);
      |          ^~~~~~~~
      |          mkstemp
../../vendor/github.com/go-gl/glfw/v3.3/glfw/glfw/src/wl_window.c: In function ‘waitForData’:
../../vendor/github.com/go-gl/glfw/v3.3/glfw/glfw/src/wl_window.c:266:32: warning: implicit declaration of function ‘ppoll’; did you mean ‘poll’? [-Wimplicit-function-declaration]
  266 |             const int result = ppoll(fds, count, &ts, NULL);
      |                                ^~~~~
      |                                poll
../../vendor/github.com/go-gl/glfw/v3.3/glfw/glfw/src/wl_window.c: In function ‘_glfwPlatformGetClipboardString’:
../../vendor/github.com/go-gl/glfw/v3.3/glfw/glfw/src/wl_window.c:1866:11: warning: implicit declaration of function ‘pipe2’; did you mean ‘pipe’? [-Wimplicit-function-declaration]
 1866 |     ret = pipe2(fds, O_CLOEXEC);
      |           ^~~~~
      |           pipe
tomas-mraz commented 2 years ago

I got the same error after upgrading WSL Ubuntu from 20.04.5 to 22.04.1. libwayland-dev 1.18->1.20 wayland-protocols 1.20->1.25

Before the upgrade I was getting an error that looks like https://gitlab.gnome.org/GNOME/gtk/-/issues/5025 ... so it's gone now, but this new one appeared.

Jacalz commented 11 months ago

I tried to fix this but failed. The corresponding imports seem to all be imported already. I wonder, is this a a bug upstream?

metal3d commented 6 months ago

Hello,

At this time, what I did on Fedora 40 (because it's not errors and not warnings), was to follow the recommendations given by the compiler.

I give you the patch to see where I changed lines:

diff -aur v3.3/glfw/glfw/src/wl_window.c /home/metal3d/Projects/Scoreboard/vendor/github.com/go-gl/glfw/v3.3/glfw/glfw/src/wl_window.c
--- v3.3/glfw/glfw/src/wl_window.c  2024-04-23 22:55:46.566521640 +0200
+++ /home/metal3d/Projects/Scoreboard/vendor/github.com/go-gl/glfw/v3.3/glfw/glfw/src/wl_window.c   2024-04-23 22:52:19.169067823 +0200
@@ -51,7 +51,8 @@
 {
     int fd;

-    fd = mkostemp(tmpname, O_CLOEXEC);
+    //fd = mkostemp(tmpname, O_CLOEXEC);
+    fd = mkstemp(tmpname);
     if (fd >= 0)
         unlink(tmpname);

@@ -199,7 +200,8 @@
             const time_t seconds = (time_t) *timeout;
             const long nanoseconds = (long) ((*timeout - seconds) * 1e9);
             const struct timespec ts = { seconds, nanoseconds };
-            const int result = ppoll(fds, count, &ts, NULL);
+            //const int result = ppoll(fds, count, &ts, NULL);
+            const int result = poll(fds, count, (int) (*timeout * 1e3));
 #elif defined(__NetBSD__)
             const time_t seconds = (time_t) *timeout;
             const long nanoseconds = (long) ((*timeout - seconds) * 1e9);
@@ -1228,7 +1230,8 @@
 {
     int fds[2];

-    if (pipe2(fds, O_CLOEXEC) == -1)
+    //if (pipe2(fds, O_CLOEXEC) == -1)
+    if (pipe(fds) == -1)
     {
         _glfwInputError(GLFW_PLATFORM_ERROR,
                         "Wayland: Failed to create pipe for data offer: %s",
Jacalz commented 6 months ago

Per discussions we had on Slack, that patch changes the behaviour in a braking way. It is not equivalent. There is something else that is wrong

metal3d commented 6 months ago

Yes, actually we see that these functions are enclosed in some macro / templates. It seems that non of the ifdef are true, so the functions are not defined.

Maybe we missed an compiler option to set __USE_GNU or __USE_LARGEFILE64.

All in "feature.h" file.

metal3d commented 6 months ago

OK, so I created a pull-request to fix the problem. The only thing missing was a simple variable to set at compile time. The “features.h” file simply expects “_GNU_SOURCE” to internally activate “__USE_GNU”. As a result, the necessary functions can be created.

Don't ask me how I found it, I was just wandering through the header files and accidentally stumbled across it.

Anyway, thanks @Jacalz for pointing me in a more interesting direction than changing functions to less compatible ones.

Jacalz commented 6 months ago

This is now the warning message we get because Fedora 40 now treats implicit declarations as an error due to their stricter security:

# github.com/go-gl/glfw/v3.3/glfw
In file included from ../../c_glfw_lin.go:10:
../../glfw/src/wl_window.c:29:9: warning: "_GNU_SOURCE" redefined
   29 | #define _GNU_SOURCE
      |         ^~~~~~~~~~~
<command-line>: note: this is the location of the previous definition

I updated the issue description to mention this.