Skip to content
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

Replace binary toolchain with code to download it from SiFive #23

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

v3l0c1r4pt0r
Copy link

It should be agreed if this is the right way to get rid of these binaries. For now I just want to prepare a list of files that we know how to recreate from scratch. This is just a first batch. If bin directory turned out to be the same as in SiFive distribution, then I expect I will end up with whole Linux directory removed.

This is still a work in progress. I will try to gradually delete more files.

Files found in
https://static.dev.sifive.com/dev-tools/riscv64-unknown-elf-gcc-8.3.0-2019.08.0-x86_64-linux-centos6.tar.gz
do not have to be stored in binary form in this repo as they have known
build script and could be rebuilt from source.
@v3l0c1r4pt0r
Copy link
Author

Now I can confirm. There is only file left in toolchain/riscv/Linux - .generated.linux.gtk.x86_64. Rest is the exact copy of riscv64-unknown-elf-gcc-8.3.0-2019.08.0-x86_64-linux-centos6. Nothing more nothing less.

@schaecsn
Copy link

For the the distribution cross-compiler to be found, one also needs something like this. Do you have that in your long patch?

diff --git a/make_scripts_riscv/toolchain.mk b/make_scripts_riscv/toolchain.mk
index 5b61d705..0fd6da49 100644
--- a/make_scripts_riscv/toolchain.mk
+++ b/make_scripts_riscv/toolchain.mk
@@ -1,3 +1,3 @@
 ##change to your toolchain path
-CONFIG_TOOLPREFIX ?= $(BL60X_SDK_PATH)/toolchain/riscv/$(shell uname |cut -d '_' -f1)/bin/riscv64-unknown-elf-
-#CONFIG_TOOLPREFIX ?= riscv64-unknown-elf-
+#CONFIG_TOOLPREFIX ?= $(BL60X_SDK_PATH)/toolchain/riscv/$(shell uname |cut -d '_' -f1)/bin/riscv64-unknown-elf-
+CONFIG_TOOLPREFIX ?= riscv64-linux-gnu-

I did not get the whole thing compiling with debian 10's riscv64 crosscompiler. Did you?


/opt/src/bl_iot_sdk/customer_app/bl602_demo_event$ ./genromap
****** Please SET BL60X_SDK_PATH ******
****** Trying SDK PATH [/mnt/dell/opt/src/bl_iot_sdk/customer_app/bl602_demo_event/../..]
use exsting version.txt file
CC build_out/aws-iot/aws-iot-device-sdk-embedded-C/src/aws_iot_jobs_interface.o
In file included from /usr/riscv64-linux-gnu/include/sys/cdefs.h:442,
                 from /usr/riscv64-linux-gnu/include/features.h:424,
                 from /usr/riscv64-linux-gnu/include/bits/libc-header-start.h:33,
                 from /usr/riscv64-linux-gnu/include/stdio.h:27,
                 from /mnt/dell/opt/src/bl_iot_sdk/components/3rdparty/aws-iot/aws-iot-device-sdk-embedded-C/include/aws_iot_mqtt_client_interface.h:47,
                 from /mnt/dell/opt/src/bl_iot_sdk/components/3rdparty/aws-iot/aws-iot-device-sdk-embedded-C/include/aws_iot_jobs_interface.h:37,
                 from /mnt/dell/opt/src/bl_iot_sdk/components/3rdparty/aws-iot/aws-iot-device-sdk-embedded-C/src/aws_iot_jobs_interface.c:16:
/usr/riscv64-linux-gnu/include/bits/wordsize.h:28:3: error: #error "rv32i-based targets are not supported"
 # error "rv32i-based targets are not supported"
   ^~~~~

@v3l0c1r4pt0r
Copy link
Author

@schaecsn I think you got me wrong.

Maybe I'll start from beginning. I compared the toolchain in Linux directory here with toolchain distributed by SiFive here. Then I removed each file that have same checksum as in this linked package from this repository. So, it has nothing to do with toolchain available in any operating system. SiFive is developing a toolchain, where they introduce their own changes, so that's why having this SDK compile with other compiler might be a problem. But at least, unlike in this repo, they provide a method to build their toolchain from source.

