1
Fork 0
mirror of https://github.com/RGBCube/uutils-coreutils synced 2025-07-28 11:37:44 +00:00

Require = for --tmpdir in mktemp

Fixes #3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
https://github.com/clap-rs/clap/issues/4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
This commit is contained in:
Thayne McCombs 2023-02-10 23:31:28 -07:00 committed by Sylvestre Ledru
parent 1d171b5166
commit b77a08c134

View file

@ -8,7 +8,7 @@
// spell-checker:ignore (paths) GPGHome findxs // spell-checker:ignore (paths) GPGHome findxs
use clap::{crate_version, Arg, ArgAction, ArgMatches, Command}; use clap::{builder::ValueParser, crate_version, Arg, ArgAction, ArgMatches, Command};
use uucore::display::{println_verbatim, Quotable}; use uucore::display::{println_verbatim, Quotable};
use uucore::error::{FromIo, UError, UResult, UUsageError}; use uucore::error::{FromIo, UError, UResult, UUsageError};
use uucore::{format_usage, help_about, help_usage}; use uucore::{format_usage, help_about, help_usage};
@ -38,6 +38,7 @@ static OPT_DRY_RUN: &str = "dry-run";
static OPT_QUIET: &str = "quiet"; static OPT_QUIET: &str = "quiet";
static OPT_SUFFIX: &str = "suffix"; static OPT_SUFFIX: &str = "suffix";
static OPT_TMPDIR: &str = "tmpdir"; static OPT_TMPDIR: &str = "tmpdir";
static OPT_P: &str = "p";
static OPT_T: &str = "t"; static OPT_T: &str = "t";
static ARG_TEMPLATE: &str = "template"; static ARG_TEMPLATE: &str = "template";
@ -130,7 +131,7 @@ struct Options {
/// The directory in which to create the temporary file. /// The directory in which to create the temporary file.
/// ///
/// If `None`, the file will be created in the current directory. /// If `None`, the file will be created in the current directory.
tmpdir: Option<String>, tmpdir: Option<PathBuf>,
/// The suffix to append to the temporary file, if any. /// The suffix to append to the temporary file, if any.
suffix: Option<String>, suffix: Option<String>,
@ -142,72 +143,32 @@ struct Options {
template: String, template: String,
} }
/// Decide whether the argument to `--tmpdir` should actually be the template.
///
/// This function is required to work around a limitation of `clap`,
/// the command-line argument parsing library. In case the command
/// line is
///
/// ```sh
/// mktemp --tmpdir XXX
/// ```
///
/// the program should behave like
///
/// ```sh
/// mktemp --tmpdir=${TMPDIR:-/tmp} XXX
/// ```
///
/// However, `clap` thinks that `XXX` is the value of the `--tmpdir`
/// option. This function returns `true` in this case and `false`
/// in all other cases.
fn is_tmpdir_argument_actually_the_template(matches: &ArgMatches) -> bool {
if !matches.contains_id(ARG_TEMPLATE) {
if let Some(tmpdir) = matches.get_one::<String>(OPT_TMPDIR) {
if !Path::new(tmpdir).is_dir() && tmpdir.contains("XXX") {
return true;
}
}
}
false
}
impl Options { impl Options {
fn from(matches: &ArgMatches) -> Self { fn from(matches: &ArgMatches) -> Self {
// Special case to work around a limitation of `clap`; see let tmpdir = matches
// `is_tmpdir_argument_actually_the_template()` for more .get_one::<PathBuf>(OPT_TMPDIR)
// information. .or_else(|| matches.get_one::<PathBuf>(OPT_P))
// .cloned();
// Fixed in clap 3 let (tmpdir, template) = match matches.get_one::<String>(ARG_TEMPLATE) {
// See https://github.com/clap-rs/clap/pull/1587
let (tmpdir, template) = if is_tmpdir_argument_actually_the_template(matches) {
let tmpdir = Some(env::temp_dir().display().to_string());
let template = matches.get_one::<String>(OPT_TMPDIR).unwrap().to_string();
(tmpdir, template)
} else {
// If no template argument is given, `--tmpdir` is implied. // If no template argument is given, `--tmpdir` is implied.
match matches.get_one::<String>(ARG_TEMPLATE) { None => {
None => { let tmpdir = Some(tmpdir.unwrap_or_else(env::temp_dir));
let tmpdir = match matches.get_one::<String>(OPT_TMPDIR) { let template = DEFAULT_TEMPLATE;
None => Some(env::temp_dir().display().to_string()), (tmpdir, template.to_string())
Some(tmpdir) => Some(tmpdir.to_string()), }
}; Some(template) => {
let template = DEFAULT_TEMPLATE; let tmpdir = if env::var(TMPDIR_ENV_VAR).is_ok() && matches.get_flag(OPT_T) {
(tmpdir, template.to_string()) env::var_os(TMPDIR_ENV_VAR).map(|t| t.into())
} } else if tmpdir.is_some() {
Some(template) => { tmpdir
let tmpdir = if env::var(TMPDIR_ENV_VAR).is_ok() && matches.get_flag(OPT_T) { } else if matches.get_flag(OPT_T) || matches.contains_id(OPT_TMPDIR) {
env::var(TMPDIR_ENV_VAR).ok() // If --tmpdir is given without an argument, or -t is given
} else if matches.contains_id(OPT_TMPDIR) { // export in TMPDIR
matches.get_one::<String>(OPT_TMPDIR).map(String::from) Some(env::temp_dir())
} else if matches.get_flag(OPT_T) { } else {
// mktemp -t foo.xxx should export in TMPDIR None
Some(env::temp_dir().display().to_string()) };
} else { (tmpdir, template.to_string())
matches.get_one::<String>(OPT_TMPDIR).map(String::from)
};
(tmpdir, template.to_string())
}
} }
}; };
Self { Self {
@ -372,7 +333,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
if env::var("POSIXLY_CORRECT").is_ok() { if env::var("POSIXLY_CORRECT").is_ok() {
// If POSIXLY_CORRECT was set, template MUST be the last argument. // If POSIXLY_CORRECT was set, template MUST be the last argument.
if is_tmpdir_argument_actually_the_template(&matches) || matches.contains_id(ARG_TEMPLATE) { if matches.contains_id(ARG_TEMPLATE) {
// Template argument was provided, check if was the last one. // Template argument was provided, check if was the last one.
if args.last().unwrap() != &options.template { if args.last().unwrap() != &options.template {
return Err(Box::new(MkTempError::TooManyTemplates)); return Err(Box::new(MkTempError::TooManyTemplates));
@ -444,8 +405,16 @@ pub fn uu_app() -> Command {
.value_name("SUFFIX"), .value_name("SUFFIX"),
) )
.arg( .arg(
Arg::new(OPT_TMPDIR) Arg::new(OPT_P)
.short('p') .short('p')
.help("short form of --tmpdir")
.value_name("DIR")
.num_args(1)
.value_parser(ValueParser::path_buf())
.value_hint(clap::ValueHint::DirPath),
)
.arg(
Arg::new(OPT_TMPDIR)
.long(OPT_TMPDIR) .long(OPT_TMPDIR)
.help( .help(
"interpret TEMPLATE relative to DIR; if DIR is not specified, use \ "interpret TEMPLATE relative to DIR; if DIR is not specified, use \
@ -457,6 +426,9 @@ pub fn uu_app() -> Command {
// Allows use of default argument just by setting --tmpdir. Else, // Allows use of default argument just by setting --tmpdir. Else,
// use provided input to generate tmpdir // use provided input to generate tmpdir
.num_args(0..=1) .num_args(0..=1)
// Require an equals to avoid ambiguity if no tmpdir is supplied
.require_equals(true)
.value_parser(ValueParser::path_buf())
.value_hint(clap::ValueHint::DirPath), .value_hint(clap::ValueHint::DirPath),
) )
.arg( .arg(