-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Fix instruction for RISC-V (the previous instruction didn't work) #10374
Merged
skyace65
merged 5 commits into
godotengine:master
from
OS-of-S:RISC-V-instruction-edited
Dec 26, 2024
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We shouldn't remove this section for telling the user to install Clang, even if they can use the toolchain-provided one. The system clang worked for me, and there is no harm in having it. It's also important to tell the user that GCC does not work, and telling the user how to check if Clang supports the target.
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.
Well, the Clang dosn't work for me! It may be matter on the version you're using. What versions of Clang and riscv-gnu-toolchain do you use?
My versions:
Also a small note about riscv-gnu-toolchain: the official Godot documentation says: "the older the toolchain, the more compatible our final binaries will be", and recommends installing version 2021.12.22-ubuntu-18.04. But I installed version 2023.06.09-ubuntu-20.04, since this is the oldest version with which I was able to compile Godot. For newer ones I get the error
mold: fatal: huf_decompress_amd64.linuxbsd.editor.rv64.llvm.o: incompatible file type: riscv64 is expected but got x86_64
Most likely, a more correct way to install the necessary tools exists (I suspect, related to configuring Clang). It should also be noted that I did not check the functionality of the resulting Godot builds, and given the content of the forms on this topic, there is a high chance that they won't even start.
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.
The rest of the comments make sense and I fully agree with them. I will make corrections regarding the gcc and correct the command for the terminal.
But I still don't know what to say about Clang. May be you have an idea for appropriate wording that will not confuse the reader?
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.
For RISC-V, this platform is so new that the advice about compiling for older Ubuntu versions for better compatibility is superseded by the need to use newer systems for including critical bugfixes. I have upgraded my Ubuntu systems to 24.04 since I last tried this, and I think that is fine as a baseline, given that RISC-V devices are still to this day very niche and will be for several years.
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.
Ok, so... No changes on this page needed?
I still think that if the toolchain have it's own Clang with which it works fine insted of newer one version of Clang we shouldn't insist on installing different Clang. I don't know how it works, but I have an issue with it. Otherwise we need to add a comment that the user should rely on newer versions of Linux (which is already contrary to the documentation and makes it more complicated for user).
I made changes on all other claims in my PR. Maybe we could add such note in page:
"You can use a self-installed version of Clang, but it may have compatibility issues with riscv-gnu-toolchain."
Will this be enough? I'll wait for your opinion.