net-libs/nodejs: remove gcc dependency #33141
Conversation
Previously, libgcc was needed for `__atomic_is_lock_free`, which was originally not implemented in LLVM's compiler-rt. With this LLVM upstream commit[0] being merged, gcc is now unnecessary. This commit reverts 7dc748f for all nodejs ebuilds. [0] llvm/llvm-project@00530de Bug: https://bugs.gentoo.org/869992 Closes: https://bugs.gentoo.org/911340 Signed-off-by: Siva Mahadevan <me@svmhdvn.name>
Pull Request assignmentSubmitter: @svmhdvn net-libs/nodejs: At least one of the listed packages is maintained entirely by non-GitHub developers! Linked bugsIn order to force reassignment and/or bug reference scan, please append Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide |
gentoo-bot
added
need assignment
|
@williamh assigning to you. Not sure why the bot has your name crossed out. |
Pull request CI reportReport generated at: 2023-09-30 19:25 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
|
I don't think this adds up, though. The LLVM commit cited was already in the versions in the linked bugs. Maybe the issue was a combination of whatever was passing -latomic (was it nodejs?) and the symbols missing, but the timeline still doesn't seem right. |
|
Even if the explanation isn't quite correct and the timeline isn't right, is it ok to go through with this PR? I have tested this build locally with the musl+clang Gentoo system profile and things seem to be working fine without gcc. I can change the commit message to simply say it's an unnecessary dependency now. |
|
I think you need to inspect the build system to at least make sure it doesn't try to pass -latomic by itself on any architecture when using compiler-rt. Skimming, I see https://github.com/nodejs/node/blob/092fb9f541ce8cc07289b5a69eb93892445739f5/tools/v8_gypfiles/v8.gyp#L1076 still which implies this remains broken on arm at least. Would probably be worth trying on i586 (a chroot on amd64 is fine) too. |
|
Thanks for the help. I tried to test these builds using sys-devel/crossdev, but there's a bug[0] I found on the musl+clang profile that isn't allowing me to build the required cross toolchains. Without using a cross build environment, it might take a while for me to spin up the required VMs to test this on a few arches. In the meantime, if anyone wants to continue this PR, please feel free. |
|
I was able to build node on musl+clang using the patch from the Chimera Linux repo here which is similar to the patch in this pr but also removes the section referenced above and so should build on arm and other architectures as well. I've got an arm machine that I'd be happy to test on this week, would love to see this pr merged if possible! |
| @@ -41,8 +41,7 @@ RDEPEND=">=app-arch/brotli-1.0.9:= | |||
| sys-libs/zlib | |||
| corepack? ( !sys-apps/yarn ) | |||
| system-icu? ( >=dev-libs/icu-67:= ) | |||
| system-ssl? ( >=dev-libs/openssl-1.1.1:0= ) | |||
| sys-devel/gcc:*" | |||
| system-ssl? ( >=dev-libs/openssl-1.1.1:0= )" | |||
| BDEPEND="${PYTHON_DEPS} | |||
| dev-util/ninja | |||
There was a problem hiding this comment.
This needs to be changed to
| dev-util/ninja | |
| app-alternatives/ninja |
I guess a rebase would solve it.
|
I don't have access to a Gentoo system anymore. I'd like to see this PR be merged though, so it would be nice if someone could take over this PR. |
Previously, libgcc was needed for
__atomic_is_lock_free, which was originally not implemented in LLVM's compiler-rt. With this LLVM upstream commit[0] being merged, gcc is now unnecessary. This commit reverts 7dc748f for all nodejs ebuilds.[0] llvm/llvm-project@00530de
Bug: https://bugs.gentoo.org/869992
Closes: https://bugs.gentoo.org/911340