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

_POSIX_C_SOURCE not defined on a musl system #113

Closed
Derriick opened this Issue Feb 20, 2019 · 12 comments

Comments

Projects
None yet
4 participants
@Derriick
Copy link
Contributor

Derriick commented Feb 20, 2019

#if defined _GNU_SOURCE || _POSIX_C_SOURCE >= 199309L

When I try to build sway-1.0-rc3 on the musl version of Void Linux, meson stops saying that "_POSIX_C_SOURCE" is not defined.

[255/272] Compiling C object 'swaybar/cf785bf@@swaybar@exe/main.c.o'.
[256/272] Compiling C object 'swaybar/cf785bf@@swaybar@exe/render.c.o'.
[257/272] Compiling C object 'swaybar/cf785bf@@swaybar@exe/status_line.c.o'.
[258/272] Compiling C object 'swaybar/cf785bf@@swaybar@exe/tray_host.c.o'.
[259/272] Compiling C object 'swaybar/cf785bf@@swaybar@exe/tray_tray.c.o'.
FAILED: swaybar/cf785bf@@swaybar@exe/tray_tray.c.o
cc -Iswaybar/cf785bf@@swaybar@exe -Iswaybar -I../swaybar -Iinclude -I../include -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libdrm -I/usr/include/libpng16 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/json-c -I/usr/include/pango-1.0 -I/usr/include/fribidi -I/usr/include/harfbuzz -I/usr/include/elogind -flto -fdiagnostics-color=always -DNDEBUG -pipe -D_FILE_OFFSET_BITS=64 -Werror -std=c11 '-DSWAY_SRC_DIR="/builddir/sway-1.0-rc3"' -DWL_HIDE_DEPRECATED -DWLR_USE_UNSTABLE -Wno-unused-parameter -Wno-unused-result -Wundef -Wvla '-DSYSCONFDIR="//etc"' '-DSWAY_VERSION="1.0"' -fstack-clash-protection -D_FORTIFY_SOURCE=2 -mtune=generic -O2 -pthread  -MD -MQ 'swaybar/cf785bf@@swaybar@exe/tray_tray.c.o' -MF 'swaybar/cf785bf@@swaybar@exe/tray_tray.c.o.d' -o 'swaybar/cf785bf@@swaybar@exe/tray_tray.c.o' -c ../swaybar/tray/tray.c
In file included from /usr/include/elogind/systemd/sd-bus.h:26,
                 from ../include/swaybar/tray/tray.h:6,
                 from ../include/swaybar/tray/item.h:7,
                 from ../swaybar/tray/tray.c:9:
/usr/include/elogind/systemd/sd-event.h:74:28: error: "_POSIX_C_SOURCE" is not defined, evaluates to 0 [-Werror=undef]
 #if defined _GNU_SOURCE || _POSIX_C_SOURCE >= 199309L
                            ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.
=> ERROR: sway-1.0+rc3_1: do_build: '${make_cmd} -C ${meson_builddir} ${makejobs} ${make_build_args} ${make_build_target}' exited with 1
=> ERROR:   in do_build() at common/build-style/meson.sh:126

More details are available in this issue: void-linux/void-packages#8962

@Derriick

This comment has been minimized.

Copy link
Contributor Author

Derriick commented Feb 20, 2019

I have created a pull request with the fix: #114

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Feb 21, 2019

_POSIX_C_SOURCE not defined on a musl system

If that was the case, how was it possible then to build elogind?

git blame tells me, that the line you have been tripping over, is there since 15 Mar 2017.

I can't find a definition of _POSIX_C_SOURCE in the build system or src/shared/musl_missing.h. Actually I can't find it anywhere in the whole tree despite the line in question.

So I have either missed something elogind-specific that enables it to be built on musl-systems, or there is something wrong with either the sway build system or your setup.

The change suggested by your PR is seemingly harmless. It would not, in any case, break stuff for others. But this is a delivered header ending up in /usr/include/systemd, so I am very reluctant to put elogind masks into it.

The masks are needed, or your changes would be possibly reverted by any upstream commit I migrate. Therefore the patch should look like this:

diff --git a/src/systemd/sd-event.h b/src/systemd/sd-event.h
index d82a1ba5d..e26c89ff1 100644
--- a/src/systemd/sd-event.h
+++ b/src/systemd/sd-event.h
@@ -71,7 +71,11 @@ typedef int (*sd_event_handler_t)(sd_event_source *s, void *userdata);
 typedef int (*sd_event_io_handler_t)(sd_event_source *s, int fd, uint32_t revents, void *userdata);
 typedef int (*sd_event_time_handler_t)(sd_event_source *s, uint64_t usec, void *userdata);
 typedef int (*sd_event_signal_handler_t)(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata);