Surely, we can't just delete the toolchain and leave the rest unchanged, because this breaks the rest completely. My idea is to download and unpack compiler from SiFive to the exactly same path, where these were present. But it would also be nice to integrate this with the rest of the SDK, so it does not have to be done manually. And for this, I don't have a solution, yet.

@gamelaster
Copy link
Member

Hi @v3l0c1r4pt0r ,
Thank for your contribution. Even the PR is not merged yet, you contributed a lot in issues with interesting information, so you can apply for free EVB.
For receiving the free PineCone, please sign up at this link.

Download tarball from SiFive and extract to exactly same location, where
it was committed, just before attempting to make any further make rules.

File .generated.linux.gtk.x86_64 is used as an indicator that toolchain
is fine, so download and unpack is done only, if it's not there. This
avoids redownloading on every make call.
Whole toolchain should not make its way into repository, anymore
Using .generated* of a name that was used before would break Darwin and
MSYS. Now they should work, too.
@v3l0c1r4pt0r
Copy link
Author

I made toolchain recreation part. Now it works the way, that when for the first time any makefile from within the SDK is called, it downloads the toolchain from SiFive and unpacks to the right directory. Next time it will simply work as usual. This is for Linux part, same should be possible also for Windows and OS X.

Is there something to improve? Should it be done different way? Let me know, what do you think in comments.

@v3l0c1r4pt0r v3l0c1r4pt0r changed the title [WiP] Remove parts of toolchain from SiFive distribution Replace binary toolchain with code to download it from SiFive Oct 29, 2020
@v3l0c1r4pt0r v3l0c1r4pt0r marked this pull request as ready for review October 29, 2020 19:52
@Avamander
Copy link

Avamander commented Oct 29, 2020

I don't think non-standard dependency management is truly the right solution, it has somewhat tedious downsides - it doesn't work on non-Linux, it would mandate a specific approach to downloading (e.g. it's more difficult to use your local mirror in your CI), it's unclear without reading the Makefile and it has to be maintained.

My personal opinion is that if an identical toolchain is downloadable from external source, it's fine to remove it from this repository but should be added as a step to the quick start guide. This would be a platform-independent, common approach.

The decision in the end is @gamelaster's though.

@v3l0c1r4pt0r
Copy link
Author

@Avamander In fact you can still provide the toolchain yourself. You just have to remember in the current state of this PR to create .installed file in the right directory. Note also that I made changes only for Linux. MSYS and Darwin should work as they were at the moment.

But you are right, automating things have its downsides. But simply removing is also not an option. So this is for maintainers to decide whether they would like to keep this huge binary package, remove it and tell users to download on their own, or have it automated in some way (not necessarily the way I did).

@Avamander
Copy link

You just have to remember in the current state of this PR to create .installed file in the right directory.

Yes, but that's self-invented dependency management with the downsides I listed. Something like Meson would make more sense but that's a lot more work.

But simply removing is also not an option.

Of course it is. No common SDK ships the arch's toolchain with it e.g. if you look at the Nordic SDK you have to download ARM's GCC yourself.

@v3l0c1r4pt0r
Copy link
Author

Of course it is. No common SDK ships the arch's toolchain with it e.g. if you look at the Nordic SDK you have to download ARM's GCC yourself.

I mean, at least adding a description on how to get the toolchain is a must in my opinion.

By the way, it should be possible to configure Github Actions to prepare a tarball where we could have all toolchain binaries ready to use for average users and publish it on Github as release from time to time. But I truly hate the way of developing Actions. Maybe I just never discovered the right way to it.

@Avamander
Copy link

@v3l0c1r4pt0r

I mean, at least adding a description on how to get the toolchain is a must in my opinion.

Oh yes, definitely.

By the way, it should be possible to configure Github Actions to prepare a tarball where we could have all toolchain binaries ready to use for average users and publish it on Github as release from time to time. But I truly hate the way of developing Actions. Maybe I just never discovered the right way to it.

Theoretically yes, I think that's a topic for an another issue.

@v3l0c1r4pt0r
Copy link
Author

@Avamander

Theoretically yes, I think that's a topic for an another issue.

I know. I hope someone willing to get his reward is listening :)

@gamelaster gamelaster added the needs testing The functionality needs to be compiled/tested label Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs testing The functionality needs to be compiled/tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants