Validate scripts in 'bin', warn if they don't exist. #35

Closed
wants to merge 1 commit into from

3 participants

@nicks

As discussed in npm/npm#4668

@rlidwka rlidwka commented on an outdated diff Feb 18, 2014
@@ -318,6 +318,21 @@ function githead_ (file, data, dir, head, cb) {
})
}
+/**
+ * Warn if the bin references don't point to anything. This might be better in
+ * normalize-package-data if it had access to the file path.
+ */
+function checkBinReferences_ (file, data, warn) {
+ if (!(data.bin instanceof Object)) return
+
+ for (var key in data.bin) {
+ var binPath = path.resolve(path.dirname(file), data.bin[key])
+ if (!fs.existsSync(binPath)) {
@rlidwka
rlidwka added a note Feb 18, 2014

Using fs.exists is usually a bad idea because of race conditions. It might be fine in this case, but it's better to let chmod fail and handle the error instead.

Using fs.existsSync is always a bad idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nicks

@rlidwka good point, I updated the PR to use fs.exists

which chmod are you talking about? if you're talking about the one described in npm/npm#4668, that happens way too late: the script is half-way installed, and the game is already lost. This warning is for pre-publish.

@smikes

/cc @othiym23 this PR addresses npm/npm#4668 , for consideration in an upcoming npm minor?

@othiym23

Thanks for the pointer, @smikes! This has been rebased and landed as 35cef90. I'm in the process of incorporating it into npm right now. Thanks for putting this together, following @isaacs's crazy coding conventions, and your patience!

@othiym23 othiym23 closed this Feb 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment