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.

Sign up for GitHub

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

Jump to bottom

fix: fix the preinstall function logic #55

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+21 −26
Open

fix: fix the preinstall function logic #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3fe1454
fix: fix the preinstall function logic
aminya Nov 18, 2022
40e26bf
test: run standard --fix
aminya Nov 18, 2022
2c0dbc1
fix: remove the rest of process.argv executions
aminya Nov 27, 2022
9bcfec9
fix: fix spawning node-gyp-build-test
aminya Nov 27, 2022
4a173b4
fix: use absolute paths for the scripts
aminya Nov 27, 2022
cfb83ce
fix: add logging for the build-test execution
aminya Nov 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* text eol=lf
33 changes: 9 additions & 24 deletions bin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ var os = require('os')
var path = require('path')

if (!buildFromSource()) {
proc.exec('node-gyp-build-test', function (err, stdout, stderr) {
proc.exec(process.execPath, [path.join(__dirname, 'build-test.js')], function (err, stdout, stderr) {
console.log(stdout)
if (err) {
if (verbose()) console.error(stderr)
preinstall()
Expand All @@ -30,35 +31,19 @@ function build () {

proc.spawn(args[0], args.slice(1), { stdio: 'inherit', shell: win32, windowsHide: true }).on('exit', function (code) {
if (code || !process.argv[3]) process.exit(code)
exec(process.argv[3]).on('exit', function (code) {
process.exit(code)
})
})
}

function preinstall () {
if (!process.argv[2]) return build()
exec(process.argv[2]).on('exit', function (code) {
if (code) process.exit(code)
try {
// try to load the prebuild
const load = require(path.join(__dirname, 'index.js'))
load()
} catch (err) {
// report the error and fall to a build
console.error(err.message)
Copy link

Choose a reason for hiding this comment

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

I don't think this should be an error. That would imply the consuming package has done something wrong or has an unexpected condition to recover from.

More likely in my experience, the package was simply not built.

build()
})
}

function exec (cmd) {
if (process.platform !== 'win32') {
var shell = os.platform() === 'android' ? 'sh' : true
return proc.spawn(cmd, [], {
shell,
stdio: 'inherit'
})
}

return proc.spawn(cmd, [], {
windowsVerbatimArguments: true,
stdio: 'inherit',
shell: true,
windowsHide: true
})
}

function buildFromSource () {
Expand Down
13 changes: 11 additions & 2 deletions build-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
process.env.NODE_ENV = 'test'

var path = require('path')

// find the test command from package.json prebuild.test entry
var test = null

try {
Expand All @@ -15,5 +17,12 @@ try {
// do nothing
}

if (test) require(path.join(process.cwd(), test))
else require('./')()
if (test) {
const testPath = path.join(process.cwd(), test)
console.log(`Running require("${testPath}")`)
require(testPath)
}
else {
console.log(`Running require("./")() at ${process.cwd()}`)
require('./')()
}