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

Windows RISC-V toolchain build #9

Merged
merged 15 commits into from
Dec 19, 2024
Merged

Conversation

ndabas
Copy link
Contributor

@ndabas ndabas commented Dec 14, 2024

I've cleaned up the Windows builds and the riscv-gnu-toolchain build works fine.

However I couldn't get it to use our pre-cloned copy of GCC -- it appears to be an issue with paths that they've tried to fix twice but it still does not work with an external GCC src on msys2/Windows.

That should not be a problem currently though because the gcc submodule points to the same branch that we are cloning ourselves, releases/gcc-14.

Can I ask what was the original motivation/intention behind cloning GCC separately?

Fixes #8.

Successful actions run and artifacts.

@will-v-pi
Copy link
Contributor

Thanks, this looks good - could you target it at the main branch rather than develop (unlike the other pico repos, this one still uses main directly, develop was just to test the develop branches of the tools)? Make sure to rebase onto the latest main, as I've just pushed a commit to run our actions on PRs.

Can I ask what was the original motivation/intention behind cloning GCC separately?

Because when this was released the toolchain was using GCC 13, so this provided a simple way to ensure we were using GCC 14 as recommended by some of our developers.

@ndabas ndabas changed the base branch from develop to main December 17, 2024 13:05
@ndabas
Copy link
Contributor Author

ndabas commented Dec 17, 2024

Thanks, I have rebased on main and re-targeted the PR.

Sounds good on the GCC version then, I guess we could remove our clone of GCC entirely in a future refactor.

@ndabas
Copy link
Contributor Author

ndabas commented Dec 18, 2024

@will-v-pi I see that you approved the workflow but it failed where it's trying to read an SSH key from secrets -- I guess that might have been needed when development was being done in private repos? So we could remove that step, or perhaps make it conditional, so it only runs on main?

@will-v-pi
Copy link
Contributor

@will-v-pi I see that you approved the workflow but it failed where it's trying to read an SSH key from secrets -- I guess that might have been needed when development was being done in private repos? So we could remove that step, or perhaps make it conditional, so it only runs on main?

If you just comment out those sections that would probably be best, so the code is still there if we need it later, but doesn't get used as it's not required right now.

@ndabas
Copy link
Contributor Author

ndabas commented Dec 18, 2024

If you just comment out those sections that would probably be best, so the code is still there if we need it later, but doesn't get used as it's not required right now.

Sure, done.

@will-v-pi
Copy link
Contributor

Sorry, I meant for all the jobs (so the MacOS and Linux jobs as well)

@ndabas
Copy link
Contributor Author

ndabas commented Dec 18, 2024

Sorry, I meant for all the jobs (so the MacOS and Linux jobs as well)

Ah, okay, no worries -- done. I didn't want to touch the non-Windows related parts of the build so did not assume that.

@ndabas
Copy link
Contributor Author

ndabas commented Dec 18, 2024

Okay so looks like the Windows build works but there's something wrong with the macOS one. Should we consider this good to go and fix the macOS one separately? Or would you like me to give that a shot in this PR too? (It will take me a few days though.)

@will-v-pi
Copy link
Contributor

I'm happy to merge this as-is, and fix the MacOS build separately, as it looks like that was a pre-existing issue with the current Risc-V toolchain.

@will-v-pi will-v-pi linked an issue Dec 19, 2024 that may be closed by this pull request
@will-v-pi will-v-pi merged commit fa2d797 into raspberrypi:main Dec 19, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows RISC-V build can be fixed Wrong GCC version in Windows toolchain name
2 participants