Re: [PATCH] VPATH build of viewres, xedit, xfd, xgc, and xgc fails

[Peter Breitenlohner]

On Mon, 10 Nov 2008, Peter Breitenlohner wrote:

 There is, however, a radically different idea. Originally the app default
 file, e.g., Xmessage was distributed as Xmessage.ad for the sake of case
 insensitive filesystems in order not to cause conflict with the executable
 xmessage.  Now the application defaults are distributed in the subdirectory
 app-defaults and there is no reason not to distribute them under their final
 name.  That would certainly simplify the Makefile.ams.  The problem to
 create that subdirectory in the build tree would be restricted to cases such
 as xedit where either Xedit-print or Xedit-noprint is installed as Xedit.

And even that could be done by configure, e.g., for xedit:

if test x$enable_xprint = xyes; then
AC_CONFIG_LINKS([app-defaults/Xedit:app-defaults/Xedit-xprint])
else

AC_CONFIG_LINKS([app-defaults/Xedit:app-defaults/Xedit-noxprint])
fi

Regards,
Peter Breitenlohner [EMAIL PROTECTED]
___
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg

Re: [PATCH] VPATH build of viewres, xedit, xfd, xgc, and xgc fails

[Peter Breitenlohner]

On Sun, 9 Nov 2008, Peter Breitenlohner wrote:

 There is actually a much better way to avoid race conditions: whenever there
 are several app-default files, make them depend on a phony target that
 ensures the existence of ./app-defaults.  

The phony target I proposed yesterday was a bad idea. It certainly avoids
races but causes the files to be copied again and again. Also as part of
'make install' and that is really bad (root might have no write permissions
in the build tree (e.g., nfs filesystem with root_squashing).

the manual for Automake 1.10 says:

  If you are using Automake, there is normally no reason to call this
  macro, because `AM_INIT_AUTOMAKE' already does so.  However, make
  sure that the custom rules in your `Makefile's use `$(MKDIR_P)'
  and not `$(mkdir_p)'.  Even if both variables still work, the
  latter should be considered obsolete.

I suppose by now all developpers have at least automake-1.10 (released
2006-10-15), thus using $(MKDIR_P) should be safe.

There is, however, a radically different idea. Originally the app default
file, e.g., Xmessage was distributed as Xmessage.ad for the sake of case
insensitive filesystems in order not to cause conflict with the executable
xmessage.  Now the application defaults are distributed in the subdirectory
app-defaults and there is no reason not to distribute them under their final
name.  That would certainly simplify the Makefile.ams.  The problem to
create that subdirectory in the build tree would be restricted to cases such
as xedit where either Xedit-print or Xedit-noprint is installed as Xedit.

Regards
Peter Breitenlohner [EMAIL PROTECTED]
___
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg

Re: [PATCH] VPATH build of viewres, xedit, xfd, xgc, and xgc fails

[Peter Breitenlohner]

On Sat, 8 Nov 2008, Dan Nicholson wrote:


On Fri, Nov 7, 2008 at 12:49 AM, Peter Breitenlohner [EMAIL PROTECTED]
wrote:


Attached are five tiny patches. Could one of you you please apply them.


In my original message I had overlooked xmh with the same problem, patch is
attached.


In order to guard against races with parallel jobs, can you change to
`mkdir -p'? ...


Sorry, I missed this aspect (simply repeated what was already done for other
modules).

I should have said: Yes, of course. I overlooked this aspect 


How about simply ignoring errors resulting from mkdir, i.e., using
   -test -d app-defaults || mkdir app-defaults
That might produce spurious errors (ignored) for parallel jobs, but if mkdir
really fails (e.g., a regular file of that name already exists) the next
command 'cp $ $@' will fail.


Yeah, that'd be fine, too.


There is actually a much better way to avoid race conditions: whenever there
are several app-default files, make them depend on a phony target that
ensures the existence of ./app-defaults.  Attached is a patch implementing
this for xedit (where my original patch has already been applied).

Here a list of all app modules with that problem (more than one .ad
file):
bitmap
editres
xcalc
xclock
xditview
xedit
xlogo
xmessage

BTW: The only module with an .ad file still in the top-level directory is
oclock, but in that case there is no name conflict on case insensitive
filesystems.

Regards
Peter Breitenlohner [EMAIL PROTECTED]From a228bb1f92980c4c3b654fab188c4798b4b73dee Mon Sep 17 00:00:00 2001
From: Peter Breitenlohner [EMAIL PROTECTED]
Date: Sat, 8 Nov 2008 18:47:06 +0100
Subject: [PATCH] enabled VPATH build

---
 Makefile.am |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index f930a3c..9387cb3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -73,6 +73,7 @@ APPDEFAULTFILES = \
 SUFFIXES = .ad
 
 .ad:
+   test -d app-defaults || mkdir app-defaults
cp $ $@
 
 appdefault_DATA = $(APPDEFAULTFILES)
-- 
1.6.0.3

From 0687ace9392c5801929d81380ac03bd851514971 Mon Sep 17 00:00:00 2001
From: Peter Breitenlohner [EMAIL PROTECTED]
Date: Sun, 9 Nov 2008 14:24:10 +0100
Subject: [PATCH] avoid race condition for parallel jobs

---
 Makefile.am |   11 ---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1a32798..7c13340 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -167,20 +167,25 @@ APPDEFAULTFILES = \
 app-defaults/Xedit-color \
app-defaults/Xedit
 
+$(APPDEFAULTFILES): appsubdir
+
+.PHONY: appsubdir
+
+# Ensure ./app-defaults exists, avoiding race condition for parallel jobs
+appsubdir:
+   test -d app-defaults || mkdir app-defaults
+
 if USE_XPRINT
 app-defaults/Xedit.ad: 
-   test -d app-defaults || mkdir app-defaults
cp $(top_srcdir)/app-defaults/Xedit-xprint.ad app-defaults/Xedit.ad
 else
 app-defaults/Xedit.ad:
-   test -d app-defaults || mkdir app-defaults
cp $(top_srcdir)/app-defaults/Xedit-noxprint.ad app-defaults/Xedit.ad
 endif
 
 SUFFIXES = .ad 
 
 .ad:
-   test -d app-defaults || mkdir app-defaults
cp $ $@
 
 appdefaultdir = @appdefaultdir@
-- 
1.6.0.3

___
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg