autoconf-patches
[Top][All Lists]
Advanced

[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







reply via email to

[Prev in Thread] Current Thread [Next in Thread]