-
Notifications
You must be signed in to change notification settings - Fork 38.7k
[build] Add risc-v support to gitian #13665
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
Conversation
|
ACK but only after we can compile with a compiler that doesn't produce a broken |
|
@laanwj How about force it to use gcc-8? |
|
Yes, that was what I meant. But how? |
|
Just see what I do in the fourth commit. |
|
Didn't know that a package for that was available! Edit: interesting; Edit no, apparently the risc-v compiler packages do not provide alternatives |
|
Concept ACK |
theuni
left a comment
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.
Concept ACK, but I don't think it's wise for us to be producing supported binaries for a platform that's still going through teething issues with compilers. Especially since so few people are able to test.
I'm all for it for 0.18, but -1 from me for 0.17.
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 suspect that this will cause more glibc back-compat issues. That's fine, of course, they just need to be accounted for.
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've tested this. make -C src check-symbol is happy about this in gitian build.
depends/packages/qt.mk
Outdated
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.
This shouldn't be needed after #13696 (comment).
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.
This is not my commit, just based on #13696
src/compat/glibc_compat.cpp
Outdated
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.
Is this necessary? We don't have to support much back-compat for riscv, as its glibc support is pretty new anyway.
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.
This might not be necessary. But we should drop --enable-glibc-back-compat flag when building for RISC-V
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 think he did this to appease the symbols check, the alternative would be to do a different symbols check per target platform.
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 did this so the linker would know what log2f_old is.
Added appropriate milestone |
Note to reviewers: This pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
gitian linux failure: |
|
@MarcoFalke Sry, missing one line during rebasing #13710 |
|
@MarcoFalke @DrahtBot fetch the wrong commit. |
|
What is the reason this is still WIP? would like to merge this early in the 0.18 cycle if possible. |
|
I'm not sure how worried we should be about exporting unnecessary symbols on RISC-V at this point, from what I gather everything is still extremely experimental, there aren't even ready-to-use linux distribution images. The "don't export unnecessary symbols" check was, at the time, added to avoid collisions between statically-linked and dynamically-linked OpenSSL, back in the day that Qt was still linked dynamically. This is no longer an actual issue. So I don't think that needs to hold back progress, seems like be something that is improved in a later PR, but I'll leave it up to you. |
|
@laanwj Thanks for clarification. The export symbol things can be done in future PR. At least I can't solve the problem for now, leave for someone who can work on improving this. |
|
ACK c4aecd1 Also synced the beginning of the chain, I'll keep this node running. Going to merge this because I want to keep building for RISC-V on the 0.18 branch. If any issues come up we can revert this before the 0.18.0 release. |
This was successful! |
Based on
#13660#13710 , add gitian tarball for RISC-V