diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 21adaabd9bb..d579562d8cf 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1032,7 +1032,10 @@ impl Options { let recursive = matches.get_flag(options::RECURSIVE) || matches.get_flag(options::ARCHIVE); - let backup_mode = match backup_control::determine_backup_mode(matches) { + let backup_mode = match backup_control::determine_backup_mode( + std::env::var("VERSION_CONTROL").ok(), + matches, + ) { Err(e) => return Err(CpError::Backup(BackupError(format!("{e}")))), Ok(mode) => mode, }; diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index f19f8aca249..c958633029a 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -361,7 +361,8 @@ fn behavior(matches: &ArgMatches) -> UResult { None }; - let backup_mode = backup_control::determine_backup_mode(matches)?; + let backup_mode = + backup_control::determine_backup_mode(std::env::var("VERSION_CONTROL").ok(), matches)?; let target_dir = matches.get_one::(OPT_TARGET_DIRECTORY).cloned(); let no_target_dir = matches.get_flag(OPT_NO_TARGET_DIRECTORY); if target_dir.is_some() && no_target_dir { diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index 96e708b2b1a..2e0459673cd 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -128,7 +128,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { OverwriteMode::NoClobber }; - let backup_mode = backup_control::determine_backup_mode(&matches)?; + let backup_mode = + backup_control::determine_backup_mode(std::env::var("VERSION_CONTROL").ok(), &matches)?; let backup_suffix = backup_control::determine_backup_suffix(&matches); // When we have "-L" or "-L -P", false otherwise diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index b750428f7e5..a95e5a75b22 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -173,7 +173,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } let overwrite_mode = determine_overwrite_mode(&matches); - let backup_mode = backup_control::determine_backup_mode(&matches)?; + let backup_mode = + backup_control::determine_backup_mode(env::var("VERSION_CONTROL").ok(), &matches)?; let update_mode = update_control::determine_update_mode(&matches); if backup_mode != BackupMode::None diff --git a/src/uucore/src/lib/features/backup_control.rs b/src/uucore/src/lib/features/backup_control.rs index f56d131c7d6..4042e86c456 100644 --- a/src/uucore/src/lib/features/backup_control.rs +++ b/src/uucore/src/lib/features/backup_control.rs @@ -60,7 +60,7 @@ //! "command", "--backup=t", "--suffix=bak~" //! ]); //! -//! let backup_mode = match backup_control::determine_backup_mode(&matches) { +//! let backup_mode = match backup_control::determine_backup_mode(std::env::var("VERSION_CONTROL").ok(), &matches) { //! Err(e) => { //! show!(e); //! return; @@ -340,7 +340,7 @@ pub fn determine_backup_suffix(matches: &ArgMatches) -> String { /// show!(err); /// } /// ``` -pub fn determine_backup_mode(matches: &ArgMatches) -> UResult { +pub fn determine_backup_mode(env_ctl: Option, matches: &ArgMatches) -> UResult { if matches.contains_id(arguments::OPT_BACKUP) { // Use method to determine the type of backups to make. When this option // is used but method is not specified, then the value of the @@ -349,7 +349,7 @@ pub fn determine_backup_mode(matches: &ArgMatches) -> UResult { if let Some(method) = matches.get_one::(arguments::OPT_BACKUP) { // Second argument is for the error string that is returned. match_method(method, "backup type") - } else if let Ok(method) = env::var("VERSION_CONTROL") { + } else if let Some(method) = env_ctl { // Second argument is for the error string that is returned. match_method(&method, "$VERSION_CONTROL") } else { @@ -360,7 +360,7 @@ pub fn determine_backup_mode(matches: &ArgMatches) -> UResult { // the short form of this option, -b does not accept any argument. // if VERSION_CONTROL is not set then using -b is equivalent to // using --backup=existing. - if let Ok(method) = env::var("VERSION_CONTROL") { + if let Some(method) = env_ctl { match_method(&method, "$VERSION_CONTROL") } else { Ok(BackupMode::Existing) @@ -368,7 +368,7 @@ pub fn determine_backup_mode(matches: &ArgMatches) -> UResult { } else if matches.contains_id(arguments::OPT_SUFFIX) { // Suffix option is enough to determine mode even if --backup is not set. // If VERSION_CONTROL is not set, the default backup type is 'existing'. - if let Ok(method) = env::var("VERSION_CONTROL") { + if let Some(method) = env_ctl { match_method(&method, "$VERSION_CONTROL") } else { Ok(BackupMode::Existing) @@ -502,9 +502,6 @@ mod tests { // it concurrently. static TEST_MUTEX: Mutex<()> = Mutex::new(()); - // Environment variable for "VERSION_CONTROL" - static ENV_VERSION_CONTROL: &str = "VERSION_CONTROL"; - fn make_app() -> Command { Command::new("command") .arg(arguments::backup()) @@ -518,7 +515,7 @@ mod tests { let _dummy = TEST_MUTEX.lock().unwrap(); let matches = make_app().get_matches_from(vec!["command", "-b"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(None, &matches).unwrap(); assert_eq!(result, BackupMode::Existing); } @@ -529,7 +526,7 @@ mod tests { let _dummy = TEST_MUTEX.lock().unwrap(); let matches = make_app().get_matches_from(vec!["command", "-b", "--backup=none"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(None, &matches).unwrap(); assert_eq!(result, BackupMode::None); } @@ -540,7 +537,7 @@ mod tests { let _dummy = TEST_MUTEX.lock().unwrap(); let matches = make_app().get_matches_from(vec!["command", "--backup"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(None, &matches).unwrap(); assert_eq!(result, BackupMode::Existing); } @@ -551,7 +548,7 @@ mod tests { let _dummy = TEST_MUTEX.lock().unwrap(); let matches = make_app().get_matches_from(vec!["command", "--backup=simple"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(None, &matches).unwrap(); assert_eq!(result, BackupMode::Simple); } @@ -562,7 +559,7 @@ mod tests { let _dummy = TEST_MUTEX.lock().unwrap(); let matches = make_app().get_matches_from(vec!["command", "--backup=foobar"]); - let result = determine_backup_mode(&matches); + let result = determine_backup_mode(None, &matches); assert!(result.is_err()); let text = format!("{}", result.unwrap_err()); @@ -575,7 +572,7 @@ mod tests { let _dummy = TEST_MUTEX.lock().unwrap(); let matches = make_app().get_matches_from(vec!["command", "--backup=n"]); - let result = determine_backup_mode(&matches); + let result = determine_backup_mode(None, &matches); assert!(result.is_err()); let text = format!("{}", result.unwrap_err()); @@ -588,7 +585,7 @@ mod tests { let _dummy = TEST_MUTEX.lock().unwrap(); let matches = make_app().get_matches_from(vec!["command", "--backup=si"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(None, &matches).unwrap(); assert_eq!(result, BackupMode::Simple); } @@ -597,69 +594,59 @@ mod tests { #[test] fn test_backup_mode_short_does_not_ignore_env() { let _dummy = TEST_MUTEX.lock().unwrap(); - unsafe { env::set_var(ENV_VERSION_CONTROL, "numbered") }; let matches = make_app().get_matches_from(vec!["command", "-b"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(Some("numbered".into()), &matches).unwrap(); assert_eq!(result, BackupMode::Numbered); - unsafe { env::remove_var(ENV_VERSION_CONTROL) }; } // --backup can be passed without an argument, but reads env var if existent #[test] fn test_backup_mode_long_without_args_with_env() { let _dummy = TEST_MUTEX.lock().unwrap(); - unsafe { env::set_var(ENV_VERSION_CONTROL, "none") }; let matches = make_app().get_matches_from(vec!["command", "--backup"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(Some("none".into()), &matches).unwrap(); assert_eq!(result, BackupMode::None); - unsafe { env::remove_var(ENV_VERSION_CONTROL) }; } // --backup errors on invalid VERSION_CONTROL env var #[test] fn test_backup_mode_long_with_env_var_invalid() { let _dummy = TEST_MUTEX.lock().unwrap(); - unsafe { env::set_var(ENV_VERSION_CONTROL, "foobar") }; let matches = make_app().get_matches_from(vec!["command", "--backup"]); - let result = determine_backup_mode(&matches); + let result = determine_backup_mode(Some("foobar".into()), &matches); assert!(result.is_err()); let text = format!("{}", result.unwrap_err()); assert!(text.contains("invalid argument 'foobar' for '$VERSION_CONTROL'")); - unsafe { env::remove_var(ENV_VERSION_CONTROL) }; } // --backup errors on ambiguous VERSION_CONTROL env var #[test] fn test_backup_mode_long_with_env_var_ambiguous() { let _dummy = TEST_MUTEX.lock().unwrap(); - unsafe { env::set_var(ENV_VERSION_CONTROL, "n") }; let matches = make_app().get_matches_from(vec!["command", "--backup"]); - let result = determine_backup_mode(&matches); + let result = determine_backup_mode(Some("n".into()), &matches); assert!(result.is_err()); let text = format!("{}", result.unwrap_err()); assert!(text.contains("ambiguous argument 'n' for '$VERSION_CONTROL'")); - unsafe { env::remove_var(ENV_VERSION_CONTROL) }; } // --backup accepts shortened env vars (si for simple) #[test] fn test_backup_mode_long_with_env_var_shortened() { let _dummy = TEST_MUTEX.lock().unwrap(); - unsafe { env::set_var(ENV_VERSION_CONTROL, "si") }; let matches = make_app().get_matches_from(vec!["command", "--backup"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(Some("si".into()), &matches).unwrap(); assert_eq!(result, BackupMode::Simple); - unsafe { env::remove_var(ENV_VERSION_CONTROL) }; } // Using --suffix without --backup defaults to --backup=existing @@ -668,7 +655,7 @@ mod tests { let _dummy = TEST_MUTEX.lock().unwrap(); let matches = make_app().get_matches_from(vec!["command", "--suffix", ".bak"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(None, &matches).unwrap(); assert_eq!(result, BackupMode::Existing); } @@ -677,13 +664,11 @@ mod tests { #[test] fn test_backup_mode_suffix_without_backup_option_with_env_var() { let _dummy = TEST_MUTEX.lock().unwrap(); - unsafe { env::set_var(ENV_VERSION_CONTROL, "numbered") }; let matches = make_app().get_matches_from(vec!["command", "--suffix", ".bak"]); - let result = determine_backup_mode(&matches).unwrap(); + let result = determine_backup_mode(Some("numbered".into()), &matches).unwrap(); assert_eq!(result, BackupMode::Numbered); - unsafe { env::remove_var(ENV_VERSION_CONTROL) }; } #[test]