Skip to content

Bump Arm build to ARMv8 #2940

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 5, 2022
Merged

Bump Arm build to ARMv8 #2940

merged 1 commit into from
Jul 5, 2022

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Jun 2, 2022 •
edited
Loading

@haslinghuis haslinghuis self-assigned this Jun 2, 2022
@github-actions

This comment has been minimized.

Sign in to view
@blckmn
Copy link
Member

blckmn commented Jun 3, 2022 •
edited
Loading

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> FAIL
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> FAIL
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

McGiverGim
McGiverGim reviewed Jun 3, 2022
View reviewed changes
Copy link
Member

@McGiverGim McGiverGim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too sure if we need to remove armv7 support or support both. The only difference I suppose is the downloadUrl?

@haslinghuis
Copy link
Member Author

haslinghuis commented Jun 3, 2022

Raspberry Pi 3 can run on ARMv8 kernel not sure about other platforms.

@McGiverGim
Copy link
Member

McGiverGim commented Jun 3, 2022

The original PR is here: #1024

@nmaggioni please, can you help with this?

@nmaggioni
Copy link
Contributor

nmaggioni commented Jun 3, 2022

The latest NW.JS community builds should be compatible with both ARMv7 and v8, but as far as I understood (and already linked to HN by @haslinghuis) it will be down to the specific hardware implementations to properly support them or not.
I'd say go for it and drop the v7-only build, it's not like it's a huge changeset to monkeypatch back in if needed.

nmaggioni
nmaggioni reviewed Jun 3, 2022
View reviewed changes
gulpfile.js Outdated
}
const downloadedArchivePath = `${versionFolder}/nwjs${flavorPostfix}-v${nwArmVersion}-linux-arm.tar.gz`;
const downloadUrl = `https://github.com/LeonardLaszlo/nw.js-armv7-binaries/releases/download/v${nwArmVersion}/nwjs${flavorDownloadPostfix}-v${nwArmVersion}-linux-arm.tar.gz`;
const downloadUrl = 'https://github.com/LeonardLaszlo/nw.js-armv7-binaries/releases/download/nw60-arm64_2022-01-08/nw60-arm64_2022-01-08.tar.gz';
Copy link
Contributor

@nmaggioni nmaggioni Jun 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoding SDK URLs deep down in the build code is almost never a good idea, especially if prospecting to have multiple builds branch out of this one. I'd keep the modular URL approach even if needs to be simplified to a single global var.

Copy link
Member Author

@haslinghuis haslinghuis Jun 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2022

Do you want to test this code? Here you have an automated build:
Betaflight-Configurator-Android
Betaflight-Configurator-Linux
Betaflight-Configurator-macOS
Betaflight-Configurator-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

@McGiverGim McGiverGim requested a review from blckmn July 2, 2022 07:38
@blckmn blckmn merged commit f9ac9f8 into betaflight:master Jul 5, 2022
@haslinghuis haslinghuis mentioned this pull request Aug 10, 2022
Closed
@haslinghuis haslinghuis deleted the bump-arm-build branch August 10, 2022 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@McGiverGim McGiverGim McGiverGim approved these changes

@limonspb limonspb limonspb approved these changes

@blckmn blckmn blckmn approved these changes

+1 more reviewer

@nmaggioni nmaggioni nmaggioni left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@haslinghuis haslinghuis

Labels

RN: BUILD SYSTEM

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Need builds for Apple Silicon (arm) chips.

5 participants

@haslinghuis @blckmn @McGiverGim @nmaggioni @limonspb