-
-
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.
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