+#if 0 /** in rare cases _POSIX_C_SOURCE is not defined on musl systems, which elogind supports */
 #if defined _GNU_SOURCE || _POSIX_C_SOURCE >= 199309L
+#else
+#if defined _GNU_SOURCE || (defined _POSIX_C_SOURCE && _POSIX_C_SOURCE >= 199309L)
+#endif /* 0 */
 typedef int (*sd_event_child_handler_t)(sd_event_source *s, const siginfo_t *si, void *userdata);
 #else
 typedef void* sd_event_child_handler_t;

Masks like this are protected by my migration tools.

However, I hope you can see why such masks should be rare in API headers, right?

Down the line I'd like to make sure that this is actually a problem caused by elogind, and not an issue caused elsewhere. We can't fix other people bugs, right?

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Feb 21, 2019

I think this is a problem with sway:

seden@LAPTOP-SED ~/sway
$ grep -s -R '#define _POSIX_C_SOURCE' . | wc -l
89

The file in question, /swaybar/tray/tray.c, does not define it.

There had been a few issues about that define. See:

Sorry, but this is not our problem here. The sway crew could move such stuff into config.h. Which macros to define where can be configured with meson.

Short term fix would be to add a #define for _POSIX_C_SOURCE to /swaybar/tray/tray.c like they do for 89 other source files already.

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Feb 21, 2019

@emersion

This comment has been minimized.

Copy link
Contributor

emersion commented Feb 21, 2019

I believe this is a elogind issue. We don't use POSIX functions in this file, so I don't see why we should define it.

Headers in /usr/include always do this kind of things:

#if defined _POSIX_C_SOURCE && (_POSIX_C_SOURCE - 0) >= 199309L

or

#ifdef _POSIX_C_SOURCE
#if    _POSIX_C_SOURCE >= 199309L

elogind should do the same.

@emersion

This comment has been minimized.

Copy link
Contributor

emersion commented Feb 21, 2019

I've been looking into sending this as a systemd patch, but this #if is already a elogind-local patch on top on systemd.

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Feb 21, 2019

@emersion : Not quite.

I believe this is a elogind issue.
(...)
elogind should do the same.

We do not change systemd source code as long as we do not have to.

I've been looking into sending this as a systemd patch, but this #if is already a elogind-local patch on top on systemd.

Nope. If it were, you'd see elogind specific preprocessor masks like above around it. This is a systemd line.

This is the offending line in systemd master:
https://github.com/systemd/systemd/blame/6f0475879a7fa59f2de6912caca5c014fe59c748/src/systemd/sd-event.h#L74

So the very latest version of that file does it like that since May 2017.

However, this line means, that neither _GNU_SOURCE nor _POSIX_C_SOURCE are defined.

Very well. As the sway crew declines, and the systemd crew will definitely answer that they do not support non-gnu-non-posix systems, it's up to us again.

@Derriick : Please adapt your commit like I noted above and I'll merge it.

@emersion

This comment has been minimized.

Copy link
Contributor

emersion commented Feb 21, 2019

Wait, I somehow skipped that line.

This isn't about non-GNU. It's just about building sd-event.h without _GNU_SOURCE nor _POSIX_C_SOURCE defined.

I'll send a systemd patch.

@emersion

This comment has been minimized.

Copy link
Contributor

emersion commented Feb 21, 2019

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Feb 21, 2019

This isn't about non-GNU. It's just about building sd-event.h without _GNU_SOURCE nor _POSIX_C_SOURCE defined.

Oh, man... You are right of course!
This shall be a lesson about wanting to get something done ASAP while in a hurry.

I'll send a systemd patch.

That's great! And Lennart Poettering has already checked in on the PR. That's fast!

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Feb 22, 2019

@emersion : Your PR on systemd is merged to master! 👍

@Derriick : As you brought the issue up, I want to merge your PR nevertheless, so your name is documented in the history. I can cherry-pick the systemd change later.

Yamakuzure added a commit that referenced this issue Feb 22, 2019

Check if _POSIX_C_SOURCE is defined
If a project uses sd-event.h, either directly or through sd-bus.h,
and _POSIX_C_SOURCE is not defined while -Wundef is set, the build
will break.

Bug: #113
Closes: #113
Closes: #114
Signed-off-by: Sven Eden <sven.eden@prydeworx.com>

Yamakuzure added a commit that referenced this issue Feb 22, 2019

Check if _POSIX_C_SOURCE is defined
If a project uses sd-event.h, either directly or through sd-bus.h,
and _POSIX_C_SOURCE is not defined while -Wundef is set, the build
will break.

Bug: #113
Closes: #113
Closes: #114
Signed-off-by: Sven Eden <sven.eden@prydeworx.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.