Re: [PATCH] tests: Let grub_cmd_cryptomount by default operate in /tmp rather than in /

[Thomas Schmitt via Grub-devel <grub-devel@gnu.org>]

Hi,

i wrote:
> > Further delete each created directory as soon as the command of its
> > test case is finished.
> > [...]
> >      mkdir -p "$TMPDIR"
> >
> >      output=`"$@" 2>&1` || res=$?
> > +
> > +    rmdir "$TMPDIR"

Daniel Kiper wrote:
> s/rmdir/rm -rf/?

This is equivalent to the question whether remaining content shall be
removed silently. In my case the directories were all empty.

The worker in TMPDIR is  @builddir@/grub-shell-luks-tester  which stems
from  tests/util/grub-shell-luks-tester.in . It shows own effort to leave
a clean TMPDIR:

  cleanup() {
      ...
      if [ -z "$debug" ] && [ "${RET:-1}" -eq 0 ]; then
          rm -rf "$lukstestdir" || :
      fi
  }
  trap cleanup EXIT INT TERM KILL QUIT
  ...
  lukstestdir="`mktemp -d "${TMPDIR:-/tmp}/$(basename "$0").XXXXXXXXXX"`" || exit 20

Seeing the condition before "rm -rf", i guess tests/grub_cmd_cryptomount
should rather not remove the directory if it is not empty. A smarter way
than letting rmdir loudly fail seems appropriate, though.

If you agree to my assessment i will try to propose one in v2.

-----------------------------------------------------------------------
Self criticism:

I have recognized meanwhile that the proposed gesture

> > +    : ${TMPDIR:=/tmp}

stems from the gnulib subdirectory and is not tradition in the rest
of the GRUB git repo.
GRUB test tradition seems to be temporary defaulting of TMPDIR like in
line 174 of tests/grub_cmd_cryptomount.in :

  csscript=`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"` || exit 99

Indeed the patch would be less needy of a lengthy comment if i propose
something like this yet untested change instead:

-    TMPDIR=$TMPDIR/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[ /],_,g' -e 's,:$,,g'`
+    TMPDIR="${TMPDIR:-/tmp}"/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[ /],_,g' -e 's,:$,,g'`

If you agree to my assessment, i will propose this in patch v2.
(Tested, of course.)


Have a nice day :)

Thomas


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel