[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Reworking the Apache package (with patches)



Hi Arno,

On Wednesday 07 December 2011, Stefan Fritsch wrote:
> On Monday 05 December 2011, Arno Töll wrote:
> > * I reworked the rules file entirely. It is now using the short
> > dh(1) syntax and (a lot of) overrides. Much magic still happens
> > in custom targets which are filed as dependencies to some
> > targets. Knowing the older rules you will recognize many
> > fragments of it. Some parts have also been moved to
> > package.debhelper.files where possible. I also removed most
> > DEB_BUILD_OPTIONS processing as dh mostly takes care.
> 
> Not sure about this one. Last time I talked to Joey H. about this,
> he recommended using the traditional debhelper style and not dh
> for packages that rebuild the source several times. But that was
> quite some time ago and dh has evolved a lot since then. I will
> have to look at the result in more detail.
> 
> Have you tried building with -j8? This gives a huge speed-up with
> the old package because all 4 configure runs are done in parallel.

The rules file looks quite good in general. But it runs the 4 MPM 
builds sequentially which causes the build time of the package to 
increase from 1:30 to 2:40 for me. I think this is annoying if one 
tries to debug some problem and has to rebuild the package several 
times.

An alternative may be to make override_dh_auto_configure, 
override_dh_auto_build, and override_dh_auto_install do basically 
nothing, put all the logic in a single "build-and-install-%" pattern 
rule and then build the four build-and-install-* targets with another 
make invocation that enables parallel build. I have attached a proof-
of-concept diff to illustrate the idea. This is not really beautiful 
but reduces the build time to 1:55. A bit more reduction should be 
possible by not copying the source tree for the non-itk MPMs.

Here are some more nitpick in addition to what I have written in the 
other mail:

The "@set -ex" calls are mostly useless, because they only affect the 
current shell. You would have to add a line continuation backslash to 
make them affect whatever script comes after.

The first line of the auto-install-% rules begins with spaces while it 
should be a tab.

The "cp debian/default-index.html ..." line is duplicate.

We don't need DH_VERBOSE=1 by default.

We can probably get rid of the config.sub/config.guess patch by using 
"dh --with autotools_dev". I just saw that in the man page, but 
haven't tried it. Would be nice, though.

If you want to have a verbose description in the rules file, you 
should probably mention in paragraph 2 that some develpment files are 
also taken from prefork mpm.

For the icons dir, "Options -Indexes -MultiViews" means "take whatever 
the Options were, but disable Indexes MultiViews". I think "Options 
FollowSymlinks", meaning "only enable FollowSymlinks", is more 
appropriate/predictable.

> > I didn't find any regressions and the new rules file seems to
> > produce the very same binary package, but you still might want to
> > double-check that.

I didn't find any unexpected differences either.

So, feel free to commit to svn. I don't mind if you fix the issues 
first, or commit the current state right away. You should also add 
yourself as Uploader.

Cheers,
Stefan
--- apache2-2.2.21/debian/rules	2011-12-04 02:21:36.000000000 +0100
+++ apache2-2.2.21par/debian/rules	2011-12-10 16:50:38.558905932 +0100
@@ -110,13 +110,7 @@
 	rm -rf $(INSTALL_DIR)
 	rm -f debian/tmp
 
-$(BUILD_DIR):
-	mkdir $(BUILD_DIR)
-
-$(INSTALL_DIR):
-	mkdir $(INSTALL_DIR)
-
-configure-%: $(BUILD_DIR)
+build-%:
 	@set -ex
 	mkdir "$(BUILD_DIR)/$*"
 	cp -a `find . -maxdepth 1 -mindepth 1 -not -name debian` $(BUILD_DIR)/$*
@@ -138,18 +132,15 @@
 	cd $(BUILD_DIR)/$* && ./configure \
 		$(AP2_COMMON_CONFARGS) --with-mpm=$* $(AP2_$(*)_CONFARGS) $(CONFFLAGS) \
 		CFLAGS="$(AP2_CFLAGS)" CPPFLAGS="$(AP2_CPPFLAGS)" LDFLAGS="$(AP2_LDFLAGS)"
-
-build-%:
 	dh_auto_build -D "$(BUILD_DIR)/$*"
 	cd "$(BUILD_DIR)/$*" && ./apache2 -l |grep -v $* > mods.list
-
-
-auto-install-%: $(INSTALL_DIR)
-        # force one process since mkdir.sh used by 'make install' is not
+	# force one process since mkdir.sh used by 'make install' is not
 	# reliable otherwise
 	dh_auto_install -D "$(BUILD_DIR)/$*" --destdir="$(INSTALL_DIR)/$*" -- -j1
+	install -m 755 $(BUILD_DIR)/$*/apache2 debian/apache2.2-bin/usr/lib/apache2/mpm-$*/apache2
+
 
-install-dev:
+install-dev: build-worker build-prefork
 	for i in worker prefork; do \
 		if [ "$$i" = "prefork" ]; then \
 			TARGET=prefork ;\
@@ -173,10 +164,6 @@
 		mv tmp_config_vars.mk config_vars.mk ) ; \
 	done
 
