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

fswin fails to build on Linux or MacOS #26

Closed
aminya opened this issue Dec 10, 2020 · 8 comments
Closed

fswin fails to build on Linux or MacOS #26

aminya opened this issue Dec 10, 2020 · 8 comments

Comments

@aminya
Copy link

aminya commented Dec 10, 2020

Shouldn't node-fswin generate a dummy .node file so it can be required from different operating systems?

Linux:

[Error: Error: Not a valid ELF file: /home/vsts/work/1/s/out/app/node_modules/fswin/node/x64/fswin.node

MacOS

[Error: Error: /Users/runner/work/1/s/out/app/node_modules/fswin/node/x64/fswin.node: file is neither a fat binary file nor a Mach-O object file
@xxoo
Copy link
Owner

xxoo commented Dec 11, 2020

the npm package is using a index.js to detect os, that makes require('fswin') avoids loading fswin.node when process.platform is not win32.
could you supply some code to reproduce the error?

% node -e "console.log(require('fswin'))"
null

@aminya
Copy link
Author

aminya commented Dec 16, 2020

@xxoo Did this package had this issue in older versions?

See the CI errors in
https://github.visualstudio.com/Atom/_build/results?buildId=92731&view=logs&j=e2cf4b02-5697-54ad-cf7c-fc2a840d53af&t=9fd275d3-5d0a-5af8-58ae-a01a7d61dbd5&l=39

atom/atom#21777

I am updating fswin just in case if that is solved in the newer versions.

@xxoo
Copy link
Owner

xxoo commented Dec 16, 2020

@aminya i don't think so. index.js has been unchanged for over 2 years.
the log says it was trying to load fswin/node/ia32/fswin.node. which means process.platform is win32, process.arch is ia32 and process.versions.electron is equal to false. however the script was definitely not running on windows. in my opinion, that just doesn't make sense. there must be something wrong with the environment.
this is the source code of index.js

'use strict';
if (process.platform === 'win32') {
	module.exports = require((process.versions && process.versions.electron ? './electron/' : './node/') + process.arch + '/fswin.node');
} else {
	module.exports = null;
}

@aminya
Copy link
Author

aminya commented Dec 16, 2020

winattr package is out of date. I have fixed it in this PR. It is published in my @aminya/winattr package.

It was using "fswin": "^3.18.918".

@xxoo
Copy link
Owner

xxoo commented Dec 16, 2020

does it solve the problem? if so i'd like to close this issue

@aminya
Copy link
Author

aminya commented Dec 16, 2020

It takes some time until I can give you a solid answer.

This PR should be merged, and then it should be merged to Atom itself.
atom/text-buffer#336

@aminya
Copy link
Author

aminya commented Dec 17, 2020

We still hit the error atom/text-buffer#336 (comment)

The error is thrown from node-minidump package.

@xxoo
Copy link
Owner

xxoo commented Dec 19, 2020

i see. seems there's nothing i can do for that...

@xxoo xxoo closed this as completed Dec 19, 2020
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

2 participants