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

Failure to build on musl due usage of qsort_r #83

Closed
maxice8 opened this Issue Oct 17, 2018 · 12 comments

Comments

Projects
None yet
2 participants
@maxice8
Copy link
Contributor

maxice8 commented Oct 17, 2018

on src/basic/util.h

[11/344] Compiling C object 'src/basic/src@basic@@basic@sta/alloc-util.c.o'.
FAILED: src/basic/src@basic@@basic@sta/alloc-util.c.o 
cc -Isrc/basic/src@basic@@basic@sta -Isrc/basic -I../src/basic -Isrc/shared -I../src/shared -Isrc/systemd -I../src/systemd -Isrc/login -I../src/login -Isrc/core -I../src/core -I../src/libelogind/sd-bus -I../src/libelogind/sd-id128 -Isrc/sleep -I../src/sleep -Isrc/update-utmp -I../src/update-utmp -I. -I../ -flto -fdiagnostics-color=always -DNDEBUG -pipe -D_FILE_OFFSET_BITS=64 -std=gnu99 -Wextra -Werror=undef -Wlogical-op -Wmissing-include-dirs -Wold-style-definition -Wpointer-arith -Winit-self -Wfloat-equal -Wsuggest-attribute=noreturn -Werror=missing-prototypes -Werror=implicit-function-declaration -Werror=missing-declarations -Werror=return-type -Werror=incompatible-pointer-types -Werror=format=2 -Wstrict-prototypes -Wredundant-decls -Wmissing-noreturn -Wimplicit-fallthrough=5 -Wshadow -Wendif-labels -Wstrict-aliasing=2 -Wwrite-strings -Werror=overflow -Werror=shift-count-overflow -Werror=shift-overflow=2 -Wdate-time -Wnested-externs -ffast-math -fno-common -fdiagnostics-show-option -fno-strict-aliasing -fvisibility=hidden -fstack-protector -fstack-protector-strong --param=ssp-buffer-size=4 -fPIE -ffunction-sections -fdata-sections -Wno-unused-parameter -Wno-missing-field-initializers -Wno-unused-result -Wno-format-signedness -Wno-error=nonnull -Werror=shadow -include config.h -D_FORTIFY_SOURCE=2 -mtune=generic -O2 -fPIC -pthread -fvisibility=default  -MD -MQ 'src/basic/src@basic@@basic@sta/alloc-util.c.o' -MF 'src/basic/src@basic@@basic@sta/alloc-util.c.o.d' -o 'src/basic/src@basic@@basic@sta/alloc-util.c.o' -c ../src/basic/alloc-util.c
In file included from ../src/basic/alloc-util.c:8:
../src/basic/util.h: In function 'qsort_r_safe':
../src/basic/util.h:121:9: error: implicit declaration of function 'qsort_r'; did you mean 'qsort'? [-Werror=implicit-function-declaration]
         qsort_r(base, nmemb, size, compar, userdata);
         ^~~~~~~
         qsort
../src/basic/util.h:121:9: warning: nested extern declaration of 'qsort_r' [-Wnested-externs]
cc1: some warnings being treated as errors
[12/344] Compiling C object 'src/basic/src@basic@@basic@sta/bus-label.c.o'.
ninja: build stopped: subcommand failed.
=> ERROR: elogind-239.1_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:110

@Yamakuzure Yamakuzure self-assigned this Oct 18, 2018

@Yamakuzure Yamakuzure added the bug label Oct 18, 2018

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Oct 18, 2018

@maxice8 : Sounds like there is something for musl_missing.[hc] ...

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Oct 18, 2018

For reference : https://groups.google.com/forum/#!topic/osv-dev/csfKS5z6MUo :

2014-12-25
qsort_r() patch for musl is available but haven't merged upstream musl:
esmil/musl@194f9cf

@maxice8

This comment has been minimized.

Copy link
Contributor Author

maxice8 commented Oct 18, 2018

Musl upstream has refused to merge qsort_r since there are 2 competing implementations claiming the same name from GNU and from FreeBSD i guess in the meantime the GNU qsort_r can be provided on musl_missing ?

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Oct 18, 2018

@maxice8 : I guess so. Currently it looks like v239 will be the version with the fastest service release ever! 😬

@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Oct 23, 2018

@maxice8 : I have started looking for the glibc implementation of qsort_r. It seems too heavy to put it in musl_missing.[hc]. So I am also checking whether an own source file, maybe in 'shared' works out.

Yamakuzure added a commit that referenced this issue Oct 25, 2018

