From 0147dae9fe2a863105596ae81c61078bde6cb8af Mon Sep 17 00:00:00 2001 From: Dan Wilhelm Date: Thu, 10 Jan 2019 23:10:13 -0800 Subject: [PATCH] login: Fix empty input prompt Issue #484. PR392 inadvertantly replaced the `login` interactive process spawner with `child::run`, which is hard-coded to buffer stdout/stderr. This caused `login` to become essentially unusable; the user could no longer see interactive input prompts or error messages displayed by `npm adduser`. The code was not directly reverted because the previous version: 1. Returned Error instead of failure::Error. (Updated to use `bail!`, which is consistent with `publish`.) 2. Displayed all stderr only upon exit, rather than interactively displaying it. This led to repeated interactive prompts without informing the user why. (Updated to use `status()` which inherits stdin/stdout/stderr by default.) 3. Did not provide logging. (Now duplicates the logging in `child::run`.) --- src/npm.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/npm.rs b/src/npm.rs index 76fe841..4f7b18a 100644 --- a/src/npm.rs +++ b/src/npm.rs @@ -62,12 +62,14 @@ pub fn npm_login( args.push_str(&format!(" --auth_type={}", auth_type)); } + // Interactively ask user for npm login info. + // (child::run does not support interactive input) let mut cmd = Command::new("npm"); - cmd.arg("login") - .arg(args) - .stdin(Stdio::inherit()) - .stdout(Stdio::inherit()); - child::run(log, cmd, "npm login") - .with_context(|_| format!("Login to registry {} failed", registry))?; - Ok(()) + cmd.arg("login").arg(args); + + info!(log, "Running {:?}", cmd); + match cmd.status()?.success() { + true => Ok(()), + false => bail!("Login to registry {} failed", registry), + } }