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

[Peter Hutterer]

On Fri, Jan 27, 2017 at 02:32:48PM +0000, Emil Velikov wrote:
> 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".

the only time we care about generating install or changelog is during make
dist, and that's done in the worktree now and should assume a RO srcdir,
right? 

> 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.

it's a dependency of dist-hook though, at least in the server. so it'll only
get called on make dist.

> 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.

I think you're reading too much into this. all I'm suggesting is a
s/top_srcdir/top_builddir/ here :) which possibly maybe theoretically should
just work :)

Cheers,
   Peter

> 
> 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