[PATCH util/macros 1/2] Create CHANGELOG/INSTALL_CMD .tmp files in builddir

[Emil Velikov]

On 27 January 2017 at 04:53, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> On Thu, Jan 26, 2017 at 05:53:19PM +0000, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> Under normal build rules one should consider srcdir as RO, thus creating
>> files in srcdir is going to fail.
>>
>> This was flagged with a recent work in release.sh
>>
>> Cc: Peter Hutterer <peter.hutterer at who-t.net>
>> Cc: Gaetan Nadon <memsize at videotron.ca>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>>  xorg-macros.m4.in | 6 +++---
>>  xorgversion.m4    | 6 +++---
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in
>> index 2ed7837..675d07d 100644
>> --- a/xorg-macros.m4.in
>> +++ b/xorg-macros.m4.in
>> @@ -1837,9 +1837,9 @@ m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])],
>>  AC_DEFUN([XORG_INSTALL], [
>>  AC_REQUIRE([PKG_PROG_PKG_CONFIG])
>>  macros_datadir=`$PKG_CONFIG --print-errors --variable=pkgdatadir xorg-macros`
>> -INSTALL_CMD="(cp -f "$macros_datadir/INSTALL" \$(top_srcdir)/.INSTALL.tmp && \
>> -mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \
>> -|| (rm -f \$(top_srcdir)/.INSTALL.tmp; touch \$(top_srcdir)/INSTALL; \
>> +INSTALL_CMD="(cp -f "$macros_datadir/INSTALL" \$(top_builddir)/.INSTALL.tmp && \
>> +mv \$(top_builddir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \
>
> aren't you still trying to create files in srcdir here? or is there
> something outside the context that makes it sufficient to have .INSTALL.tmp?
>
There's a, small, subtle difference in the rules:
INSTALL_CMD has an initial condition/command meaning "do we have
xorg-macros on the system" while the changelog one "is git around + is
foo/.git a git repo".

For the latter one can reasonably say - yes the command will succeed
_only_ on the first iteration of make dist, with any consecutive
iterations/build commands (remember it's a .PHONY thus it gets called
everytime we do make) will fall-back to the "else" statement.

While the part "mv builddir/foo.tmp srcdir/foo" seems a bit abusive
it's the most elegant solution that I can think of. Note the "touch"
in the else case is a no-op since the file should already be there.
Admittedly we can omit it and error out in the [extremely unlikely]
case that file is missing.

In the former (INSTALL) case, one _cannot_ expect builders to remove
xorg-macros.pc. Thus we end up overwriting the file (as seen in 2/2) a
bit too often. An alternative solution is something like the
following:

 - create builddir/FOO.tmp file
 - diff srcdir/FOO and builddir/FOO.tmp -> rm builddir/FOO.tmp if the
same, mv otherwise.
 - failed to create FOO.tmp -> rm builddir/FOO.tmp; touch srcdir/FOO
(we could consider the above comment, dropping touch and adding the
extra pedantic error if the file is missing)

The above logic can be applied for the ChangeLog rule as well...
albeit it won't make much difference in the end result.

I realise that things are a bit convoluted/confusing, it took me a few
runs to consider the possibilities and come with something that' not
too hacky.
I'd really appreciate if people can let me know how much/which of the
above/similar information would be appreciated - be that in commit
summary/inline comments/etc.

Thanks
Emil