Skip to content

Conversation

@ken2812221
Copy link
Contributor

@ken2812221 ken2812221 commented Jul 15, 2018

Based on #13660 #13710 , add gitian tarball for RISC-V

@laanwj
Copy link
Member

laanwj commented Jul 17, 2018

ACK but only after we can compile with a compiler that doesn't produce a broken test_bitcoin. If inlining is broken, who knows what other bugs are introduced.
(see #13543 (comment) and following comments)
Maybe Ubuntu will provide a newer version of gcc at some point.

@ken2812221
Copy link
Contributor Author

@laanwj How about force it to use gcc-8?

@laanwj
Copy link
Member

laanwj commented Jul 19, 2018

Yes, that was what I meant. But how?

@ken2812221
Copy link
Contributor Author

ken2812221 commented Jul 19, 2018

Just see what I do in the fourth commit.

@laanwj
Copy link
Member

laanwj commented Jul 19, 2018

Didn't know that a package for that was available!
Can someone confirm that the resulting test_bitcoin runs without issues?

Edit: interesting;
I wonder if this is the only way to get the -8 variant to be used.
If there's no update-alternatives incantation to update the symlinks. This would be more convenient for the build guide.

Edit no, apparently the risc-v compiler packages do not provide alternatives

@laanwj
Copy link
Member

laanwj commented Jul 19, 2018

Concept ACK

@laanwj laanwj requested a review from theuni July 19, 2018 16:55
Copy link
Member

@theuni theuni left a 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.

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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

Copy link
Contributor Author

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

Copy link
Member

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.

Copy link
Contributor Author

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

Copy link
Member

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.

Copy link
Contributor Author

@ken2812221 ken2812221 Jul 19, 2018

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.

@laanwj laanwj added this to the 0.18.0 milestone Jul 20, 2018
@laanwj
Copy link
Member

laanwj commented Jul 20, 2018

I'm all for it for 0.18, but -1 from me for 0.17.

Added appropriate milestone

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 22, 2018

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.

@ken2812221 ken2812221 changed the title [build] Add risc-v support to gitian [WIP][build] Add risc-v support to gitian Jul 22, 2018
@bitcoin bitcoin deleted a comment from DrahtBot Aug 7, 2018
@maflcko
Copy link
Member

maflcko commented Aug 8, 2018

gitian linux failure:

ERROR: Feature 'system-zlib' was enabled, but the pre-condition 'libs.zlib' failed.

ERROR: Feature 'openssl-linked' was enabled, but the pre-condition '!features.securetransport && libs.openssl' failed.

ERROR: Feature 'xcb' was enabled, but the pre-condition 'libs.xcb' failed.

ERROR: Feature 'system-freetype' was enabled, but the pre-condition 'features.freetype && libs.freetype' failed.

ERROR: Feature 'fontconfig' was enabled, but the pre-condition '!config.win32 && !config.darwin && features.system-freetype && libs.fontconfig' failed.
funcs.mk:242: recipe for target '/home/ubuntu/build/bitcoin/depends/work/build/riscv64-linux-gnu/qt/5.9.6-c69c157d1d9/qtbase/.stamp_configured' failed
make: *** [/home/ubuntu/build/bitcoin/depends/work/build/riscv64-linux-gnu/qt/5.9.6-c69c157d1d9/qtbase/.stamp_configured] Error 3
make: Leaving directory '/home/ubuntu/build/bitcoin/depends'

@ken2812221
Copy link
Contributor Author

@MarcoFalke Sry, missing one line during rebasing #13710

@ken2812221
Copy link
Contributor Author

@MarcoFalke @DrahtBot fetch the wrong commit.

@laanwj
Copy link
Member

laanwj commented Aug 15, 2018

What is the reason this is still WIP? would like to merge this early in the 0.18 cycle if possible.
(going to test)

@ken2812221
Copy link
Contributor Author

@laanwj You can see @DrahtBot 's binary, it exports so many unnecessary symbols

@laanwj
Copy link
Member

laanwj commented Aug 16, 2018

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.
Now it's only a "nice" sanity and hygiene check, to avoid the overhead of huge unnecessary symbol tables in an executable.

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.

@ken2812221 ken2812221 changed the title [WIP][build] Add risc-v support to gitian [build] Add risc-v support to gitian Aug 16, 2018
@ken2812221
Copy link
Contributor Author

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

@laanwj
Copy link
Member

laanwj commented Aug 16, 2018

ACK c4aecd1
gitian build result tested on actual hardware (HiFive Unleashed=U540) w/ Fedora release 29 (Rawhide):

# ./test_bitcoin 
Running 303 test cases...
test/scheduler_tests.cpp(141): info: counter2++
test/scheduler_tests.cpp(141): info: counter2++
test/scheduler_tests.cpp(145): info:  == 
test/scheduler_tests.cpp(145): info: i          
*** No errors detected

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.

@laanwj laanwj merged commit c4aecd1 into bitcoin:master Aug 16, 2018
@ken2812221 ken2812221 deleted the gitian-risc-v branch August 16, 2018 18:36
@laanwj
Copy link
Member

laanwj commented Aug 23, 2018

Also synced the beginning of the chain, I'll keep this node running.

This was successful!
Synchronization of the full chain on the HiFive Unleashed board to a nbd partition took about 2-3 days.
(had to restart at some point as there were problems with the SD card interface or driver, but there were zero issues with bitcoind)

@bitcoin bitcoin deleted a comment from DrahtBot Aug 23, 2018
@bitcoin bitcoin deleted a comment from DrahtBot Aug 23, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants