On 09/21/2013 01:40 AM, Pádraig Brady wrote: > On 09/21/2013 01:14 AM, Pádraig Brady wrote: >> On 09/20/2013 05:50 PM, Assaf Gordon wrote: >>> Hello, >>> >>> Not sure if this is a bug or just unintended side-effect, but it caused >>> some scripts here to fail and took a while to troubleshoot: >>> It basically boils down to the interplay between the deprecated "-t" "-p" >>> and "$TMPDIR" and "--tmpdir". >>> >>> === >>> ## >>> ## Problem 1: "-t" and TMPDIR (if non-empty) overrides "--tmpdir" >>> ## >>> $ TMPDIR= mktemp -u --tmpdir=. -t XXXXXX >>> ./og9G5s >>> $ TMPDIR=/foo/ mktemp -u --tmpdir=. -t XXXXXX >>> /foo/AAWcOl >> >> Yes this is confusing, and _not_ order dependent: >> >> $ TMPDIR=/foo/ mktemp -t -u --tmpdir=. XXXXXX >> /foo/AAWcOl >> >>> >>> ## >>> ## Problem 2: if TMPDIR is empty, "--tmpdir" overrides "-p", despite having >>> "-t" >>> ## >>> $ TMPDIR=/foo/ mktemp -u -p /bar/ --tmpdir=. -t XXXXXX >>> /foo/tHXcrq >>> $ TMPDIR= mktemp -u -p /bar/ --tmpdir=. -t XXXXXX >>> ./OfWXSS >>> ## I'd expect the above to use "/bar/OfWXSS", to be consistent with TMPDIR >>> overriding "--tmpdir". >>> === >> >> Also confusing, and order dependent too: >> >> $ mktemp -p /aaa -u --tmpdir=. asdf/XXXX >> ./asdf/wdIT >> $ mktemp -u --tmpdir=. -p /aaa asdf/XXXX >> /aaa/asdf/3Jc7 >> >>> I realize "-t" and "-p" are deprecated, and it's best not to meddle with >>> them - >>> but for consistency, perhaps consider having the new "--tmpdir" always take >>> precedence over "-t" and "-p" ? >>> or print a warning to STDERR when "-t" and "--tmpdir" are mixed? >> >> Yes I agree that --tmpdir should override -pt no matter what the order. >> In fact there is no reason to use -pt with --tmpdir and since the former >> are deprecated, I suggest we just disallow that combination with an error. >> >> Patch coming up. > > I also see the --help says that -p implies -t > AFAICS -t in the context of -p just means to interpret template as a single > file without dirs, > however the example below shows that -t is not in fact implied with -p > > $ mktemp -u -p dir1 dir2/XXXXXX > dir1/dir2/qQBdsf > $ mktemp -u -t -p dir1 dir2/XXXXXX > src/mktemp: invalid template, ‘dir2/XXXXXX’, contains directory separator > > So rather than adjusting any functionality of these deprecated options, > I'll just remove the --help text saying that -t is implied.
Which would make -p = --tmpdir, which is what the code has done since being introduced to coreutils 6 years ago, and what the texinfo has documented for the last 4 years. So I've just aligned the --help to say -p is the short form of --tmpdir. Hopefully the attached addresses all these issues. thanks, Pádraig.
>From 53e0ce82ddd47e0d166742426c91d9b8473b8006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Sat, 21 Sep 2013 03:33:12 +0100 Subject: [PATCH] mktemp: disallow deprecated -t with --tempdir * doc/coreutils.texi (mktemp invocation): Clarify that _all_ error messages are suppressed with the -q option. * src/mktemp.c (usage): Synchronize the -p option description with the logic and info docs. I.E. that -p is just an alias of --tmpdir. (main): Disallow the deprecated -t option in combination with --tmpdir, which would result in -t getting precedence. * tests/misc/mktemp.pl: Add a test case. * NEWS: Mention the change in behavior. Fixes http://bugs.gnu.org/154245 --- NEWS | 2 ++ doc/coreutils.texi | 5 +++-- src/mktemp.c | 21 +++++++++++---------- tests/misc/mktemp.pl | 5 +++++ 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/NEWS b/NEWS index d26722d..4855475 100644 --- a/NEWS +++ b/NEWS @@ -65,6 +65,8 @@ GNU coreutils NEWS -*- outline -*- dd status=none now suppresses all non fatal diagnostic messages, not just the transfer counts. + mktemp now disallows using the deprecated -t option along with --tmpdir. + stdbuf now requires at least one buffering mode option to be specified, as per the documented interface. diff --git a/doc/coreutils.texi b/doc/coreutils.texi index 21216b4..f2eba72 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -13421,8 +13421,9 @@ umask is more restrictive. @itemx --quiet @opindex -q @opindex --quiet -Suppress diagnostics about failure to create a file or directory. The -exit status will still reflect whether a file was created. +Suppress all error messages, including command option syntax errors, +and failure to create a file or directory. +The exit status will still reflect whether a file was created. @item -u @itemx --dry-run diff --git a/src/mktemp.c b/src/mktemp.c index 44845c3..bb0e8c1 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -43,7 +43,6 @@ static const char *default_template = "tmp.XXXXXXXXXX"; enum { SUFFIX_OPTION = CHAR_MAX + 1, - TMPDIR_OPTION }; static struct option const longopts[] = @@ -52,7 +51,7 @@ static struct option const longopts[] = {"quiet", no_argument, NULL, 'q'}, {"dry-run", no_argument, NULL, 'u'}, {"suffix", required_argument, NULL, SUFFIX_OPTION}, - {"tmpdir", optional_argument, NULL, TMPDIR_OPTION}, + {"tmpdir", optional_argument, NULL, 'p'}, {GETOPT_HELP_OPTION_DECL}, {GETOPT_VERSION_OPTION_DECL}, {NULL, 0, NULL, 0} @@ -78,14 +77,14 @@ Files are created u+rw, and directories u+rwx, minus umask restrictions.\n\ fputs (_("\ -d, --directory create a directory, not a file\n\ -u, --dry-run do not create anything; merely print a name (unsafe)\n\ - -q, --quiet suppress diagnostics about file/dir-creation failure\n\ + -q, --quiet suppress all error messages\n\ "), stdout); fputs (_("\ --suffix=SUFF append SUFF to TEMPLATE; SUFF must not contain a slash.\n\ This option is implied if TEMPLATE does not end in X\n\ "), stdout); fputs (_("\ - --tmpdir[=DIR] interpret TEMPLATE relative to DIR; if DIR is not\n\ + -p DIR, --tmpdir[=DIR] interpret TEMPLATE relative to DIR; if DIR is not\n\ specified, use $TMPDIR if set, else /tmp. With\n\ this option, TEMPLATE must not be an absolute name;\n\ unlike with -t, TEMPLATE may contain slashes, but\n\ @@ -93,7 +92,6 @@ Files are created u+rw, and directories u+rwx, minus umask restrictions.\n\ "), stdout); fputs ("\n", stdout); fputs (_("\ - -p DIR use DIR as a prefix; implies -t [deprecated]\n\ -t interpret TEMPLATE as a single file name component,\n\ relative to a directory: $TMPDIR, if set; else the\n\ directory specified via -p; else /tmp [deprecated]\n\ @@ -158,6 +156,7 @@ main (int argc, char **argv) char *suffix = NULL; bool use_dest_dir = false; bool deprecated_t_option = false; + bool tmpdir_option = false; bool create_directory = false; bool dry_run = false; int status = EXIT_SUCCESS; @@ -183,6 +182,7 @@ main (int argc, char **argv) case 'p': dest_dir_arg = optarg; use_dest_dir = true; + tmpdir_option = true; break; case 'q': suppress_stderr = true; @@ -195,11 +195,6 @@ main (int argc, char **argv) dry_run = true; break; - case TMPDIR_OPTION: - use_dest_dir = true; - dest_dir_arg = optarg; - break; - case SUFFIX_OPTION: suffix = optarg; break; @@ -223,6 +218,12 @@ main (int argc, char **argv) _("failed to redirect stderr to /dev/null")); } + if (deprecated_t_option && tmpdir_option) + { + error (0, 0, _("the deprecated -t option is not allowed with --tmpdir")); + usage (EXIT_FAILURE); + } + n_args = argc - optind; if (2 <= n_args) { diff --git a/tests/misc/mktemp.pl b/tests/misc/mktemp.pl index b15b669..f9baac3 100755 --- a/tests/misc/mktemp.pl +++ b/tests/misc/mktemp.pl @@ -64,6 +64,11 @@ my @Tests = {ERR=>"$prog: too few X's in template 'foo.XX'\n"}], ['too-few-xq', '-q foo.XX', {EXIT => 1} ], + ['t-and-tmpdir', '-t --tmpdir', + {ERR=>"$prog: the deprecated -t option is not allowed with --tmpdir\n" + . "Try '$prog --help' for more information.\n"}, {EXIT => 1} ], + ['t-and-tmpdir-q', '-q -t --tmpdir', {EXIT => 1} ], + ['1f', 'bar.XXXX', {OUT => "bar.ZZZZ\n"}, {OUT_SUBST => 's,\.....$,.ZZZZ,'}, {POST => sub { my ($f) = @_; defined $f or return; chomp $f; -- 1.7.7.6
