fix(publish): reland error if there are uncommitted changes (#22613) (#22632)

Reverted in https://github.com/denoland/deno/pull/22625

cli/args/flags.rs

@@ -307,6 +307,7 @@ pub struct PublishFlags {
   pub token: Option<String>,
   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,

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;

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");
+}

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()),
   ]