[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: replace autom4te output file atomically
From: |
Eric Blake |
Subject: |
Re: replace autom4te output file atomically |
Date: |
Wed, 20 Aug 2008 20:20:08 +0000 (UTC) |
User-agent: |
Loom/3.14 (http://gmane.org/) |
Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de> writes:
> Passing --force to autom4te fixes the race, but uncovers more issues
> with the test. The patch below, on top of yours, fixes the test in the
> sense that it passes. However, it passes with or without your
> bin/autom4te.in change: when m4 is interrupted during its expansion
> stage, it is writing to a file below the $tmp temporary directory, not
> to the output file. That doesn't mean there is no race to fix, just
> that the race happens after m4 runs.
>
> As I wrote earlier, it is really not easy to expose the interrupt
> race in a testsuite test. I wouldn't know how to do it. Consequently
> I'm wondering whether to drop the test completely.
I think the test is still valuable, even if it isn't testing the patch. I have
no problem keeping it, but I also agree with Ralf's assessment that reliably
forcing an interrupt in the small window where autom4te is replacing the output
file with the post-processing of the m4 temp output is practically impossible.
> The only other questions I ask myself wrt. Ben's patch is: when autoconf
> is interrupted by SIGQUIT (C-\) or KILL, then temporary files may build
> up. Maybe this should be mentioned in the manual somewhere (just to be
> safe against user complaints later)?
That's true of any program that uses temporary files; it seems like
documentation is the right solution (since you obviously can't hook SIGKILL to
do cleanup). This could be as simple as adding a statement that autom4te
honors $TMPDIR (defaulting to /tmp), and uses the namespace am4t* within that
directory. Hmm, the manual doesn't even have an instance of @env{TMPDIR}.
> -AT_CHECK_AUTOM4TE([-o file file.m4])
> +dnl need --force, because input and output may have the same time stamp.
> +dnl autom4te should exit with 128 + SIGTERM on Posix systems.
> +AT_CHECK_AUTOM4TE([--language=m4sh --force -o file file.m4 || exit 1], [1],
[], [ignore])
Style: wrap this line; use m4 to your advantage to keep within 80 columns:
AT_CHECK_AUTOM4TE([--language=m4sh --force -o file file.m4 || exit 1],
[1], [], [ignore])
Technical: this is still begging the question of whether autom4te should
automatically enable --force if input and output share a timestamp. In other
words, in lib/Autom4te/FileUtils.pm, should up_to_date_p be using '<=' instead
of '<'?
Process: still waiting on Ben's paperwork to clear before any of this can be
pushed to savannah.
--
Eric Blake
- replace autom4te output file atomically, Ben Pfaff, 2008/08/12
- Re: replace autom4te output file atomically, Ralf Wildenhues, 2008/08/12
- Re: replace autom4te output file atomically, Ben Pfaff, 2008/08/13
- Re: replace autom4te output file atomically, Ben Pfaff, 2008/08/19
- Re: replace autom4te output file atomically, Ralf Wildenhues, 2008/08/19
- Re: replace autom4te output file atomically, Ben Pfaff, 2008/08/19
- Re: replace autom4te output file atomically, Ralf Wildenhues, 2008/08/20
- Re: replace autom4te output file atomically,
Eric Blake <=
- Re: replace autom4te output file atomically, Ben Pfaff, 2008/08/21
- Re: replace autom4te output file atomically, Ralf Wildenhues, 2008/08/21
- Re: replace autom4te output file atomically, Eric Blake, 2008/08/19