GNOME Bugzilla – Bug 587768
[patch] Don't build pango-view twice
Last modified: 2009-11-11 16:05:51 UTC
Please describe the problem: I'll attach build log and patch to this bug. Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information:
Created attachment 137849 [details] build log I'm hitting this race condition due to pango-view being built twice in parallel reliably with -j4 builds inside Scratchbox for the ARM Linux Internet Platform.
Created attachment 137851 [details] [review] fix for this bug The full description is in the patch. You can also pull it from the master branch of git://linux.onarm.com/git/generic/source/pango.git
Created attachment 137852 [details] [review] sorry, I attached the wrong version of this patch...
The reason the rule is written like that is such that the manual page is not rebuilt every time pango-view is built. That is, not unless the sources changed. I'll dig the automake manual to see what the correct idiom is in that case.
Why does it rebuild for you anyway? Cause your building from git I guess? The issue doesn't affect tarball builds.
And, are you disabling libtool's locking mechanism? Normally it should take care of it.
(In reply to comment #5) > Why does it rebuild for you anyway? Cause your building from git I guess? The > issue doesn't affect tarball builds. Yes, I'm building from git. Tarballs are not affected since they ship pango-view.1 (In reply to comment #6) > And, are you disabling libtool's locking mechanism? Normally it should take > care of it. No, I'm seeing the double build with something as simple as tar xzf pango-1.24.5.tar.gz cd pango-1.24.5 rm pango-view/pango-view.1 ./configure make -C pango make -j10 -C pango-view I'm getting the build errors only inside Scratchbox (whether it breaks or builds everything just fine twice looks timing related and therefore kinda random), but the double building is also visible outside.
*** Bug 600207 has been marked as a duplicate of this bug. ***
Ok, I think I fixed this properly in master. Please test. For reference, here's what I did: MAINTAINERCLEANFILES = pango-view.1 pango-view.stamp EXTRA_DIST += pango-view.stamp dist_man_MANS = pango-view.1 # The indirection through pango-view.stamp is to make parallel build work. # See bug 587768. pango-view.stamp: ../configure.in $(pango_view_SOURCES) $(AM_V_GEN) X="$(srcdir)/pango-view.1"; \ $(top_builddir)/missing --run \ help2man --no-info --section=1 \ --help-option="--help-all" --output="$$X.tmp" \ --name 'Pango text viewer' ./pango-view \ && mv "$$X.tmp" "$$X" && touch $@ \ || (echo Failed to update pango-view.1, the man page may be outdated >&2; \ (test -f "$$X" || echo help2man is required to generate this file. >> "$$X")); $(srcdir)/pango-view.1: pango-view$(EXEEXT) pango-view.stamp $(AM_V_GEN) if test -f $@; then touch $@; else \ $(RM) pango-view.stamp; \ $(MAKE) $(AM_MAKEFLAGS) pango-view.stamp; \ fi
(In reply to comment #9) > Ok, I think I fixed this properly in master. Please test. Haven't tested, but I don't think this is a solution. pango-view.1 depends on pango-view.stamp, so the MAKE invocation is superfluous, as it will never get executed. Furthermore, because pango-view.stamp uses pango-view but doesn't depend on it, you still get a race condition for parallel builds. I guess you don't want to list pango-view.stamp as a prerequisite of pango-view.1. If you drop it, however, you probably want to make pango-view.stamp unconditionally, so that the check for updated sources is always performed. The result leads to the following simple rule: $(srcdir)/pango-view.1: pango-view$(EXEEXT) $(MAKE) $(AM_MAKEFLAGS) pango-view.stamp However, this still won't work with the shipped pango-view.1, because you don't ship a pango-view.stamp. So building from tarball would still require you to rebuild the man page. Let's analyse this: 1. We want pango-view.1 updated only if its older than one of the sources 2. In order to update it, we need pango-view$(EXEEXT) 3. We can't build pango-view inside a recursive call, as it would break parallel builds How about this: $(srcdir)/pango-view.1.in: ../configure.in $(pango_view_SOURCES) $(AM_V_GEN) $(top_builddir)/missing --run \ help2man --no-info --section=1 \ --help-option="--help-all" --output="$@.tmp" \ --name 'Pango text viewer' ./pango-view \ && mv "$@.tmp" "$@" \ || ($(RM) "$@"; \ echo Failed to update pango-view.1, the man page may be outdated >&2; \ (test -f "$@" || echo help2man is required to generate this file. >> "$@")); pango-view.1: pango-view$(EXEEXT) $(MAKE) $(AM_MAKEFLAGS) $(srcdir)/pango-view.1.in cp $(srcdir)/pango-view.1.in $@ The idea is to build a separate target in the source directory, which then gets shipped in the tarball. It is copied to the build directory in the pango-view.1 rule, where the dependency on pango-view$(EXEEXT) ensures that whenever the manpage is regenerated in this way, the executable does already exist. I'm not experienced enough with autotools to figure out how I could convice it to ship the pango-view.1.in but still install the pango-view.1 man page. Do you know?
(In reply to comment #10) > I'm not experienced enough with autotools Why don't you just test what I've pushed then? I know what I'm doing :). > to figure out how I could convice it to ship the pango-view.1.in but still > install the pango-view.1 man page. Do you know? The code I pasted above ships pango-view.stamp as well as pango-view.1
(In reply to comment #10) > Furthermore, because pango-view.stamp uses pango-view but doesn't > depend on it, you still get a race condition for parallel builds. Right, I totally missed this part. Lemme think about it a bit more.
(In reply to comment #10) > How about this: > > $(srcdir)/pango-view.1.in: ../configure.in $(pango_view_SOURCES) > $(AM_V_GEN) $(top_builddir)/missing --run \ > help2man --no-info --section=1 \ > --help-option="--help-all" --output="$@.tmp" \ > --name 'Pango text viewer' ./pango-view \ > && mv "$@.tmp" "$@" \ > || ($(RM) "$@"; \ > echo Failed to update pango-view.1, the man page may be outdated >&2; > \ > (test -f "$@" || echo help2man is required to generate this file. >> > "$@")); > > pango-view.1: pango-view$(EXEEXT) > $(MAKE) $(AM_MAKEFLAGS) $(srcdir)/pango-view.1.in > cp $(srcdir)/pango-view.1.in $@ I like this. Pushed something similar out.