-
Notifications
You must be signed in to change notification settings - Fork 19
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
Fixup build on modern gcc #26
Conversation
This is excellent, have you tested flashing after build and all features working ? Which modern gcc did you use ? |
I'm scared. I only have one buds ^_^
gcc version 11.3.0 (Ubuntu 11.3.0-1ubuntu1~22.04) |
Thats fair enough; happy to do the testing here. Sad to report that while this does make the build work; the resulting file wont run on the buds. They just refuse to boot the program. I'm suspect that something changes the start of the file that the bootloader is looking at to decide if it starts the file or not. |
I'm not sure what the issue is here either. Maybe it's specific to your buds, @Ralim? I would test this PR myself, but I don't have any buds yet. I'm looking to base my PR for conversion to CMake on this PR. So if we could rebase it against mainline, that'd be great. Or I could do that, and push, if I had access (not that I'm asking for it, but I'd be happy to rebase this PR against main and get it ready for testing). Cheers. |
@shymega Would basically need to make build as deterministic as possible then run it on both compilers and compare the output section by section to find it. In the past when I ran into this, cleaning up the compiler chain helped flush out the quirks that caused these issues. |
Closes pine64#26, this is the rebased and newer version. Co-authored-by: Dom Rodriguez <[email protected]>
Closes pine64#26, this is the rebased and newer version. Co-authored-by: Dom Rodriguez <[email protected]>
@neochapay I want to get your opinion on something. I opened #55 because I didn't want to mess up your PR, but it messes the tracker. Do you mind if we force-push your PR to fix the merge conflicts? I'm not keen on opening yet another PR to rebase. I am looking forward to your response. |
9d6d64c
to
faa37ca
Compare
@shymega i was update and fix all conflicts |
@neochapay Thanks. However, my query was more relating to whenever we, maintainers, can edit your PR. I see we can push, but I want to get your explicit approval. |
Closes #26, this is the rebased and newer version. Co-authored-by: Dom Rodriguez <[email protected]>
Rebased against main. @Ralim Can we get this merged once tests pass? |
No description provided.