Subject: bug#15425: closed (Re: bug#15425: mktemp - conflicting options affected by ENV)

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 21 Sep 2013 02:45:03 +0000

Your bug report

#15425: mktemp - conflicting options affected by ENV

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
15425: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15425
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#15425: mktemp - conflicting options affected by ENV
From: Pádraig Brady <[email-masked]>
To: Assaf Gordon <[email-masked]>
Cc: [email-masked]
Date: Sat, 21 Sep 2013 03:44:05 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

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-masked]>
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

Subject: mktemp - conflicting options affected by ENV
From: Assaf Gordon <[email-masked]>
To: [email-masked]
Date: Fri, 20 Sep 2013 10:50:25 -0600
List-Id: <debbugs-submit.debbugs.gnu.org>

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

##
## 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".
===

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?

Regards,
  -gordon