Portable mktemp calls in the tests

[Martin Schwenke]

On Tue, 2 Jan 2018 09:56:48 +0000, Rowland Penny <rpenny at samba.org>
wrote:

> On Tue, 2 Jan 2018 12:54:18 +1100
> Martin Schwenke via samba-technical <samba-technical at lists.samba.org>
> wrote:
> 
> > Hi Timur,
> > 
> > On Fri, 22 Dec 2017 06:46:24 +0100, "Timur I. Bakeyev"
> > <timur at freebsd.org> wrote:
> >   
> > > On Fri, Dec 22, 2017 at 5:55 AM, Martin Schwenke
> > > <martin at meltin.net> wrote:
> > >   
>  [...]  
>  [...]  
>  [...]  
> > > 
> > > Sounds smarter way of doing it :)
> > > 
> > > Note there's another problem!  Where a template is used (and,  
>  [...]  
> > > 
> > > I've tested it, but seems to forgot to properly add it everywhere...
> > > 
> > > Seems, construction like:
> > > 
> > > TMPDIR=/var/tmp mktemp -t blah-xxxxx
> > > 
> > > Note '-t' flag ^
> > > 
> > > Works both for FreeBSD and Linux(at least, Debian) as intended.
> > > Creation of directories
> > > with extra '-d' also works.
> > > 
> > > So, let me re-send updated patch.  
> > 
> > I've looked at both the GNU and FreeBSD sources for mktemp:
> > 
> >   https://svnweb.freebsd.org/base/release/11.1.0/usr.bin/mktemp/mktemp.c?view=markup
> > 
> >   https://git.savannah.gnu.org/cgit/coreutils.git/tree/src/mktemp.c?h=v8.29&id=27b2b19aa8d8b30b8cb4198b2f4b54568e10a35e
> > 
> > and I have also run some tests with both versions.
> > 
> > It is possible to have portability without using the deprecated -t
> > option.
> > 
> > * When a template is being used then put the directory inline:
> > 
> >     mktemp /var/tmp/blah-xxxxx
> > 
> > * When a template is not being used then use TMPDIR:
> > 
> >     TMPDIR=/var/tmp mktemp
> > 
> > Both of these appear to work with and without the -d option.
> > 
> > I really don't think we should be using -t, since it deprecated in the
> > GNU version.
> > 
> > I am wondering if your testing is having problems due to the typo
> > here:
> >   
> > >   tmpeditor=$(mktemp "$STpath/bin/samba-tool-editor-XXXXXXXX")
> > >   tmpeditor="${tempeditor}.sh"  
> > 
> > or if the "tempeditor" typo is only in your email.  :-)
> > 
> > peace & happiness,
> > martin
> >   
> 
> This is all my fault, I wrote the script in question.
> I initially used a hardcoded '/tmp/editor.sh', but was advised that
> this wasn't a good idea, I changed it to what is there now (without
> really thinking it through or considering portability)
> 
> With hindsight and thought, it would have been better to use:
> 
> tmpeditor=$(mktemp /tmp/samba-tool-editor-XXXXXXXX)
> 
> or
> 
> tmpeditor=$(mktemp /var/tmp/samba-tool-editor-XXXXXXXX)
> 
> Adding the '.sh' suffix is a bit of a red herring, 'scriptname' will
> work just as well as 'scriptname.sh'

I think that if we make portability changes to the scripts then we
shouldn't change behaviour in the same commit.  If we decide to drop
the ".sh" suffix then we should probably do that explicitly in a
separate commit, either before or afterwards.

That said, I don't think the ".sh" suffix is a problem.  It means the
temporary filename is self-documenting.

It is the wide range of non-portable options passed to mktemp that is
the problem... and I've certainly been using them...  :-)

peace & happiness,
martin