| Summary: | net-misc/asterisk fails to compile: clang/LLVM | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Agostino Sarubbo <ago> |
| Component: | Current packages | Assignee: | Jaco Kroon <jaco> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | grobian, jaco, proxy-maint, slyfox |
| Priority: | Normal | Keywords: | PullRequest |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| See Also: | https://github.com/gentoo/gentoo/pull/18994 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 408963 | ||
| Attachments: |
build.log
other.tar.bz2 |
||
|
Description
Agostino Sarubbo
2020-07-07 15:16:39 UTC
Created attachment 648236 [details]
build.log
build log and emerge --info
Created attachment 648238 [details]
other.tar.bz2
other logs
checking for RAII support... checking for clang -fblocks... configure: error: BlocksRuntime is required for clang, please install libblocksruntime Happy to make this work if possible, but I'm not sure what's required here. https://github.com/mackyle/blocksruntime Seems to explain the issue. Not sure if either of these USE flags on clang might take care of the issue, and then, how can I enforce that: - - default-compiler-rt : Use compiler-rt instead of libgcc as the default rtlib for clang - - default-libcxx : Use libc++ instead of libstdc++ as the default stdlib for clang Hi Agostino, Could you please look at and respond on the below? (In reply to Jaco Kroon from comment #3) > checking for RAII support... checking for clang -fblocks... configure: > error: BlocksRuntime is required for clang, please install libblocksruntime > > Happy to make this work if possible, but I'm not sure what's required here. > > https://github.com/mackyle/blocksruntime > > Seems to explain the issue. Not sure if either of these USE flags on clang > might take care of the issue, and then, how can I enforce that: > > - - default-compiler-rt : Use compiler-rt instead of libgcc as > the default rtlib for > clang > - - default-libcxx : Use libc++ instead of libstdc++ as the > default stdlib for > clang Hello Jaco, I'm the reporter of something that runs automatically on a machine. If you are unsure about the solution I'd suggest to ask on the gentoo-dev mailing list. blocks is a "vector feature" from Apple, Clang supports this because Apple systems use Clang as system compiler (and the system headers use blocks features) In this case, I cannot imagine Asterisk to /need/ blocks support, it probably just wants to exploit it whenever it is available. That said, the configure logic probably dies at this point because it assumes if the compiler accepts -fblocks, the final linking also succeeds. Perhaps you can 1) either disable the entire blocks check (blocksruntime isn't in Gentoo's shipping, and I don't think there's much interest for it -- fair) 2) extend the check to disable blocks support when linking failed after compiler support was detected. (In reply to Fabian Groffen from comment #6) > blocks is a "vector feature" from Apple, Clang supports this because Apple > systems use Clang as system compiler (and the system headers use blocks > features) > > In this case, I cannot imagine Asterisk to /need/ blocks support, it > probably just wants to exploit it whenever it is available. That said, the > configure logic probably dies at this point because it assumes if the > compiler accepts -fblocks, the final linking also succeeds. Perhaps you can > 1) either disable the entire blocks check (blocksruntime isn't in Gentoo's > shipping, and I don't think there's much interest for it -- fair) > 2) extend the check to disable blocks support when linking failed after > compiler support was detected. I tend to agree with you on this, from the RAII m4 file (./autoconf/ast_check_raii.m4): Lines 10 to 15, check for clang vs everything else. If NOT CLANG, lines 19 to 30, no possible failure, but it tries to use -fnested-functions if possible. Lines 35 to 49 ... does a test compile with -lfblocks, failing that -fblocks -lBlocksRuntime, and failing that fails, if I'm understanding you correctly, I reckon just nuke the AC_MSG_ERROR on line 45 and explicitly set set AST_CLANG_BLOCKS_LIBS and AST_CLANG_BLOCKS ="", and we test that? If that works, push it upstream. I think turning the AC_MSG_ERROR into AC_MSG_WARN and indeed zapping the vars should be acceptable for upstream, since it wouldn't not disable the feature on macOS, yet allow the build to continue on other systems using Clang. I hope this helps/works. (In reply to Fabian Groffen from comment #8) > I think turning the AC_MSG_ERROR into AC_MSG_WARN and indeed zapping the > vars should be acceptable for upstream, since it wouldn't not disable the > feature on macOS, yet allow the build to continue on other systems using > Clang. > > I hope this helps/works. This indeed does. Assuming that the sys-libs/blocksruntime build from Sergei makes it into the tree, is there a way to set a dependency based on the toolchain in use? Something like: {B,R}DEPEND="if_using_clang? ( sys-libs/blocksruntime )" Based on a quick grep, it looks like clang? ( sys-libs/blocksruntime ) may do the trick? But I'll probably double that into: clang? ( fblocks? ( sys-libs/blocksruntime )) depending on whether or not I manage to modify configure. I already have clang installed ... will figure out how to get asterisk compiled with clang whilst still using gcc for everything else. I disagree a bit with what Sergei did. What should be done IMO is the same as we do for OpenMP support. 1. global USE-flag blocks 2. clang[blocks] should pull in the blocksruntime 3. asterix[blocks] should depend on clang[blocks] 4. the blocks check should be made a enable/disable feature, but if that's too much, or gets resistance from upstream, USE=!blocks should ensure the configure script doesn't even try to enable -fblocks (e.g. no unrecorded dependency on blocksruntime would be possible). (little caveat, since GCC doesn't support blocks syntax [yet], the flag should probably just pull in Clang, unlike with USE=openmp) Note that I think we should not start requiring Clang for packages just because they /can/ use blocks support. It's kind of a huge hassle to compile an llvm toolchain first just to compile a package that would also work fine with GCC. If there's additional functionality exposed due to blocks usage, then it must be a USE-flag choice, which would be USE=blocks as I mentioned before. (In reply to Fabian Groffen from comment #10) > I disagree a bit with what Sergei did. > > What should be done IMO is the same as we do for OpenMP support. > > 1. global USE-flag blocks > 2. clang[blocks] should pull in the blocksruntime > 3. asterix[blocks] should depend on clang[blocks] asterisk works just fine with gcc. This means there really is three choices at this stage, and I suspect other packages have similar issues: 1. gcc 2. clang[!blocks] 3. clang[blocks] Frankly, I'm inclined to just let ./configure figure out, but the dep on blocksruntime is an interesting one. > 4. the blocks check should be made a enable/disable feature, but if that's > too much, or gets resistance from upstream, USE=!blocks should ensure the > configure script doesn't even try to enable -fblocks (e.g. no unrecorded > dependency on blocksruntime would be possible). And I'm on the "if it's available, use it" page, but how does one deal with the automagic dependency? And your suggestion of putting that as a USE flag on clang actually makes sense, then asterisk could have a depend on clang that simply states: IUSE+="clang blocks" clang? ( sys-devel/clang[?blocks] ) That, however, feels like a BDEPEND, and the RDEPEND for clang && blocks is *probably* only on the blocksruntime library (but I'm not sure how the libc et all works here as I suspect it still refers back to glibc). > (little caveat, since GCC doesn't support blocks syntax [yet], the flag > should probably just pull in Clang, unlike with USE=openmp) (In reply to Fabian Groffen from comment #11) > Note that I think we should not start requiring Clang for packages just > because they /can/ use blocks support. It's kind of a huge hassle to > compile an llvm toolchain first just to compile a package that would also > work fine with GCC. The double negation had me there for a bit. I fully agree. > If there's additional functionality exposed due to blocks usage, then it > must be a USE-flag choice, which would be USE=blocks as I mentioned before. There is not. It's more of an internal optimization from what I can tell. Basically if it's using CLANG, it WANTS to pass -fblocks [+-lBlocksRuntime] to everything, which simply implies it's using it as an optional extra. What bugs me, let's say I hack the ./configure simply to change AC_MSG_ERROR to AC_MSG_WARN (which is fair), that'll make it pass, however, now if the blocksruntime is available this will introduce an automagic dependency at BUILD time on blocksruntime, and this is for me more the problematic issue currently. So I could make this into ./configure --{enable,disable,with,without}-fblocks option that just happens to depend on clang && blocks USE flags. But really, is clang USE flag the right thing? Surely, CC= determines the compiler to use, not the USE=clang? Consider this sequence: 1. USE=blocks emerge clang 2. USE=-blocks emerge clang 3. emerge asterisk Due to the first emerge pulling in BlocksRuntime ... the RUNTIME will be available for asterisk to use. My implied assumption of course is that -fblocks will still work, but linking will fail due to the lack of runtime in the case where the first emerge was never run. I'm getting really rude now ... triple post, sorry. Figured I best test my assumption:
jkroon@plastiekpoot ~/src/test $ cat blocks.c
int main()
{
^{return 42;}();
}
jkroon@plastiekpoot ~/src/test $ clang -fblocks -c blocks.c
jkroon@plastiekpoot ~/src/test $ clang -fblocks -o blocks blocks.c
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: /tmp/blocks-5aa57a.o:(.rodata+0x20): undefined reference to `_NSConcreteGlobalBlock'
clang-10: error: linker command failed with exit code 1 (use -v to see invocation)
[1] jkroon@plastiekpoot ~/src/test $ clang -o blocks blocks.c
blocks.c:3:2: error: blocks support disabled - compile with -fblocks or pick a deployment target that supports them
^{return 42;}();
^
1 error generated.
[1] jkroon@plastiekpoot ~/src/test $
So my assumption does seem to be correct. The last compile does hint that a simple configure change may not be sufficient. So looking at the code in include/asterisk/utils.h:
1097 #if defined(__clang__)
1098 typedef void (^_raii_cleanup_block_t)(void);
1099 static inline void _raii_cleanup_block(_raii_cleanup_block_t *b) { (*b)(); }
1100
1101 #define RAII_VAR(vartype, varname, initval, dtor) \
1102 __block vartype varname = initval; \
1103 _raii_cleanup_block_t _raii_cleanup_ ## varname __attribute__((cleanup(_raii_cleanup_block) ,unused)) = \
1104 ^{ {(void)dtor(varname);} };
1105
1106 #elif defined(__GNUC__)
1107
1108 #define RAII_VAR(vartype, varname, initval, dtor) \
1109 auto void _dtor_ ## varname (vartype * v); \
1110 void _dtor_ ## varname (vartype * v) { dtor(*v); } \
1111 vartype varname __attribute__((cleanup(_dtor_ ## varname))) = (initval)
1112
1113 #else
1114 #error "Cannot compile Asterisk: unknown and unsupported compiler."
1115 #endif /* #if __GNUC__ */
Which is the ONLY place I can locate '^{' in the *asterisk* code (there are a few in the third-party/pjproject in .m files ... so since we build pjproject independently I don't think we care here, we might on the pjproject side), we may just need to change that into a kind of "if flocks is available use the current __clang__ variant, else, use the current __GNUC__ variant (I'm not seeing anything gcc specific in that variant, but I only ever really use gcc so I can't say with 100% certainty).
So whilst I believe I understand the problem, and (I believe) I can fix (and submit fix upstream), I would not know how to best deal with ebuild side. For asterisk I believe the following changes required:
Add an explicit --{enable,disable}-fblocks, logic follows:
if explicit disabled; then
: # do nothing
else
test -fblocks options availability, including RunTime requirements.
if explicitly enable && not available; then
fail
fi
if available; then
set options to use
fi
fi
And obviously update the utils.h to only use that syntax if available, probably by way of a HAVE_FBLOCKS option set by the above.
This would then make the ebuild easier in that we can ALWAYS pass --{enable,disable}-fblocks based on USE flag, however, we only really want to allow setting USE=blocks IFF CC=clang (we can pull in the runtime dep based on USE flag.
So the masking of USE=blocks if CC!=clang is the part that I'll need help with.
automagic dependencies are a no-no per Gentoo policies: https://devmanual.gentoo.org/general-concepts/use-flags/index.html In the case of Asterisk, I think you really don't have to solve this, and just unconditionally disable blocks support. The gain is probably minimal, and you can always wait for a user to really want its support. Then is a good time to see if the blocks thing was resolved on a higher level (= clang) already or not. The question is whether Clang supports nested functions here the way GCC does. Can you: sed -i -e 's:choke:/*let clang pass as gcc*/:' autoconf/ast_check_raii.m4 eautoreconf and see if with CC=clang asterisk actually compiles (assuming configure goes to check as if it were using the GCC case now). If not, then likely if tc-is-clang, you need blocks for raii. Final resolution: I've pulled in blocksruntime as a dependency for asterisk if CC=clang (by way of USE flag and some pkg_pretend checks). I will however have to use/stable mask the USE blocks USE flag unless blocksruntime can get stabled for x86 and amd64 soon. Ideally keyworded for arm, arm64, ppc and ppc64 so that I can drop the package.use.mask values. Added Sergei since he maintains blocksruntime. Sergei, would you be able to provide us with your plans and timelines regarding blocksruntime w.r.t. the above? Long Explanation: The problem with asterisk is that it needs scoped destructors in "C", as such, it uses __attribute__((cleanup)), but it uses it with wrapper functions to avoid some extra pointer indirection in the destructor (by way of nested functions in gcc, or -fblocks in the case of clang). I've started inquiring about whether a patch to fix this and rather have the extra indirection (which works on both gcc and clang cleanly without nested functions / -fblocks and is thus a generic solution) would be something that can be considered, but so far it hasn't garnered much interest. I'll escalate this to the asterisk-dev ML probably. For us this means we have two options for immediate clang support: 1. Cook the patch (which affects a HUGE amount of code). I've counted this at over 125 destructor functions, and over 2000 users of these destructors (which I don't think there should be change involved with at since this is all wrapped up in a very nasty RAII_VAR macro). As such, a patch like this would be a big one, and be heavy effort to maintain due to large probability of code changes around the changes, or additional users that may need patching too. Minimum such a patch would affect probably about 500-1000 lines of code (not counting context). (read: it must go upstream or not at all.) 2. Pull in blocksruntime for clang support. The associated PR does this in the cleanest way that I could come up with currently. This however means that if we want clang on the asterisk keyworded arches for arm, arm64, ppc and ppc64 we need to see if we can get blocksruntime keyworded for those arches too, and stabled for x86 and amd64. Option 1 is long term in my opinion a cleaner/better solution, actually dropping potential extra code (slightly smaller binaries, possibly fewer indirect trampoline style calls) and enlarging compiler support for asterisk from not only gcc and clang, but to other compilers too that does support __attribute__((cleanup(...))). For now there is no real choice but to go with opion 2. (In reply to Jaco Kroon from comment #16) > Final resolution: I've pulled in blocksruntime as a dependency for asterisk > if CC=clang (by way of USE flag and some pkg_pretend checks). > > I will however have to use/stable mask the USE blocks USE flag unless > blocksruntime can get stabled for x86 and amd64 soon. Ideally keyworded for > arm, arm64, ppc and ppc64 so that I can drop the package.use.mask values. > > Added Sergei since he maintains blocksruntime. Sergei, would you be able to > provide us with your plans and timelines regarding blocksruntime w.r.t. the > above? I don't plan to call stabilization myself as I have no stable tools that rely on sys-libs/blocksruntime. But I don't mind it getting more unstable or stable keywords as a dependency. I suggest you to file KEYWORDREQ/STABLEREQ bugs either against sys-libs/blocksruntime explicitly or as part of net-misc/asterisk. Word of caution: your bigger blockers might be llvm packages. For example we don't have `ppc` keywords for llvm. The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=6202f7bf618faf8a3fb24e6c7b586dd7aad5d144 commit 6202f7bf618faf8a3fb24e6c7b586dd7aad5d144 Author: Jaco Kroon <jaco@uls.co.za> AuthorDate: 2021-01-08 18:20:54 +0000 Commit: Sam James <sam@gentoo.org> CommitDate: 2021-01-22 03:50:57 +0000 net-misc/asterisk: 16.15.1-r2 version bump. * clang/LLVM support * subslots * autoconf 2.70 * codec2 support * drop /var/spool/asterisk from tmpfiles * Move check_extra_config (kernel checks) to pkg_pretend. * (Hopefully) fix use of $ED vs $D. * Other minor non-functional tweaks. Closes: https://bugs.gentoo.org/show_bug.cgi?id=731280 Closes: https://bugs.gentoo.org/show_bug.cgi?id=750581 Closes: https://bugs.gentoo.org/show_bug.cgi?id=763918 Signed-off-by: Jaco Kroon <jaco@uls.co.za> Closes: https://github.com/gentoo/gentoo/pull/18994 Signed-off-by: Sam James <sam@gentoo.org> net-misc/asterisk/asterisk-16.15.1-r2.ebuild | 322 +++++++++++++++++++++ .../files/asterisk-16.15.1-r2-autoconf-2.70.patch | 14 + 2 files changed, 336 insertions(+) Additionally, it has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=880e7d3d5531e20e13b35683fa352671bd838726 commit 880e7d3d5531e20e13b35683fa352671bd838726 Author: Jaco Kroon <jaco@uls.co.za> AuthorDate: 2021-01-08 18:18:20 +0000 Commit: Sam James <sam@gentoo.org> CommitDate: 2021-01-22 03:50:56 +0000 net-misc/asterisk: 13.18.1-r2 version bump. * clang/LLVM support * subslots * autoconf 2.70 * drop /var/spool/asterisk from tmpfiles * Move check_extra_config (kernel checks) to pkg_pretend. * (Hopefully) fix use of $ED vs $D. * Other minor non-functional tweaks. Bug: https://bugs.gentoo.org/show_bug.cgi?id=731280 Bug: https://bugs.gentoo.org/show_bug.cgi?id=750581 Signed-off-by: Jaco Kroon <jaco@uls.co.za> Signed-off-by: Sam James <sam@gentoo.org> net-misc/asterisk/asterisk-13.38.1-r2.ebuild | 313 +++++++++++++++++++++ .../files/asterisk-13.18.1-r2-autoconf-2.70.patch | 10 + net-misc/asterisk/files/asterisk.tmpfiles2.conf | 1 + net-misc/asterisk/metadata.xml | 2 + profiles/arch/amd64/package.use.mask | 5 + profiles/arch/base/package.use.mask | 4 + profiles/arch/x86/package.use.mask | 5 + 7 files changed, 340 insertions(+) |