mixmaxhq / install-files

Share files like you would code, using npm.
MIT License
3 stars 2 forks source link

Ensure install/postinstall does not run on self. #9

Closed GoGoCarl closed 7 years ago

GoGoCarl commented 7 years ago

The intention here is to avoid error messages like the following (particularly npm v3, but I'd imagine a similar issue with any version):

&> npm install

> my-ebextension@1.0.8 install /path/to/my-ebextension
> install-files shared

[ { Error: ENOENT: no such file or directory, lstat '/path/to/my-ebextension/node_modules/my-ebextension/shared'
      at Error (native)
    errno: -2,
    code: 'ENOENT',
    syscall: 'lstat',
    path: '/path/to/my-ebextension/node_modules/my-ebextension/shared' } ]
└── install-files@1.1.1 

The interesting bit of my-ebextension's package.json looks like:

"scripts": {
    "install": "install-files shared",
}

which, of course, is required for my-microservice. But there are also other dependencies declared, unrelated to install-files, that require npm install to run, as my-ebextension provides additional code and services to my-microservice.

In this case, the failing code path is skipped because either the module name found in /path/to/my-ebextension/package.json or, that failing, the path.basename of /path/to/my-ebextension is the same as the package name, my-ebextension. I moved away from the regex as the filter would have erroneously caught things like foo-my-ebextension.

Now, I am no longer seeing the above error, and installing package name can be sought more reliably to avoid self-targeting.

GoGoCarl commented 7 years ago

I don't have NPMv2, but I can grab it to test on there at some point unless someone beats me to it.

GoGoCarl commented 7 years ago

Tested this on NPMv2, and this use case is caught by the check for target. In this case, target would end up being null because fileInstallingPackagePath is, of course, /path/to/my-ebextension, which does not include the node_modules token. Therefore, the ensuing call to hostPackageDir returns null. So, the installer halts with [Error: Could not determine the install destination directory.].

So, good to go for NPMv2.

Also, two related things that came up when checking this. First, just to note, with NPMv3, the target path ends up being /path/to/my-ebextension, which, by happenstance, is the same as fileInstallingPackagePath, which was why that was used in 1.1.0. target is desired, though. Second, there really isn't a check to see whether the source dir actually exist. Is that something anyone cares about, or just let it fail? In this case here, the source dir obviously ends up being a non-existent directory, but the same could happen if someone misspells something in scripts or perhaps a directory is moved or renamed.

wearhere commented 7 years ago

Thanks @GoGoCarl! Sorry for the delayed response. Looks good to me!

there really isn't a check to see whether the source dir actually exist. Is that something anyone cares about, or just let it fail? In this case here, the source dir obviously ends up being a non-existent directory, but the same could happen if someone misspells something in scripts or perhaps a directory is moved or renamed.

I'd imagine ncp returns an ENOENT error, right? I think that's fine.

wearhere commented 7 years ago

Published as 1.1.3.

wearhere commented 7 years ago

So did this fix #8 @GoGoCarl ?