-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
enhance(libllvm) Using relative path for sysroot. #25178
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
base: master
Are you sure you want to change the base?
Conversation
2355d26
to
26696c5
Compare
ERROR: Version mismatch between libllvm and flang. libllvm=20.1.7:1, flang=20.1.7:0 |
This makes us one step closer to achieve dynamic prefix. termux#24407 A relative path for the sysroot was first discussed on WebAssembly/wasi-sdk#58 And got implemented on the upstream libllvm back in 2020 https://reviews.llvm.org/D76653
Very good idea! I really think this is a very cool idea and I hope it works. There is just one potential problem, which is that I am pretty sure some applications that are packaged in Termux cannot work properly if there are relative paths in some of their internal directories. When it finishes I will try testing your PR and I will let you know what happens and if anything has an error! |
@robertkirkman Glad that you liked the idea.
I think that there should be no problems because llvm automaticlly converts the sysroot to absolute path. if ((!SysRoot.empty()) && llvm::sys::path::is_relative(SysRoot)) {
// Prepend InstalledDir if SysRoot is relative
SmallString<128> P(InstalledDir);
llvm::sys::path::append(P, SysRoot);
SysRoot = std::string(P);
} SysRoot is set to the absolute path. So there shouldn't be any side effects. |
The CI finally finished .. After only 5 hours 🥴 |
That is to be expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this with building some packages on-device and some software both with and without build-package.sh
, and I actually don't see any problems introduced by this. Good job!
I thought at first that these commands might be able to produce a problem:
cd $HOME
mkdir -p code
cd code
cp $PREFIX/bin/clang-20 .
./clang-20 -c test.c
however, those commands currently produce an error no matter whether this PR is used or not, so if this PR helps you, it is good since it doesn't seem to break anything I have found so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit busy right now, give me some time to look into this next week.
Busy with this? Ewww! https://forums.swift.org/t/announcing-the-android-workgroup/80666 |
That's alright. Take your time. |
Before this and even now, this is assuming that I assume
|
Let us know when the rewrite of Termux-app in Swift starts, looking forward to trying it. 😛 |
Eww eww ewww!!! Don't even say those words! Gosh! |
I don't think that i completely understand but is this right?
When it is installed inside /system/bin the sysroot should be /system maybe? I tried to search for usages for TERMUX__ROOTFS inside packages to understand better but I didn't find any |
I have seen certain forks use something like So if both So if prefix and rootfs are equal, then go one directory up
Yes.
Yeah, rootfs variable isn't really used other than some termux internal packages as it was recently added and packages are more concerned with bin/lib, etc and those are referred by prefix variable. |
@agnostic-apollo I applied the changes that you suggested. if [ "$TERMUX__PREFIX" = "$TERMUX__ROOTFS" ]; then
DEFAULT_SYSROOT=".."
else
DEFAULT_SYSROOT="../.."
fi |
Thanks, that should work, I'll let package maintainers test it further. |
@finagolfin Sorry for the ping. But what do you think?
|
I'll take a look this week, got delayed. |
OK, got some time today, will take a look. |
This makes us one step closer to achieve dynamic prefix. (#24407)
A relative path for the sysroot was first discussed on WebAssembly/wasi-sdk#58
And got implemented on the upstream libllvm back in 2020 https://reviews.llvm.org/D76653