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

Update Node.js to 20.10.0 LTS and replace appdmg with macdmg #3333

Closed
wants to merge 8 commits into from

Conversation

haslinghuis
Copy link
Member

No description provided.

@haslinghuis haslinghuis added the dependencies Pull requests that update a dependency file label Feb 9, 2023
@haslinghuis haslinghuis added this to the 10.10.0 milestone Feb 9, 2023
@haslinghuis haslinghuis self-assigned this Feb 9, 2023
@haslinghuis haslinghuis changed the title Update Node.js to 18.14.0LTS Update Node.js to 18.14.0 LTS Feb 9, 2023
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@haslinghuis
Copy link
Member Author

Dependency hell :)

@blckmn
Copy link
Member

blckmn commented Feb 9, 2023

AUTOMERGE: (FAIL)

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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@haslinghuis haslinghuis force-pushed the update-nodejs branch 2 times, most recently from ca373fe to 1ed972d Compare February 10, 2023 03:15
@sonarqubecloud
Copy link

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

This comment has been minimized.

@McGiverGim
Copy link
Member

It seems MAC build is failing?

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@haslinghuis
Copy link
Member Author

Already upgraded all dependencies and it works on Linux :)
Requires more work.

@haslinghuis haslinghuis marked this pull request as draft March 5, 2023 19:30
@haslinghuis haslinghuis force-pushed the update-nodejs branch 3 times, most recently from 0f2a7a2 to 46a5ec6 Compare May 23, 2023 18:28

This comment has been minimized.

Copy link

sonarqubecloud bot commented Dec 9, 2023

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

Copy link
Contributor

github-actions bot commented Dec 9, 2023

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!

@haslinghuis haslinghuis marked this pull request as ready for review December 9, 2023 20:51
@haslinghuis haslinghuis changed the title Update Node.js to 20.10.0 LTS Update Node.js to 20.10.0 LTS and replace appdmg with macdmg Dec 9, 2023
@ctzsnooze
Copy link
Member

This makes it very difficult on a mac.
It requires me to re-install rpm and that fails on my (older) mac.

@ctzsnooze
Copy link
Member

Doesn't work, wasted two hours on this.
Had to reinstall node, nvm, nothing worked.

@ctzsnooze
Copy link
Member

This is a summary:

chris:betaflight-config-ctzsnooze chris$ node -v
v20.10.0
chris:betaflight-config-ctzsnooze chris$ nvm -v
0.37.2
chris:betaflight-config-ctzsnooze chris$ gulp
/Users/chris/Documents/Personal/RC/betaflight-config-ctzsnooze/gulpfile.js:11
const NwBuilder = require('nw-builder');
                  ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/chris/Documents/Personal/RC/betaflight-config-ctzsnooze/node_modules/nw-builder/src/index.js from /Users/chris/Documents/Personal/RC/betaflight-config-ctzsnooze/gulpfile.js not supported.
Instead change the require of index.js in /Users/chris/Documents/Personal/RC/betaflight-config-ctzsnooze/gulpfile.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/Users/chris/Documents/Personal/RC/betaflight-config-ctzsnooze/gulpfile.js:11:19) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v20.10.0
chris:betaflight-config-ctzsnooze chris$ yarn start
yarn run v1.22.21
$ run-script-os
$ NODE_ENV=development NW_PRE_ARGS=--load-extension='./node_modules/nw-vue-devtools-prebuilt/extension' gulp debug
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/chris/Documents/Personal/RC/betaflight-config-ctzsnooze/node_modules/nw-builder/src/index.js from /Users/chris/Documents/Personal/RC/betaflight-config-ctzsnooze/gulpfile.js not supported.
Instead change the require of index.js in /Users/chris/Documents/Personal/RC/betaflight-config-ctzsnooze/gulpfile.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/Users/chris/Documents/Personal/RC/betaflight-config-ctzsnooze/gulpfile.js:11:19) {
  code: 'ERR_REQUIRE_ESM'
}
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
chris:betaflight-config-ctzsnooze chris$ 

@ctzsnooze
Copy link
Member

Well, don't ask me why... I tried to fix it by adding "type": "module" to package.json, and in gulpfile.js changing const NwBuilder = require('nw-builder'); to const NwBuilder = module ('nw-builder');
None of that worked, so I reverted those changes.
Then suddenly it started working!
These commands resulted in mac binaries...

  • yarn start orgulp makes a directory tree /debug/betaflight-configurator/osx64/, and in osx64/ the betaflight-configurator.app is locate (with other files). The app works and supports developer tools.
  • yarn release makes a directory release/ containing a mag dmg file which, when double-clicked, mounts as a volume and contains the betaflight-configurator.app. This file could be distributed 'as it is` without conversion to zip.

@chmelevskij
Copy link
Member

tbh, haven't tried i myself. @haslinghuis did you verify it actually works or does it need more testing?

Copy link
Member

@chmelevskij chmelevskij left a comment

Choose a reason for hiding this comment

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

Sorry, doesn't work on apple silicon:

[17:55:55] '<anonymous>' errored after 4.04 s
[17:55:55] Error in plugin "gulp-macdmg"
Message:
    Command failed: bless --folder /Volumes/Betaflight Configurator --openfolder /Volumes/Betaflight Configurator
bless: The 'openfolder' is not supported on Apple Silicon devices.


Details:
    code: 1
    stdout:
    stderr: bless: The 'openfolder' is not supported on Apple Silicon devices.

    failed: true
    signal: null
    cmd: bless --folder /Volumes/Betaflight Configurator --openfolder /Volumes/Betaflight Configurator
    timedOut: false
    killed: false
    domainEmitter: [object Object]
    domainThrown: false

Weirdly enough it worked for start development mode, but got the error on release 🤔

@haslinghuis
Copy link
Member Author

Did you check if node.js is the arm64 (not x64) version on your Mac?

@ctzsnooze
Copy link
Member

I have had issues with stalled debug saves, slow-downs when pasting a long CLI diff all, and stalled saved Preset imports, requiring a reboot of Configurator, since testing this PR on my mac.

Let's not go down this path until the dust has settled on 4.5. Right now, at release time for 4.5, is not the best time to change these things, I think.

@nerdCopter
Copy link
Member

fully worked on Debian. installable and works. will not approve until OSX issues reported as resolved.

@haslinghuis haslinghuis added this to the 11.0 milestone Dec 11, 2023
@haslinghuis
Copy link
Member Author

Closing in favor of #3673

@haslinghuis haslinghuis deleted the update-nodejs branch December 17, 2023 23:14
@haslinghuis haslinghuis removed this from the 11.0 milestone Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Don't merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants