kriszyp / msgpackr-extract

Node addon for string extraction for msgpackr
MIT License
7 stars 3 forks source link

Error running install script for optional dependency: No such file or directory #9

Closed mischnic closed 2 years ago

mischnic commented 2 years ago
warning Error running install script for optional dependency: "/Users/niklas/Desktop/y/node_modules/msgpackr-extract: Command failed.
Exit code: 127
Command: node-gyp-build-optional-packages
Arguments:
Directory: /Users/niklas/Desktop/y/node_modules/msgpackr-extract
Output:
env: node\\r: No such file or directory"

My lockfile contains this:

// ...the various arch-specific packages...

msgpackr-extract@^1.0.14:
  version "1.1.3"
  dependencies:
    node-gyp-build-optional-packages "^4.3.1"
  optionalDependencies:
    msgpackr-extract-darwin-arm64 "1.1.0"
    msgpackr-extract-darwin-x64 "1.1.0"
    msgpackr-extract-linux-arm "1.1.0"
    msgpackr-extract-linux-arm64 "1.1.0"
    msgpackr-extract-linux-x64 "1.1.0"
    msgpackr-extract-win32-x64 "1.1.0"

msgpackr@^1.5.4:
  version "1.5.5"
  optionalDependencies:
    msgpackr-extract "^1.0.14"

But node_modules only contains: msgpackr and msgpackr-extract-darwin-x64. I can't find msgpackr-extract in there.

Also, the repo contains 1.1.0 but the npm version is actually 1.1.3. Looks like you forgot to push the commits?

kriszyp commented 2 years ago

msgpackr-extract@v1.1.3 + msgpackr-extract-darwin-x64@v1.1.0 all looks correct to me. From what I can tell, there is a carriage return in one of the scripts for node-gyp-build-optional-packages (branch of node-gyp-build), which I guess a npm publish from my windows box must have introduced.

kriszyp commented 2 years ago

I think I fixed it by publishing node-gyp-build-optional-packages from a mac, but will do more testing tomorrow.

mischnic commented 2 years ago

I don't get an error anymore with the latest version of node-gyp-build-optional-packages 👍

kriszyp commented 2 years ago

Thank you for checking. I went ahead and published a v1.1.4 of msgpackr-extract that requires the 4.3.2 of node-gyp-build-optional-packages to make sure the fixed version is used.

(a little longer explanation: inspired by the parcel-css approach of using optionalDependencies for housing the platform-specific binaries for smaller install size, I have been working on upgrading the prebuildify/node-gyp-build system to support this approach and also intend to use this for lmdb-js, which hopefully will further reduce the overall install size of parcel and other dependents of lmdb-js. But here we are in the year 2022 where computers are drawing detailed images from a simple text prompt, but you can't publish an NPM package with a hashbang script from windows machine and have it work on a mac 🤦 )

mischnic commented 2 years ago

In your defense, I've had the exact same problem with another npm CLI utility a few days ago 😄

vladimiry commented 2 years ago

I wish msgpackr module would come with a major version bump after the msgpackr-extract => msgpackr-extract-<platform>-<arch> compiled native module placing location switch, since this looks like a breaking change. I was unpacking the module native stuff using **/node_modules/msgpackr-extract/**/* pattern but didn't update the pattern since didn't get alerted by a major version bump that this patter is no longer valid.

kriszyp commented 2 years ago

Sorry about that. I generally think of major versions for breaking change to the public API (whereas there was no docs specifying any guarantees of package contents/structures). But certainly would have done a major version if I had known there was code/builds depending on the package structures, my apologies.

vladimiry commented 2 years ago

Yes, I agree that normally API change triggers the major bump. I just got affected by the change and emotionally posted the message. Please don't consider the message as a complaint, and thanks for a well maintained module. In an ideal situation, I should have implemented tests for that pattern-based unpacking.

kriszyp commented 2 years ago

No problem, It is helpful understanding how changes can break things for users for the future!

kriszyp commented 2 years ago

@vladimiry what pattern are you currently using for copying the msgpackr-extract modules? Per the discussion/updates in https://github.com/prebuild/prebuildify/pull/63 , msgpackr-extract will move to using @msgpackr-extract/msgpackr-extract-platform-arch as the location of the binary builds. I'll do major version bump for msgpackr-extract, but not clear if that is needed for msgpackr (would prefer not to since this a pretty internal thing, but I could do it)

vladimiry commented 2 years ago

Thanks for the message. I was going to patch msgpackr-extract's entry point, so it's a direct native module load (same way it worked initially), https://github.com/vladimiry/ElectronMail/commit/1381ead1d29a7f27c589503b2c898c7317525995#diff-126da4a2f32a49bc0956068d3e51194f06ae41c15c6464509e519a264ba1ff78 I don't use preloads for this project. Due to the patching (content + package version), any version bump will hint to me that something got changed there (even if it's a transient dependency, not direct one), and so I will manually review the change. So version bump of any kind works for me.

kriszyp commented 2 years ago

Ah, ok, good to know, thank you. Let me know if there is anything I can do to help make that work smoothly for you. And by the way, are you using npm install --build-from-source or npm install --no-optional to omit (optional) prebuilds and/or force compilation?

vladimiry commented 2 years ago

Before triggering the compilation, I normally remove all the possibly existing *.node files, remove the ./node_modules/prebuild-install, and then trigger the build via the electron-builder's means. For testing purposes on Linux, I normally run the HOME=~/.electron-gyp node-gyp rebuild --target=v18.2.3 --arch=x64 --dist-url=https://electronjs.org/headers-like command inside the dependency folder. This project doesn't use npm but pnpm for installing the deps.