PR - Fix build on alpine linux #3394
Comments
|
Comment from codehotter at 2019-04-24 11:14:43 Perhaps this should be done with configure instead of GLIBC define. |
|
Comment from firstyear (@Firstyear) at 2019-04-29 00:57:24 @codeHotter You might find that configure is adding nss3 into the include path, which is why the sources use cert.h not nss3/cert.h. I'm also not sure about the ifdef linux to glibc change, because of the (potential) to build on freebsd which requires a distinction for those platform features. It could be better to determine why on alpine linux is not defined as true? |
|
Comment from codehotter at 2019-05-01 11:26:54 On alpine linux is defined as true, however, alpine uses musl which does not support mallopt. Since freebsd uses glibc but also does not support mallopt, that just further confirms that the glibc define is not the proper solution. Perhaps ./configure could check for mallopt and it should be behind a mallopt ifdef instead. I'll try to see if I can get that to work. I will also test if it then builds on alpine, freebsd and fedora. I'm not sure if I understand your nss3 comment. Do you support the change to include cert.h instead of nss3/cert.h, since configure is already adding nss3 to the include path? On alpine the include path is /usr/include/nss, which is why it breaks if we hardcode nss3 in the source. |
|
Comment from firstyear (@Firstyear) at 2019-05-02 00:48:40 @codeHotter Okay, so how about we make that "if linux && glibc" then around mallopt? @kenoh What do you think about the nss3/cert.h change here? (I'll defer to people to who know more about this than me :) ) |
|
Comment from hmc at 2019-05-02 01:12:42 The NSS path issue also occurs on Debian-based systems, since cert.h exists in /use/include/nss. Note that this needs to be fixed in two places in the codebase. @Firstyear Where is cert.h for you? |
|
Comment from firstyear (@Firstyear) at 2019-05-02 01:26:00 /usr/include/nss3/cert.h is the location on SUSE. If it's an issue on debian that it's in /usr/include/nss then I think changing this to "nss3/cert.h" would break debian, because we need to -I the /usr/include/nss[3] into our configure process, so I think that changing this to <cert.h> and fixing our -I path is the right option here. I'd still like @kenoh's feedback though. |
|
Comment from hmc at 2019-05-02 13:31:15 @Firstyear Changing to <cert.h> (with no preceding path) is the correct fix. The recent I think this change should be a separate pull request. |
|
Comment from mhonek (@kenoh) at 2019-05-22 12:34:00 For the record, the I'm not sure about the
|
|
Comment from codehotter at 2019-06-11 08:41:34 The proper fix for this might be to check for it directly with configure, but that simple fix would certainly solve the problem for me. |
|
Comment from firstyear (@Firstyear) at 2019-06-11 11:16:05 So maybe we should rebase this ontop of 50352 (which is now master) and re-asses if more changes are needed around cert.h? Is that okay? |
|
Comment from firstyear (@Firstyear) at 2019-10-23 04:30:27 50352 has been merged, did you want me to finish this PR up? |
|
Comment from firstyear (@Firstyear) at 2019-12-17 00:54:34 @codeHotter Is this still an issue? |
|
Comment from codehotter at 2019-12-30 11:36:23 I've become much less enamored with alpine in general in the meantime, so I'm happy to close this, although I suspect some configure fix or ifdefs would still be necessary to make this work on alpine. |
|
Comment from firstyear (@Firstyear) at 2020-01-02 01:35:04 Sure. I'll close it and we can revive if there are requests. Currently I think the containerisation efforts are mainly driven by myself which means they'll be suse based, and that seems to be working today. |
|
Comment from firstyear (@Firstyear) at 2020-01-02 01:35:10 Pull-Request has been closed by Firstyear |
|
Patch |
|
Hello, this patch is not working, I forked this repo and create several changes to build at alpine linux with commit kazimsarikaya/389-ds-base@f4e78a5 some of changes are based on the patch defined at #3394 (comment) can you review changes? may be a pr can be created? |
Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/50335
<cert.h>instead of<nss3/cert.h>- pkgconfig will pick up the proper include path#ifdef LINUXinto#ifdef __GLIBC__for malloc options so that we can build on muslThe text was updated successfully, but these errors were encountered: