Discussion:
Change to genassym to use mktemp(1) to create tempdir
J.T. Conklin
2014-01-06 20:17:49 UTC
Permalink
At my $DAYJOB, we encounter occasional build failures when genassym
can't create its temporary directory because that directory already
exists from a previous build. I have not yet been able to determine
why this happens, especially since the genassym script contains a
"trap" to remove the directory on exit/error.

The stale temp directories contain either a truncated or a zero-length
genassym.out file. I suspect the problem may be related to our Linux
build servers or the distributed make system we use, perhaps when a
build's job steps are forcefully migrated to another node when a
higher priority build is dispatched, such that the trap does not fire.

Since tracking down the root cause is secondary to avoiding spurious
build failures, I changed to genassym.sh to use mktemp(1) to create
its temporary directory instead of using "mkdir /tmp/genassym.$$".

While this failure seems unlikely, I offer the patch, as it does make
genassym slightly more reliable.

--jtc

Index: genassym.sh
===================================================================
RCS file: /cvsroot/src/usr.bin/genassym/genassym.sh,v
retrieving revision 1.7
diff -u -r1.7 genassym.sh
--- genassym.sh 5 Jul 2011 05:19:02 -0000 1.7
+++ genassym.sh 6 Jan 2014 19:40:41 -0000
@@ -69,9 +69,9 @@
esac
done

-genassym_temp=/tmp/genassym.$$
+genassym_temp=`mktemp -d -q ${TMPDIR-/tmp}/genassym.XXXXXX`

-if ! mkdir $genassym_temp; then
+if ! test -d $genassym_temp; then
echo "${progname}: unable to create temporary directory" >&2
exit 1
fi
--
J.T. Conklin
Jeremy C. Reed
2014-01-06 21:58:02 UTC
Permalink
Post by J.T. Conklin
-genassym_temp=/tmp/genassym.$$
+genassym_temp=`mktemp -d -q ${TMPDIR-/tmp}/genassym.XXXXXX`
-if ! mkdir $genassym_temp; then
+if ! test -d $genassym_temp; then
echo "${progname}: unable to create temporary directory" >&2
exit 1
fi
This seems fine to me.

But maybe don't use -q and let it fail at same time:

genassym_temp=`mktemp -d ${TMPDIR-/tmp}/genassym.XXXXXX` || {
echo "${progname}: unable to create temporary directory" >&2 ;
exit 1
}
J.T. Conklin
2014-01-07 14:48:42 UTC
Permalink
Post by Jeremy C. Reed
Post by J.T. Conklin
-genassym_temp=/tmp/genassym.$$
+genassym_temp=`mktemp -d -q ${TMPDIR-/tmp}/genassym.XXXXXX`
-if ! mkdir $genassym_temp; then
+if ! test -d $genassym_temp; then
echo "${progname}: unable to create temporary directory" >&2
exit 1
fi
This seems fine to me.
genassym_temp=`mktemp -d ${TMPDIR-/tmp}/genassym.XXXXXX` || {
echo "${progname}: unable to create temporary directory" >&2 ;
exit 1
}
The -q option only suppresses mktemp's diagnostics, it doesn't change
the return value. Having both mktemp and genassym output a diagnostic
may be a bit messy/confusing.

That being said, I've checked other uses of mktemp in the NetBSD tree,
and the most common idiom is more like what you propose. For example,
following the cue of dg-extract-results.sh, the code would be like:

{
genassym_temp=`mktemp -d -q ${TMPDIR-/tmp}/genassym.XXXXXX` &&
test -n "$genassym_temp" &&
test -d "$genassym_temp"
} || {
genassym_temp=${TMPDIR-/tmp}/genassym.$$.$RANDOM
mkdir $genassym_temp > /dev/null 2>&1
} || {
echo "${progname}: unable to create temporary directory" >&2 ;
exit 1
}

This is probably much more conservative that we need, as we know that
mktemp(1) is available and that it exits with a non-zero return value
on failure. Simplifying, it pretty much ends up like your suggestion.
This is also the approach used by the mktemp manpage example (albeit
checking $? instead of using ||), so I'll plan to rework and send a pr
along those lines in the next few days.

--jtc
--
J.T. Conklin
Jeremy C. Reed
2014-01-07 15:45:14 UTC
Permalink
Post by J.T. Conklin
The -q option only suppresses mktemp's diagnostics, it doesn't change
the return value. Having both mktemp and genassym output a diagnostic
may be a bit messy/confusing.
But adds better (than "unable to create temporary directory") detail
like: "File exists" or "No such file or directory" or "Permission
denied" etc.
Christos Zoulas
2014-01-07 16:28:31 UTC
Permalink
Post by Jeremy C. Reed
Post by J.T. Conklin
The -q option only suppresses mktemp's diagnostics, it doesn't change
the return value. Having both mktemp and genassym output a diagnostic
may be a bit messy/confusing.
But adds better (than "unable to create temporary directory") detail
like: "File exists" or "No such file or directory" or "Permission
denied" etc.
While this discussion has been going on, I've committed a fix.

christos

Loading...