Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 745744 - llvm.eclass: needs get_llvm_prefix change for cross-compilation (was: www-client/firefox-?: cross-compilation fails)
Summary: llvm.eclass: needs get_llvm_prefix change for cross-compilation (was: www-cli...
Status: UNCONFIRMED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Eclasses (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Michał Górny
URL:
Whiteboard:
Keywords: PATCH
Depends on:
Blocks:
 
Reported: 2020-09-30 13:43 UTC by David Michael
Modified: 2023-09-03 08:48 UTC (History)
7 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Michael 2020-09-30 13:43:37 UTC
I hit this error since Firefox was updated to EAPI 7 with 81.0 today.  When cross-compiling, the llvm_pkg_setup function defines PATH to the cross-compiled LLVM bindir in the sysroot by default.  This is an error because either the cross-compiled programs will not be natively executable, or they don't exist at all in the sysroot because LLVM should be in BDEPEND with EAPI 7 (as is the case with the Firefox update).

I think that the pkg_setup function should unconditionally use -b to always set up PATH for the natively executable LLVM programs.  (Or at least be conditional on EAPI 7, since the function throws an error with EAPI 6.)

This is the change I tested to make Firefox compile:

--- eclass/llvm.eclass
+++ eclass/llvm.eclass
@@ -198,7 +198,7 @@
 	debug-print-function ${FUNCNAME} "${@}"
 
 	if [[ ${MERGE_TYPE} != binary ]]; then
-		local llvm_path=$(get_llvm_prefix "${LLVM_MAX_SLOT}")/bin
+		local llvm_path=$(get_llvm_prefix -b "${LLVM_MAX_SLOT}")/bin
 		local IFS=:
 		local split_path=( ${PATH} )
 		local new_path=()
Comment 1 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2020-09-30 17:28:35 UTC
I don't think that's correct.  When linking to LLVM libraries, you certainly want it in sysroot.
Comment 2 David Michael 2020-09-30 18:51:50 UTC
But the only effect of the pkg_setup function is to set PATH to find executable binaries.  The llvm-config command is a compiled ELF binary, so if PATH is pointed at a cross-compiling sysroot, it can't be executed (if it exists in the sysroot at all, which it probably won't).  I've used -target and --sysroot previously with LLVM-related stuff and never had an issue with no LLVM installation in the sysroot.  The change definitely fixes building Firefox 81.0 at least, but if that's not acceptable, what would you suggest?
Comment 3 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2020-10-01 11:11:09 UTC
llvm-config is deprecated and shouldn't be used.  CMake uses paths relative to PATH to find CMake files.
Comment 4 tt_1 2020-10-01 12:53:04 UTC
hi, can you please rename the title so that it reflects failure of firefox to cross compile? it would be helpfull that people can actually find the bug if they don't know that their compile error is caused by the llvm.eclass 

also I believe that this can fix part of the problems encountered at cross compiling clang, see https://bugs.gentoo.org/731264

thanks
Comment 5 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2020-10-01 13:37:35 UTC
The idea is that you should use get_llvm_prefix with appropriate arguments directly if you need non-default behavior.
Comment 6 David Michael 2020-10-01 13:45:25 UTC
Okay, the change to fix cross-compiling Firefox would be to have the following lines for configure.  (I think llvm-config is required to differentiate lib/lib64.)  The llvm_pkg_setup call could be dropped since it doesn't work.

--with-clang-path="$(get_llvm_prefix -b)/bin/clang++"
--with-libclang-path="$("$(get_llvm_prefix -b)/bin/llvm-config" --libdir)"
Comment 7 tt_1 2020-10-01 13:50:49 UTC
I can try the fix in the evening, can you give an actuall git diff to firefox-81.0.1 ebuild, including dropped eclass etc. pp?
Comment 8 David Michael 2020-10-01 13:56:13 UTC
--- a/www-client/firefox-81.0.1.ebuild
+++ b/www-client/firefox-81.0.1.ebuild
@@ -388,8 +388,6 @@
 
 		check-reqs_pkg_setup
 
-		llvm_pkg_setup
-
 		python-any-r1_pkg_setup
 
 		# Avoid PGO profiling problems due to enviroment leakage
@@ -609,7 +607,8 @@
 		--target="${CHOST}" \
 		--without-ccache \
 		--with-intl-api \
-		--with-libclang-path="$(llvm-config --libdir)" \
+		--with-clang-path="$(get_llvm_prefix -b)/bin/clang++" \
+		--with-libclang-path="$("$(get_llvm_prefix -b)/bin/llvm-config" --libdir)" \
 		--with-system-nspr \
 		--with-system-nss \
 		--with-system-png \
Comment 9 Thomas Deutschmann (RETIRED) gentoo-dev 2020-10-01 14:23:29 UTC
>  0:10.79 ERROR: --with-clang-path is not valid when the target compiler is clang

...and I am really not convinced that this is a Mozilla-only problem but I am not a cross-compile expert :/
Comment 10 David Michael 2020-10-01 15:20:51 UTC
It looks like --with-clang-path explicitly aborts when the default compiler is clang to use that instead, so the option could be conditionalized with "usex !clang" for people with USE=clang.

Alternatively, since omitting --with-clang-path defaults to clang++ in $PATH, you could drop the option and set PATH+=":$(get_llvm_prefix -b)/bin" before configure so it finds clang++ without specifying the full path.  (This is what llvm_pkg_setup should handle, hence the original bug.)
Comment 11 Thomas Deutschmann (RETIRED) gentoo-dev 2020-10-01 15:52:07 UTC
(In reply to David Michael from comment #10)
> Alternatively, since omitting --with-clang-path defaults to clang++ in
> $PATH, you could drop the option and set PATH+=":$(get_llvm_prefix -b)/bin"
> before configure so it finds clang++ without specifying the full path. 
> (This is what llvm_pkg_setup should handle, hence the original bug.)

That's the reason why I believe this is a bug in llvm eclass.
Comment 12 David Michael 2020-10-02 03:45:11 UTC
In case this turns into another Gentoo permabug and anyone finds it, this is the fix you want to apply locally.  I have not encountered an issue with it in the couple hundred packages I build.

--- eclass/llvm.eclass
+++ eclass/llvm.eclass
@@ -198,7 +198,7 @@
 	debug-print-function ${FUNCNAME} "${@}"
 
 	if [[ ${MERGE_TYPE} != binary ]]; then
-		local llvm_path=$(get_llvm_prefix "${LLVM_MAX_SLOT}")/bin
+		local llvm_path=$(get_llvm_prefix $([[ $EAPI == 6 ]] || echo -b) "${LLVM_MAX_SLOT}")/bin
 		local IFS=:
 		local split_path=( ${PATH} )
 		local new_path=()
Comment 13 Nikolay Kichukov 2021-12-23 23:55:39 UTC
As it was already suggested, same issue exists for www-servers/apache-2.4.51-r1 when cross-compiling with clang.
Comment 14 tt_1 2023-07-09 12:29:24 UTC
Is the patch still needed with recent llvm.eclass changes? 

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=8e9c05ddfcd74be3b53c7c4c2fbc799a9ab7fa61
Comment 15 David Michael 2023-07-10 17:53:43 UTC
(In reply to tt_1 from comment #14)
> Is the patch still needed with recent llvm.eclass changes? 
> 
> https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=8e9c05ddfcd74be3b53c7c4c2fbc799a9ab7fa61

I don't think the patches here have applied for a while.  I tried building Firefox without edits to the eclass, and it seems to have worked, but my environment has other changes that might be interfering.

The issue was from putting cross-compiled binaries in PATH, and it looks like the eclass still does that.  It could now be addressed by changing prefix=${ESYSROOT} to prefix=${BROOT} (visible in the diff context at that URL) if you still experience failures from this.
Comment 16 tt_1 2023-09-03 08:48:50 UTC
No, I don't experience any failures anymore due to this bug. Just rebootstrapped my cross-toolchain to confirm.