From 914b7495a854981a3671e24f527d792f2ad1b324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 7 Mar 2024 21:13:36 +0000 Subject: [PATCH] fix(publish): reland error if there are uncommitted changes (#22613) (#22632) Reverted in https://github.com/denoland/deno/pull/22625 --- cli/args/flags.rs | 9 ++++ cli/tools/registry/mod.rs | 40 ++++++++++++++ tests/integration/publish_tests.rs | 85 +++++++++++++++++++++++++++++- tests/util/server/src/lib.rs | 17 +++++- 4 files changed, 148 insertions(+), 3 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 05d9a39732..26e0b089a7 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -307,6 +307,7 @@ pub struct PublishFlags { pub token: Option, pub dry_run: bool, pub allow_slow_types: bool, + pub allow_dirty: bool, pub no_provenance: bool, } @@ -2436,6 +2437,11 @@ fn publish_subcommand() -> Command { .action(ArgAction::SetTrue), ) .arg( + Arg::new("allow-dirty") + .long("allow-dirty") + .help("Allow publishing if the repository has uncommited changed") + .action(ArgAction::SetTrue), + ).arg( Arg::new("no-provenance") .long("no-provenance") .help("Disable provenance attestation. Enabled by default on Github actions, publicly links the package to where it was built and published from.") @@ -3897,6 +3903,7 @@ fn publish_parse(flags: &mut Flags, matches: &mut ArgMatches) { token: matches.remove_one("token"), dry_run: matches.get_flag("dry-run"), allow_slow_types: matches.get_flag("allow-slow-types"), + allow_dirty: matches.get_flag("allow-dirty"), no_provenance: matches.get_flag("no-provenance"), }); } @@ -8620,6 +8627,7 @@ mod tests { "--no-provenance", "--dry-run", "--allow-slow-types", + "--allow-dirty", "--token=asdf", ]); assert_eq!( @@ -8629,6 +8637,7 @@ mod tests { token: Some("asdf".to_string()), dry_run: true, allow_slow_types: true, + allow_dirty: true, no_provenance: true, }), type_check_mode: TypeCheckMode::Local, diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index faea95a1ab..f8c79d85c5 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -2,6 +2,8 @@ use std::collections::HashMap; use std::io::IsTerminal; +use std::path::Path; +use std::process::Stdio; use std::rc::Rc; use std::sync::Arc; @@ -24,6 +26,7 @@ use lsp_types::Url; use serde::Deserialize; use serde::Serialize; use sha2::Digest; +use tokio::process::Command; use crate::args::jsr_api_url; use crate::args::jsr_url; @@ -943,6 +946,15 @@ pub async fn publish( return Ok(()); } + if std::env::var("DENO_TESTING_DISABLE_GIT_CHECK") + .ok() + .is_none() + && !publish_flags.allow_dirty + && check_if_git_repo_dirty(cli_options.initial_cwd()).await + { + bail!("Aborting due to uncomitted changes",); + } + perform_publish( cli_factory.http_client(), prepared_data.publish_order_graph, @@ -1022,6 +1034,34 @@ fn verify_version_manifest( Ok(()) } +async fn check_if_git_repo_dirty(cwd: &Path) -> bool { + let bin_name = if cfg!(windows) { "git.exe" } else { "git" }; + + // Check if git exists + let git_exists = Command::new(bin_name) + .arg("--version") + .stderr(Stdio::null()) + .stdout(Stdio::null()) + .status() + .await + .map_or(false, |status| status.success()); + + if !git_exists { + return false; // Git is not installed + } + + // Check if there are uncommitted changes + let output = Command::new(bin_name) + .current_dir(cwd) + .args(["status", "--porcelain"]) + .output() + .await + .expect("Failed to execute command"); + + let output_str = String::from_utf8_lossy(&output.stdout); + !output_str.trim().is_empty() +} + #[cfg(test)] mod tests { use super::tar::PublishableTarball; diff --git a/tests/integration/publish_tests.rs b/tests/integration/publish_tests.rs index cb1072bc09..2c3bf9ff68 100644 --- a/tests/integration/publish_tests.rs +++ b/tests/integration/publish_tests.rs @@ -1,11 +1,14 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use std::process::Command; + use deno_core::serde_json::json; use test_util::assert_contains; use test_util::assert_not_contains; use test_util::env_vars_for_jsr_npm_tests; use test_util::env_vars_for_jsr_provenance_tests; use test_util::env_vars_for_jsr_tests; +use test_util::env_vars_for_jsr_tests_with_git_check; use test_util::env_vars_for_npm_tests; use test_util::itest; use test_util::TestContextBuilder; @@ -14,6 +17,7 @@ itest!(no_token { args: "publish", cwd: Some("publish/missing_deno_json"), output: "publish/no_token.out", + envs: env_vars_for_jsr_tests(), exit_code: 1, }); @@ -21,6 +25,7 @@ itest!(missing_deno_json { args: "publish --token 'sadfasdf'", output: "publish/missing_deno_json.out", cwd: Some("publish/missing_deno_json"), + envs: env_vars_for_jsr_tests(), exit_code: 1, }); @@ -28,6 +33,7 @@ itest!(has_slow_types { args: "publish --token 'sadfasdf'", output: "publish/has_slow_types.out", cwd: Some("publish/has_slow_types"), + envs: env_vars_for_jsr_tests(), exit_code: 1, }); @@ -44,6 +50,7 @@ itest!(invalid_path { args: "publish --token 'sadfasdf'", output: "publish/invalid_path.out", cwd: Some("publish/invalid_path"), + envs: env_vars_for_jsr_tests(), exit_code: 1, }); @@ -51,6 +58,7 @@ itest!(symlink { args: "publish --token 'sadfasdf' --dry-run", output: "publish/symlink.out", cwd: Some("publish/symlink"), + envs: env_vars_for_jsr_tests(), exit_code: 0, }); @@ -58,7 +66,7 @@ itest!(invalid_import { args: "publish --token 'sadfasdf' --dry-run", output: "publish/invalid_import.out", cwd: Some("publish/invalid_import"), - envs: env_vars_for_npm_tests(), + envs: env_vars_for_jsr_npm_tests(), exit_code: 1, http_server: true, }); @@ -67,7 +75,7 @@ itest!(invalid_import_esm_sh_suggestion { args: "publish --token 'sadfasdf' --dry-run", output: "publish/invalid_import_esm_sh_suggestion.out", cwd: Some("publish/invalid_import_esm_sh_suggestion"), - envs: env_vars_for_npm_tests(), + envs: env_vars_for_jsr_npm_tests(), exit_code: 1, http_server: true, }); @@ -488,3 +496,76 @@ fn publish_context_builder() -> TestContextBuilder { .envs(env_vars_for_jsr_tests()) .use_temp_cwd() } + +fn publish_context_builder_with_git_checks() -> TestContextBuilder { + TestContextBuilder::new() + .use_http_server() + .envs(env_vars_for_jsr_tests_with_git_check()) + .use_temp_cwd() +} + +#[test] +fn allow_dirty() { + let context = publish_context_builder_with_git_checks().build(); + let temp_dir = context.temp_dir().path(); + temp_dir.join("deno.json").write_json(&json!({ + "name": "@foo/bar", + "version": "1.0.0", + "exports": "./main.ts", + })); + + temp_dir.join("main.ts").write(""); + + let cmd = Command::new("git") + .arg("init") + .arg(temp_dir.as_path()) + .output() + .unwrap(); + assert!(cmd.status.success()); + + let output = context + .new_command() + .arg("publish") + .arg("--token") + .arg("sadfasdf") + .run(); + output.assert_exit_code(1); + let output = output.combined_output(); + assert_contains!(output, "Aborting due to uncomitted changes"); + + let output = context + .new_command() + .arg("publish") + .arg("--allow-dirty") + .arg("--token") + .arg("sadfasdf") + .run(); + output.assert_exit_code(0); + let output = output.combined_output(); + assert_contains!(output, "Successfully published"); +} + +#[test] +fn allow_dirty_not_in_repo() { + let context = publish_context_builder_with_git_checks().build(); + let temp_dir = context.temp_dir().path(); + temp_dir.join("deno.json").write_json(&json!({ + "name": "@foo/bar", + "version": "1.0.0", + "exports": "./main.ts", + })); + + temp_dir.join("main.ts").write(""); + // At this point there are untracked files, but we're not in Git repo, + // so we should be able to publish successfully. + + let output = context + .new_command() + .arg("publish") + .arg("--token") + .arg("sadfasdf") + .run(); + output.assert_exit_code(0); + let output = output.combined_output(); + assert_contains!(output, "Successfully published"); +} diff --git a/tests/util/server/src/lib.rs b/tests/util/server/src/lib.rs index e06ba2b399..debe2b6980 100644 --- a/tests/util/server/src/lib.rs +++ b/tests/util/server/src/lib.rs @@ -57,7 +57,7 @@ pub fn env_vars_for_npm_tests() -> Vec<(String, String)> { ] } -pub fn env_vars_for_jsr_tests() -> Vec<(String, String)> { +pub fn env_vars_for_jsr_tests_with_git_check() -> Vec<(String, String)> { vec![ ("JSR_URL".to_string(), jsr_registry_url()), ("DISABLE_JSR_PROVENANCE".to_string(), "true".to_string()), @@ -65,6 +65,17 @@ pub fn env_vars_for_jsr_tests() -> Vec<(String, String)> { ] } +pub fn env_vars_for_jsr_tests() -> Vec<(String, String)> { + let mut vars = env_vars_for_jsr_tests_with_git_check(); + + vars.push(( + "DENO_TESTING_DISABLE_GIT_CHECK".to_string(), + "1".to_string(), + )); + + vars +} + pub fn env_vars_for_jsr_provenance_tests() -> Vec<(String, String)> { let mut envs = env_vars_for_jsr_tests(); envs.retain(|(key, _)| key != "DISABLE_JSR_PROVENANCE"); @@ -114,6 +125,10 @@ pub fn env_vars_for_jsr_npm_tests() -> Vec<(String, String)> { vec![ ("NPM_CONFIG_REGISTRY".to_string(), npm_registry_url()), ("JSR_URL".to_string(), jsr_registry_url()), + ( + "DENO_TESTING_DISABLE_GIT_CHECK".to_string(), + "1".to_string(), + ), ("DISABLE_JSR_PROVENANCE".to_string(), "true".to_string()), ("NO_COLOR".to_string(), "1".to_string()), ]