[FIX] WIndows binary #671
[FIX] WIndows binary #671
Conversation
package.json
Outdated
| ] | ||
| } | ||
| } | ||
| "name": "node-jq", |
There was a problem hiding this comment.
Could you revert this file changes?
package.json
Outdated
| "url": "https://github.com/sanack/node-jq" | ||
| }, | ||
| "bin": { | ||
| "node-jq": "bin/jq.exe" |
There was a problem hiding this comment.
That's rather annoying to add here, since it will break linux-based OS. Please remove this diff
scripts/install-binary.js
Outdated
| const OUTPUT_DIR = path.join(__dirname, '..', 'bin') | ||
| const OUTPUT_FILE = path.join(OUTPUT_DIR, JQ_NAME) | ||
|
|
||
| const renamePackageBin = () => { |
There was a problem hiding this comment.
Can you rename this function to "makeBinaryWorkInWindows"? and add comments on all things happening here? Replace \ with / and also implement the append of ".exe"?
There was a problem hiding this comment.
I resolved:
- rename renamePackageBin to makeBinaryWorkInWindows
- replace / by \ (this was for dev proposes)
I do not have access to your CI or I cannot see the CI, can you please me prompt me the error if you have ? |
… update by running "npm link"
…aded (to avoid circular calls)
|
Hi @davesnx ! After some tests, I found out that when you reinstall the dependency, the binary file is welly symbolinked. It's look like the binary symbolink are created before the preinstall hooks (so if the file is not already downloaded, the symbolink is ignored.) So I force it by running "npm link ." in the package folder after the file download (take a look at this commit). Note that I also change the test unit: I had to replace "null" with "undefined" in some places. |
I encountered issues when running the
node-jqbinary from the package. I discovered that if you don't append.exeto thebinpackage file path, the binary isn't added/symlinked in thenode_module/.bin/repository, resulting in the binary not being resolved.In response to this issue, I've added the
renamePackageBinfunction, which forces to assign the full filename as the binary file in thepackage.json, resolving the problem.I've executed this function in all OS contexts, which should work, but we can consider guarding it for Windows only (when the filename ends by
.exe, for example).