-install-%:
-	install -m 755 $(BUILD_DIR)/$*/apache2 debian/apache2.2-bin/usr/lib/apache2/mpm-$*/apache2
-
-
 undo-mpm-%-maintainer-scripts:
 	@set -ex
 	for f in postinst preinst prerm links dirs ; do \
@@ -191,24 +178,32 @@
 	done
 	perl -p -e "s/^/apache2-mpm-$*: /" < debian/mpms.lintian-overrides > debian/apache2-mpm-$*.lintian-overrides
 
+ifneq (,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))
+  NUMJOBS = $(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))
+  BUILDMAKEFLAGS = -j$(NUMJOBS)
+endif
 
 
 %:
 	dh $@ --parallel
 
-override_dh_auto_configure: prepare-custom-suexec $(patsubst %, configure-%, $(MPMS))
+override_dh_auto_configure: prepare-custom-suexec $(patsubst %, mpm-%-maintainer-scripts, $(MPMS))
+	mkdir $(BUILD_DIR)
+	mkdir $(INSTALL_DIR)
+
 
-override_dh_auto_build: $(patsubst %, mpm-%-maintainer-scripts, $(MPMS)) $(patsubst %, build-%, $(MPMS))
-	@set -ex
+override_dh_auto_build:
+
+override_dh_auto_install:
+
+override_dh_install:
+	$(MAKE) $(BUILDMAKEFLAGS) -f $(CURDIR)/debian/rules $(patsubst %, build-%, $(MPMS)) install-dev
 	for mpm in $(filter-out worker, $(MPMS)) ; do \
 		if ! diff -u $(BUILD_DIR)/$$mpm/mods.list $(BUILD_DIR)/worker/mods.list ; then \
 			echo Different modules built into httpd binaries, will not proceed ;\
 			exit 1 ;\
 		fi \
 	done
-
-
-override_dh_auto_install: $(patsubst %, auto-install-%, $(MPMS)) install-dev
 	cd debian && ln -s $(INSTALL_DIR_RELATIVE)/worker tmp
 	for m in logresolve ab; do d=debian/tmp/usr/share/man/ ;\
 		perl -p -e 's/^([.]TH.*?) 8 (.*)/$$1 1 $$2/' < $$d/man8/$$m.8 > $$d/man1/$$m.1 ;\
@@ -221,8 +216,6 @@
 	rm -f $(DEFAULT_MPM)/usr/sbin/apxs $(DEFAULT_MPM)/usr/sbin/apache2 debian/tmp/usr/sbin/apachectl
 	mv $(DEFAULT_MPM)/usr/share/man/man8/apxs.8 $(DEFAULT_MPM)/usr/share/man/man8/apxs2.8
 	mv $(DEFAULT_MPM)/usr/share/man/man8/apachectl.8 $(DEFAULT_MPM)/usr/share/man/man8/apache2ctl.8
-
-override_dh_install: $(patsubst %, install-%, $(MPMS))
 	dh_install --list-missing
 	# DO NOT FALL FOR THE TEMPTATION TO MV INTO PACKAGES OR DOOM
 	# WILL FIND YOU.  Use dh_install, this is just because dh_install

Attachment: signature.asc
Description: This is a digitally signed message part.


Reply to: