Richard W.M. Jones wrote:
> On Thu, Nov 24, 2011 at 03:01:03PM +0000, Paul Howarth wrote:
>> http://marc.info/?l=pptpclient-devel&m=132102054518031
>
> This is indeed rather unexpected behaviour of make!
>
> BTW I think your patch is incomplete, since it will create an
> incomplete config.h if the disk runs out of space.  I think this would
> be better, and should also avoid the parallel make problem:
>
> config.h:
>       echo "/* text added by Makefile target config.h */" > $@-t
>       echo "#define PPTP_LINUX_VERSION \"$(VERSION)$(RELEASE)\"" >> $@-t
>       echo "#define PPPD_BINARY \"$(PPPD)\"" >> $@-t
>       echo "#define IP_BINARY \"$(IP)\"" >> $@-t
>       mv $@-t $@

Yes, that is better and does solve the problem.

Here's a good rule I've been following since I was first burned by this:
"never redirect directly to a Makefile target". (where "target" means $@
or any literal that is equivalent).  That means redirect output to a
temporary file like Rich did above, and only when that process is known
to have succeeded do you rename the temporary file to $@.

However, I would volunteer some additional suggestions:

  - use a {...} group (cheaper than a (...) subshell) so we redirect only once;
    that reduces the duplication of the "$@-t", and eliminates the dichotomy
    of the initial "> $@-t", while all subsequent ones are ">> $@-t".

  - use outer single quotes so you don't have to backslash-escape the
    inner double quote chars.  less syntax is more maintainable.

  - make the file read-only, so it is clearer that it is generated,
    and thus harder to change accidentally.  The only down-side to
    this part is that you then have to be careful to remove any
    preexisting $@ or $@-t, since redirecting onto a read-only file
    will often fail.

config.h:
        rm -f $@ $@-t;                                                  \
        {                                                               \
          echo '/* text added by Makefile target config.h */';          \
          echo '#define PPTP_LINUX_VERSION "$(VERSION)$(RELEASE)"';     \
          echo '#define PPPD_BINARY "$(PPPD)"';                         \
          echo '#define IP_BINARY "$(IP)"';                             \
        } > $@-t && chmod a-w $@-t && mv $@-t $@
-- 
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel

Reply via email to