fix(install): deduplicate minimum release age approval output (#15090)
Resolution can return the same immature package version in multiple dependency contexts. Filter those records by name and version in the minimumReleaseAge handler, keeping distinct versions and the existing sorting and approval behavior. Dialoguer clears only the current line before rendering the selected answer. Give it only the approval question and write the preceding version list once while the reporter is paused. Preserve prompt errors, default denial, and exclude persistence. Cover duplicated records in approval and non-interactive tests in both CLI versions. Both approval regressions retain multiple versions of one package. A Unix pseudo-terminal CLI regression checks that answering the real prompt prints the version list once and persists the approved exclusions. Closes pnpm/pnpm#15083.
This commit is contained in:
1 parent
8cdc670197
commit
6ea6aaf72c
8 files changed
+179
-5
No files matched your search
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@pnpm/installing.commands": patch
|
||||
"pnpm": patch
|
||||
"pacquet": patch
|
||||
---
|
||||
|
||||
The `minimumReleaseAge` approval prompt now counts and displays each package version once [pnpm/pnpm#15083](https://github.com/pnpm/pnpm/issues/15083).
|
||||
+45
@@ -0,0 +1,45 @@
|
||||
import errno
|
||||
import os
|
||||
import pty
|
||||
import select
|
||||
import subprocess
|
||||
import sys
|
||||
import time
|
||||
|
||||
|
||||
master, slave = pty.openpty()
|
||||
process = subprocess.Popen(sys.argv[1:], stdin=slave, stdout=slave, stderr=slave)
|
||||
os.close(slave)
|
||||
output = bytearray()
|
||||
approved = False
|
||||
deadline = time.monotonic() + 60
|
||||
|
||||
try:
|
||||
while time.monotonic() < deadline:
|
||||
if not select.select([master], [], [], 0.1)[0]:
|
||||
if process.poll() is not None:
|
||||
break
|
||||
continue
|
||||
try:
|
||||
chunk = os.read(master, 65536)
|
||||
except OSError as error:
|
||||
if error.errno == errno.EIO:
|
||||
break
|
||||
raise
|
||||
if not chunk:
|
||||
break
|
||||
output.extend(chunk)
|
||||
if not approved and b"proceed with the install?" in output:
|
||||
os.write(master, b"y")
|
||||
approved = True
|
||||
else:
|
||||
raise TimeoutError("interactive install did not finish within 60 seconds")
|
||||
process.wait(timeout=5)
|
||||
finally:
|
||||
if process.poll() is None:
|
||||
process.kill()
|
||||
process.wait()
|
||||
os.close(master)
|
||||
sys.stdout.buffer.write(output)
|
||||
|
||||
sys.exit(process.returncode)
|
||||
@@ -82,6 +82,7 @@ mod lockfile_resolution_reuse;
|
||||
mod lockfile_verification;
|
||||
mod login;
|
||||
mod logout;
|
||||
mod minimum_release_age;
|
||||
mod multiple_importers;
|
||||
mod named_registry_install;
|
||||
mod nested_file_dependencies;
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
#![cfg(unix)]
|
||||
|
||||
use crate::_utils::{append_workspace_yaml_key, set_minimum_release_age, without_colors};
|
||||
use pnpm_config::WorkspaceSettings;
|
||||
use pnpm_testing_utils::{bin::CommandTempCwd, command_env::CommandTestExt};
|
||||
use std::{fs, process::Command};
|
||||
|
||||
#[test]
|
||||
fn approval_prints_the_version_list_once_and_persists_excludes() {
|
||||
let CommandTempCwd {
|
||||
pacquet,
|
||||
root,
|
||||
workspace,
|
||||
npmrc_info,
|
||||
..
|
||||
} = CommandTempCwd::init().add_mocked_registry();
|
||||
fs::write(workspace.join("package.json"), r#"{"name":"approval-test","version":"1.0.0"}"#)
|
||||
.expect("write package.json");
|
||||
set_minimum_release_age(&workspace, 60 * 24 * 365 * 100);
|
||||
append_workspace_yaml_key(&workspace, "minimumReleaseAgeStrict", true);
|
||||
|
||||
let output = without_colors(Command::new("python3").without_ambient_pnpm_config())
|
||||
.env("CI", "false")
|
||||
.env_remove("GITHUB_ACTION")
|
||||
.arg("-c")
|
||||
.arg(include_str!("../fixtures/minimum_release_age_prompt.py"))
|
||||
.arg(pacquet.get_program())
|
||||
.args([
|
||||
"add",
|
||||
"@pnpm.e2e/bravo-dep@1.0.0",
|
||||
"@pnpm.e2e/hello-world-js-bin@1.0.0",
|
||||
"--reporter=append-only",
|
||||
"--ignore-scripts",
|
||||
])
|
||||
.current_dir(&workspace)
|
||||
.output()
|
||||
.expect("run interactive install in a pseudo-terminal");
|
||||
let stdout = String::from_utf8(output.stdout).expect("terminal output is UTF-8");
|
||||
eprintln!("{stdout}");
|
||||
assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr));
|
||||
assert_eq!(
|
||||
stdout.matches("2 versions do not meet the minimumReleaseAge constraint:").count(),
|
||||
1,
|
||||
);
|
||||
|
||||
let question =
|
||||
"Add to minimumReleaseAgeExclude in pnpm-workspace.yaml and proceed with the install?";
|
||||
let prompt_output = &stdout[..stdout.rfind(question).expect("approval question is rendered")];
|
||||
let versions = ["@pnpm.e2e/bravo-dep@1.0.0", "@pnpm.e2e/hello-world-js-bin@1.0.0"];
|
||||
for version in versions {
|
||||
assert_eq!(
|
||||
prompt_output
|
||||
.lines()
|
||||
.filter(|line| line.trim() == version)
|
||||
.count(),
|
||||
1,
|
||||
"{version}",
|
||||
);
|
||||
}
|
||||
let settings = WorkspaceSettings::load_at(&workspace)
|
||||
.expect("read workspace manifest")
|
||||
.expect("workspace manifest exists");
|
||||
assert_eq!(settings.minimum_release_age_exclude.unwrap(), versions);
|
||||
|
||||
drop((root, npmrc_info));
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
use std::{marker::PhantomData, path::Path};
|
||||
use std::{io::Write, marker::PhantomData, path::Path};
|
||||
|
||||
use derive_more::{Display, Error};
|
||||
use miette::Diagnostic;
|
||||
@@ -112,8 +112,12 @@ impl ApprovalPrompt for DialoguerPrompt {
|
||||
async fn confirm(&mut self, message: &str) -> dialoguer::Result<bool> {
|
||||
let message = message.to_owned();
|
||||
tokio::task::spawn_blocking(move || {
|
||||
let (list, question) =
|
||||
message.rsplit_once('\n').expect("approval question follows the version list");
|
||||
// Dialoguer only clears the last line when it renders the answer.
|
||||
writeln!(std::io::stderr(), "{list}")?;
|
||||
dialoguer::Confirm::new()
|
||||
.with_prompt(message)
|
||||
.with_prompt(question)
|
||||
.default(false)
|
||||
.interact()
|
||||
})
|
||||
@@ -220,6 +224,7 @@ fn sorted_immature_violations(
|
||||
.filter(|violation| violation.code == MINIMUM_RELEASE_AGE_VIOLATION_CODE)
|
||||
.collect();
|
||||
immature.sort_by_cached_key(|violation| format!("{}@{}", violation.name, violation.version));
|
||||
immature.dedup_by(|left, right| left.name == right.name && left.version == right.version);
|
||||
immature
|
||||
}
|
||||
|
||||
|
||||
@@ -177,6 +177,7 @@ async fn non_interactive_strict_mode_reports_every_immature_pick() {
|
||||
config.minimum_release_age_strict = Some(true);
|
||||
let mut prompt = FakePrompt::default();
|
||||
let violations = vec![
|
||||
violation("zeta", "2.0.0", "MINIMUM_RELEASE_AGE_VIOLATION"),
|
||||
violation("zeta", "2.0.0", "MINIMUM_RELEASE_AGE_VIOLATION"),
|
||||
violation("alpha", "1.0.0", "MINIMUM_RELEASE_AGE_VIOLATION"),
|
||||
violation("ignored", "3.0.0", "TRUST_DOWNGRADE"),
|
||||
@@ -218,6 +219,8 @@ async fn approval_persists_canonical_excludes_and_brackets_the_prompt() {
|
||||
let violations = vec![
|
||||
violation("foo", "2.0.0", "MINIMUM_RELEASE_AGE_VIOLATION"),
|
||||
violation("bar", "3.0.0", "MINIMUM_RELEASE_AGE_VIOLATION"),
|
||||
violation("foo", "1.0.0", "MINIMUM_RELEASE_AGE_VIOLATION"),
|
||||
violation("foo", "2.0.0", "MINIMUM_RELEASE_AGE_VIOLATION"),
|
||||
];
|
||||
|
||||
handle_minimum_release_age_violations_with::<RecordingReporter, _>(
|
||||
@@ -232,7 +235,10 @@ async fn approval_persists_canonical_excludes_and_brackets_the_prompt() {
|
||||
.expect("approval should continue");
|
||||
|
||||
assert_eq!(prompt.messages.len(), 1);
|
||||
assert!(prompt.messages[0].contains("bar@3.0.0\n foo@2.0.0"));
|
||||
assert_eq!(
|
||||
prompt.messages[0],
|
||||
"3 versions do not meet the minimumReleaseAge constraint:\n bar@3.0.0\n foo@1.0.0\n foo@2.0.0\nAdd to minimumReleaseAgeExclude in pnpm-workspace.yaml and proceed with the install?",
|
||||
);
|
||||
let workspace = fs::read_to_string(dir.path().join("pnpm-workspace.yaml"))
|
||||
.expect("read workspace manifest");
|
||||
assert!(workspace.contains("packages:\n - packages/*"));
|
||||
|
||||
@@ -199,7 +199,14 @@ function createMinimumReleaseAgeHandler (opts: PolicyHandlersOptions): PolicyHan
|
||||
}
|
||||
|
||||
function filterImmatureViolations (violations: readonly PolicyViolation[]): PolicyViolation[] {
|
||||
return violations.filter((v) => v.code === MINIMUM_RELEASE_AGE_VIOLATION_CODE)
|
||||
const seen = new Set<string>()
|
||||
return violations.filter((v) => {
|
||||
if (v.code !== MINIMUM_RELEASE_AGE_VIOLATION_CODE) return false
|
||||
const key = `${v.name}@${v.version}`
|
||||
if (seen.has(key)) return false
|
||||
seen.add(key)
|
||||
return true
|
||||
})
|
||||
}
|
||||
|
||||
function pickImmatureEntries (
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { expect, jest, test } from '@jest/globals'
|
||||
|
||||
import { type PolicyViolation, setupPolicyHandlers } from '../lib/policyHandlers.js'
|
||||
import type { PolicyViolation } from '../lib/policyHandlers.js'
|
||||
|
||||
const confirm = jest.fn<(options: { message: string, default: boolean }) => Promise<boolean>>()
|
||||
jest.unstable_mockModule('@inquirer/prompts', () => ({ confirm }))
|
||||
const { setupPolicyHandlers } = await import('../lib/policyHandlers.js')
|
||||
|
||||
function violation (
|
||||
name: string,
|
||||
@@ -179,3 +183,36 @@ test('the hook is a no-op in loose mode regardless of violations', async () => {
|
||||
await expect(plan.handleResolutionPolicyViolations([violation('foo', '1.0.0')]))
|
||||
.resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
test('strict no-TTY errors count unique package versions', async () => {
|
||||
await withStdinTTY(false, async () => {
|
||||
const plan = setupPolicyHandlers({ minimumReleaseAge: 60, minimumReleaseAgeStrict: true, ci: false })!
|
||||
await expect(plan.handleResolutionPolicyViolations([
|
||||
violation('foo', '1.0.0'),
|
||||
violation('foo', '2.0.0'),
|
||||
violation('foo', '1.0.0'),
|
||||
violation('bar', '1.0.0'),
|
||||
])).rejects.toMatchObject({
|
||||
message: '3 versions do not meet the minimumReleaseAge constraint:\n bar@1.0.0 stub reason\n foo@1.0.0 stub reason\n foo@2.0.0 stub reason',
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
test('approval prompts list each package version once', async () => {
|
||||
confirm.mockResolvedValueOnce(true)
|
||||
await withStdinTTY(true, async () => {
|
||||
const plan = setupPolicyHandlers({ minimumReleaseAge: 60, minimumReleaseAgeStrict: true, ci: false })!
|
||||
await plan.handleResolutionPolicyViolations([
|
||||
violation('foo', '1.0.0'),
|
||||
violation('bar', '1.0.0'),
|
||||
violation('foo', '2.0.0'),
|
||||
violation('foo', '1.0.0'),
|
||||
violation('ignored', '1.0.0', 'TRUST_DOWNGRADE'),
|
||||
])
|
||||
expect(confirm).toHaveBeenCalledTimes(1)
|
||||
expect(confirm).toHaveBeenCalledWith({
|
||||
message: '3 versions do not meet the minimumReleaseAge constraint:\n bar@1.0.0\n foo@1.0.0\n foo@2.0.0\nAdd to minimumReleaseAgeExclude in pnpm-workspace.yaml and proceed with the install?',
|
||||
default: false,
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in new issue
Block a user