Prep v240: Add check for qsort_r() and the function if not provided.
As elogind supports musl-libc, we have to remedy the situation that
upstream decided to make use of qsort_r(). This function is not
provided by musl-libc, so we have to provide an own variant.

The variant is an adaption of the qsort_r() algorithm found in
glibc-2.28, the disclaimer from their source files have been added.

Bug: #83
Signed-off-by: Sven Eden <sven.eden@prydeworx.com>
@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Oct 25, 2018

@maxice8 : Could you please checkout or download f2c9a0d and try out whether this works?

Thank you very much!

@maxice8

This comment has been minimized.

Copy link
Contributor Author

maxice8 commented Oct 25, 2018

testing as a patch for Void Linux's elogind pkg

@maxice8

This comment has been minimized.

Copy link
Contributor Author

maxice8 commented Oct 25, 2018

<command-line>: note: this is the location of the previous definition
../src/shared/qsort_r_missing.c: In function 'msort_with_tmp':
../src/shared/qsort_r_missing.c:85:9: error: unknown type name '__compar_d_fn_t'; did you mean 'compare_fn_t'?
         __compar_d_fn_t cmp = p->cmp;
         ^~~~~~~~~~~~~~~
         compare_fn_t
../src/shared/qsort_r_missing.c:85:31: warning: initialization of 'int' from 'compare_fn_t' {aka 'int (* const)(const void *, const void *, void *)'} makes integer from pointer without a cast [-Wint-conversion]
         __compar_d_fn_t cmp = p->cmp;
                               ^
../src/shared/qsort_r_missing.c:90:40: error: invalid type argument of unary '*' (have 'int')
                                 if ( ( *cmp ) ( b1, b2, arg ) <= 0 ) {
                                        ^~~~
../src/shared/qsort_r_missing.c:104:40: error: invalid type argument of unary '*' (have 'int')
                                 if ( ( *cmp ) ( b1, b2, arg ) <= 0 ) {
                                        ^~~~
../src/shared/qsort_r_missing.c:122:40: error: invalid type argument of unary '*' (have 'int')
                                 if ( ( *cmp ) ( b1, b2, arg ) <= 0 ) {
                                        ^~~~
../src/shared/qsort_r_missing.c:137:40: error: invalid type argument of unary '*' (have 'int')
                                 if ( ( *cmp ) ( *( const void** ) b1, *( const void** ) b2, arg ) <= 0 ) {
                                        ^~~~
../src/shared/qsort_r_missing.c:151:40: error: invalid type argument of unary '*' (have 'int')
                                 if ( ( *cmp ) ( b1, b2, arg ) <= 0 ) {
                                        ^~~~
[135/345] Linking target libelogind.so.0.23.0.
@maxice8

This comment has been minimized.

Copy link
Contributor Author

maxice8 commented Oct 25, 2018

both downloading the commit as an archive and using it as a patch fail with this ?

can't one of the qsort_r musl patches that are floating around be used ?

Yamakuzure added a commit that referenced this issue Oct 25, 2018

Prep v240: qsort_r_missing.c : Remove glibc specific bits
Bug: #83
Signed-off-by: Sven Eden <sven.eden@prydeworx.com>
@Yamakuzure

This comment has been minimized.

Copy link
Collaborator

Yamakuzure commented Oct 25, 2018

The implementations I found looked very different.

However, I simply forgot to replace a few glibc specific identifiers. As I have a glibc stdlib.h, it didn't break for me.

This should be fixed in 955e78a - And many many thanks for testing!

(If this doesn't work at once, I have to see to it that I somehow can test against musl myself.)

@maxice8

This comment has been minimized.

Copy link
Contributor Author

maxice8 commented Oct 25, 2018

@maxice8

This comment has been minimized.

Copy link
Contributor Author

maxice8 commented Oct 25, 2018

back, works.

Thanks 👍

@maxice8 maxice8 closed this Oct 25, 2018

Yamakuzure added a commit that referenced this issue Nov 8, 2018

Prep v240: Add check for qsort_r() and the function if not provided.
As elogind supports musl-libc, we have to remedy the situation that
upstream decided to make use of qsort_r(). This function is not
provided by musl-libc, so we have to provide an own variant.

The variant is an adaption of the qsort_r() algorithm found in
glibc-2.28, the disclaimer from their source files have been added.

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

Yamakuzure added a commit that referenced this issue Nov 8, 2018

Prep v240: qsort_r_missing.c : Remove glibc specific bits
Bug: #83
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