[BUG] 7.x custom npm install scripts don't appear to be respected in dependencies

[MylesBorins]

Current Behavior:

Running npm install omg-i-pass-with-install-param with npm v7.x passes.

Expected Behavior:

Running npm install omg-i-pass-with-install-param should fail, as it does on v6.x, as a custom install script is supposed to run that verifies a npm_config_extra_param ENV VAR has been set.

Steps To Reproduce:

$ npm install omg-i-pass-with-install-param

The above fails on 6.x and passes on 7.x

if you close the repo and run the install as outlined in #1650 you will see completely different behavior, as the install script is being respected.

Environment:

OS: MacOS 10.15.6
Node: 14.6.0
npm: 7.0.0-beta.2

[MylesBorins]

You can also see this behavior by running the command

$ npm i oh-hi

In npm v6.14.7 you see the following output

[test]$ npm i oh-hi
> oh-hi@1.0.4 install /private/var/folders/7t/thbq5c7x72q06b5c4c7dj6xc0000gn/T/test/node_modules/oh-hi
> echo oh hi

oh hi
+ oh-hi@1.0.4
updated 1 package and audited 1 package in 0.348s
found 0 vulnerabilities

In npm v7.0.0-beta.2 you see that the install script was never run

[test]$ npm i oh-hi

up to date, audited 1 package in 511ms

found 0 vulnerabilities

[ljharb]

Wasn't this an intentional change, to prevent people from showing ads to their consumers on install?

[MylesBorins]

@ljharb potentially... TBH I'm just documenting breakages that I'm noticing break expecting + observable behavior.

Honestly not allowing for a custom install script is a good thing... I just was not expecting this to break

[isaacs]

There's two bugs here, one real and one imaginary. We should fix them both.

The config param was not being set in the env as expected. That's the real bug.

Second, the oh-hi script is running, it's just not printing the output. Hiding install script output when it passes is 100% an intentional change, something that has been planned for years, and not a bug :) https://github.com/npm/rfcs/blob/latest/accepted/0022-quieter-install-scripts.md

I'm not sure why you're not seeing omg-i-pass-with-install-param install successfully on v7. Here's what I see:

$ npm i omg-i-pass-with-install-param
npm ERR! code 1
npm ERR! path /Users/isaacs/dev/npm/cli/foo/node_modules/omg-i-pass-with-install-param
npm ERR! command failed
npm ERR! command sh -c node test_args.js

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/isaacs/.npm/_logs/2020-08-11T00_53_56_359Z-debug.log

When I install oh-hi, it just shows this:

$ npm i oh-hi

removed 1 package, and audited 1 package in 609ms

found 0 vulnerabilities

Even with --loglevel=silly, it's not obvious that the script is being run, or exactly which script failed and how in the omg-i-pass-with-install-param failure. So one imagines that it didn't run, or failed for no reason. That bug needs to be fixed with better logging.

[MylesBorins]

@isaacs definitely seeing the install pass.

To reproduce install with npm v6 so you have a working package-lock.json and then attempt to install. My gut here is that the install script is either being ignore OR the exit code is being ignored when there is a package-lock. Interestingly enough simply running "npm install" will fail but explicitly installing the module will pass

Make a folder with the below files and run npm install omg-i-pass-with-install-param

package.json

{
  "name": "test",
  "version": "1.0.0",
  "dependencies": {
    "omg-i-pass-with-install-param": "^3.0.0"
  }
}

package-lock.json

{
  "name": "test",
  "version": "1.0.0",
  "lockfileVersion": 1,
  "requires": true,
  "dependencies": {
    "omg-i-pass-with-install-param": {
      "version": "3.0.0",
      "resolved": "https://registry.npmjs.org/omg-i-pass-with-install-param/-/omg-i-pass-with-install-param-3.0.0.tgz",
      "integrity": "sha1-DXOJkJQp+dp51HGiN8376v8akXw="
    }
  }
}

[isaacs]

From slack:

isaacs install scripts definitely need better logging, we're gonna see a
lot of noise about that, I think

that imaginary bug will multiply in the imagination of our users :lolsob:

mylesborins I'm just confused by the exit code not being respected

isaacs the exit code is being respected. if the install succeeds, it's
because it didn't run the script, or it didn't fail.

mylesborins so then it is strange that for the case with the package-lock
the script isn't being run

(which is the other possibility)

isaacs when you install wiht v7, having a v6 lockfile, is the node_modules
tree full?

mylesborins no fresh install

isaacs because it might just not be installing anything if so

oh, ok.

then it's because it's not refreshing the Node metadata.

mylesborins I included a package.json and pacakge-lock.json in the last
comment and with those two files in a fresh folder npm install should reproduce

isaacs thanks. i know where to go digging then.

when we have a lockfileVersion:1 lockfile, we don't have all the info we need,
and have to fill in the blanks. loadVirtual() will do that with the
package.json files on disk if present, but if they're not present, it's
supposed to refresh the data about any modules based on what it sees from the
registry. I'm guessing it's not doing that.

we might have to bite the bullet and have buildIdealTree fetch all packages in
a v1 lockfile if they're not present on disk

slow af for big projects, but a one-time hit, and will ensure correct behavior

and we have to get those packuments for reify anyway, so we can do it at the
same time.

[isaacs]

Fixed in latest v7 beta. Thanks!

[CalculonPrime]

There's two bugs here, one real and one imaginary. We should fix them both.

The config param was not being set in the env as expected. That's the real bug.

Second, the oh-hi script is running, it's just not printing the output. Hiding install script output when it passes is 100% an intentional change, something that has been planned for years, and not a bug :) https://github.com/npm/rfcs/blob/latest/accepted/0022-quieter-install-scripts.md

Our module not only prints output, it queries the user for configuration during installation, using readline(), and then downloads a third party library configured as requested. With npm v7, the install script isn't running at all, because if the output was just getting suppressed, it would hang the install (I'm guessing, but I see no evidence it tried to run.) This is disastrous. Our module now doesn't install properly.

So the behavior has been crippled because some bad actors were showing ads??? Is there a workaround for this so we don't have to tell the user to manually run another step?

[Cactusbone]

Our module not only prints output, it queries the user for configuration during installation, using readline(), and then downloads a third party library configured as requested. With npm v7, the install script isn't running at all, because if the output was just getting suppressed, it would hang the install (I'm guessing, but I see no evidence it tried to run.) This is disastrous. Our module now doesn't install properly.

You can try using the foreground-scripts option.