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

macos-notification-state module imported but unavailable on non-macOS #2334

Closed
3 tasks done
selfisekai opened this issue Oct 31, 2022 · 8 comments
Closed
3 tasks done

Comments

@selfisekai
Copy link

I confirm (by marking "x" in the [ ] below: [x]):


Summary
App imports macOS-specific native node module, which does not get built on Linux by module author's design

Environment

  • Operating System: Alpine Linux edge
  • Mattermost Desktop App version: 5.2.0
  • Mattermost Server version: N/A

Steps to reproduce
try opening a mattermost-desktop build in electron

Expected behavior
app opens successfully

Observed behavior
app fails to open, the electron process keeps running anyway. in terminal:

A JavaScript error occurred in the main process
Uncaught Exception:
Error: Could not locate the bindings file. Tried:
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/build/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/build/Debug/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/build/Release/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/out/Debug/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/Debug/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/out/Release/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/Release/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/build/default/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/compiled/16.16.0/linux/x64/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/addon-build/release/install-root/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/addon-build/debug/install-root/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/addon-build/default/install-root/notificationstate.node
 → /usr/lib/mattermost/app.asar/node_modules/macos-notification-state/lib/binding/node-v109-linux-x64/notificationstate.node
    at bindings (/usr/lib/mattermost/app.asar/node_modules/bindings/bindings.js:126:9)
    at Object.<anonymous> (/usr/lib/mattermost/app.asar/node_modules/macos-notification-state/lib/index.js:1:34)
    at Module._compile (node:internal/modules/cjs/loader:1120:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1175:10)
    at Module.load (node:internal/modules/cjs/loader:988:32)
    at Module._load (node:internal/modules/cjs/loader:829:12)
    at c._load (node:electron/js2c/asar_bundle:5:13339)
    at Module.require (node:internal/modules/cjs/loader:1012:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.defineProperty.value (/usr/lib/mattermost/app.asar/index.js:53778:18)

Possible fixes

do not import macos-notification-state if not on macOS

@tboulis
Copy link
Contributor

tboulis commented Nov 1, 2022

Hello @selfisekai, which node version are you using?
Im also running Mattermost on linux with no issues

@selfisekai
Copy link
Author

$ node --version
v16.18.0
$ ELECTRON_RUN_AS_NODE=1 electron --version --
v16.16.0
$ electron --version
v21.2.1

@devinbinnie
Copy link
Member

@selfisekai We don't generally support Alpine Linux just in general, so there could be an issue building. Did you make sure to install all the required dependencies as per this document: https://developers.mattermost.com/contribute/more-info/desktop/developer-setup/

It looks to me like the version that was built is missing those packages, and those files are required for the app to run, despite not being necessary for the specific OS you're using.

@selfisekai
Copy link
Author

ok, I found the root cause. macos-notification-state does not define an install script, or even depend on node-gyp. I actually have no clue how does this package work anywhere. anyway, this means npm rebuild macos-notification-state does nothing. so, my current solution is this:

	(
		cd node_modules/macos-notification-state
		../.bin/node-gyp rebuild --build-from-source --nodedir=/usr/include/electron/node_headers
	)

I'm gonna make a PR to the module

@devinbinnie
Copy link
Member

ok, I found the root cause. macos-notification-state does not define an install script, or even depend on node-gyp. I actually have no clue how does this package work anywhere. anyway, this means npm rebuild macos-notification-state does nothing. so, my current solution is this:

	(
		cd node_modules/macos-notification-state
		../.bin/node-gyp rebuild --build-from-source --nodedir=/usr/include/electron/node_headers
	)

I'm gonna make a PR to the module

Huh, that's weird because we've added our own patch to that module to make sure it builds correctly:
"postinstall": "node scripts/patch_macos_notification_state.js && electron-builder install-app-deps"

Is your build for some reason not running npm run postinstall?

@selfisekai
Copy link
Author

yes, I routinely add --ignore-scripts to npm/yarn commands for electron app build scripts and then run npm rebuild for the needed dependencies, as the scripts often download binaries built for GNU libc which won't work on Alpine (electron, playwright, ...), and I use electron_tasje as a replacement for electron-builder to avoid, among others, the binary downloads, building dependencies with downloaded headers for a different electron version than in the repositories, to get the .desktop files generated (not available in electron-builder on target=dir), and so on.

@devinbinnie
Copy link
Member

Ahh that makes sense. I was planning on sending an update to the module at some point, but since we wanted to release sooner rather than later we opted to fix it ourselves.

@selfisekai
Copy link
Author

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

No branches or pull requests

3 participants