NpmToolResolver can hang waiting for input

[PaulKlein]

Usage Information

Nuke 8.0.0, .NET 8, Windows 11

Description

The NpmToolPathResolver calls npx which to locate the npm package. However, in some environments, npx will interactively prompt the user to confirm the which package installation. If which isn't already installed, the tool hangs as it waits for confirmation.

Npm Exec describes this behaviour (and why it only happens in some environments):

If any requested packages are not present in the local project dependencies, then a prompt is printed, which can be suppressed by providing either --yes or --no. When standard input is not a TTY or a CI environment is detected, --yes is assumed.

Reproduction Steps

I have a generated task definition that generates a task class:

[PublicAPI]
[ExcludeFromCodeCoverage]
[NpmPackageRequirement(CSpellPackageId)]
public partial class CSpellTasks
    : IRequireNpmPackage
{
    public const string CSpellPackageId = "cspell";
    /// <summary>
    ///   Path to the CSpell executable.
    /// </summary>
    public static string CSpellPath =>
        ToolPathResolver.TryGetEnvironmentExecutable("CSPELL_EXE") ??
        NpmToolPathResolver.GetNpmExecutable("cspell");

  // ...
}

And in my build script I am executing this:

Target RunCSpell => t => t
        .Requires<CSpellTasks>()
        .Executes(
        () =>
        {
            CSpellTasks.CSpellCheck();
        });

Running this build, the build hangs at the 'RunCSpell' step

Expected Behavior

The build should not hang

Actual Behavior

The build hangs at the 'RunCSpell' step. Looking in the Windows task manager I can see:

"C:\Program Files\nodejs\node.exe" "C:\Program Files\nodejs\node_modules\npm\bin\npx-cli.js" which cspell

Manually running npx which cspell, I get output like:

npx which cspell
Need to install the following packages:
[email protected]
Ok to proceed? (y)

Npx then sits indefinitely, waiting for user input.

Regression?

No response

Known Workarounds

You can manually run the command once, which adds which to the npx cache and avoids the hidden input prompt.

For a code fix, the NpmToolPathResolver should explicitly pass through --yes to avoid interactive input prompts during build.

Could you help with a pull-request?

Yes

[matkoch]

People can manually pass --yes if they want to. Passing it automatically is too opinionated. Some might use the infrastructure to build custom tools for local interactive usage.

[PaulKlein]

Hi @matkoch , I don't see how people can manually pass --yes, since it is NUKE itself that is running this npx which command.

Looking at the code in Nuke.Tooling/NpmToolPathResolver.cs it doesn't seem to accept any options or switches to let people control it, It also explicitly hides its output with logOutput: false so you don't see the prompt in the output.

If I'm missing something please let me know how I can pass yes myself if you don't want this as default.

[PaulKlein]

Hey @matkoch thanks for re-opening this for another look, and sorry if my initial issue description wasn't clear.

Are you waiting on anything from me here? I'm happy to write up a PR if that's useful, or provide any other info to help test this out.

Cheers

[matkoch]

Would it make sense to install which explicitly? I'm just wondering if --yes could potentially cause other packages to be installed unintentionally.

PS: your initial message was fine but I only skimmed through it and assumed something different. my fault.

[matkoch]

When standard input is not a TTY or a CI environment is detected, --yes is assumed.

It might make sense to work with a timeout on the npx which command instead. For local builds, the user would then know that which is missing (unsure though if the timeout exception shows the output). And on CI this doesn't seem to be an issue.

[PaulKlein]

I'm not sure sorry. It seems like installing it explicitly would be nice, if this is practical.

I see that ToolRequirementService.InstallNpmPackages() already builds a package.json and runs npm install, so possibly this would be a good place for it?

A timeout seems like it could work too. In CI case where it does auto-install the package, this could take an arbitrary amount of time to download+install, although the which package is tiny at ~7.5kb so I doubt this is an issue in practice.

The timeout option does seem like the simplest fix for this admittedly obscure case.

I did take a quick look at passing through --no instead, in non-CI environment - this would then fail immediately, though npx doesn't give good output in this case either unfortunately:

npx --no which cspell
npm ERR! canceled

npm ERR! A complete log of this run can be found in:
npm ERR! C:\Users\paul\AppData\Local\npm-cache_logs\2024-04-05T03_57_31_747Z-debug-0.log

The mentioned logfile is no clearer either.

And considering passing --yes, I think since we're not displaying input/output here, we are effectively in a non-TTY environment anyway, even though npm isn't picking up on this. But the npx documentation isn't exactly clear on what is auto-installed, so I understand the hesitation to pass yes in all cases.