On Fri, Jan 27, 2017 at 02:32:48PM +0000, Emil Velikov wrote: > On 27 January 2017 at 04:53, Peter Hutterer <[email protected]> wrote: > > On Thu, Jan 26, 2017 at 05:53:19PM +0000, Emil Velikov wrote: > >> From: Emil Velikov <[email protected]> > >> > >> 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 <[email protected]> > >> Cc: Gaetan Nadon <[email protected]> > >> Signed-off-by: Emil Velikov <[email protected]> > >> --- > >> 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
