Bug#577804: apt: debian/rules improvements -- EOL whitespace, long options, POSIX cmd substitution
David Kalnischkies <kalnischkies+debian@gmail.com> writes:
>> against 2010-03-26 r1964 http://bzr.debian.org/apt/debian-sid
>
> 1.) Why do you convert commented-out lines intend from tabs to spaces?
A leffover from editor. Fixed.
> 2.) change at line 123 regarding Makefile isn't a valid replacement.
> You replace a check for the Makefile before running the targets to
> checking for Makefile - OR if it doesn't exist run make targets…
That's a typo. Fixed to
[ ! -f makefile ] || ....
> 3.) line 225: APT doesn't ship all his manpages in the same package,
> so your removal of the parameter for dh_installman is wrong.
A typo. Fixed.
> 4.) The two last lines are separated with a newline from the rest of
> the target - that is wrong. Also, they are separated commands,
> so adding \ feels strange and conflicts with your own style.
In the original code they were placed on the same line, so I supposed
they were related. To take a closer look, they weren't.
Fixed to separate lines.
> I have not looked at each line, so it maybe includes also better
> hidden buggers ~ could you redo your patch?
Attached.
Jari
>From e4e65baa6a1deefab91db17d1bfde4bbbaf0c5d0 Mon Sep 17 00:00:00 2001
From: Jari Aalto <jari.aalto@cante.net>
Date: Fri, 16 Apr 2010 20:42:55 +0300
Subject: [PATCH] debian/rules: use --long options and POSIX command substitution
Organization: Private
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
- Spell out the meaning of the command by using --long style options
where possible. With long options, manual pages need to be consulted
less frequently.
- Remove subshell calls in simple lines, like in "(cd DIR; cmd)".
- Utilize GNU tar option --directory in cases like "cd DIR; tar ..."
- Use POSIX command substitution: $(<command sequence>)
- Put redirections like '>' at the end of line
- Remove EOL whitespace
Signed-off-by: Jari Aalto <jari.aalto@cante.net>
---
rules | 100 +++++++++++++++++++++++++++++++++++++----------------------------
1 files changed, 57 insertions(+), 43 deletions(-)
diff --git a/rules b/rules
index 2fe6ed6..e85eb88 100755
--- a/rules
+++ b/rules
@@ -47,7 +47,7 @@ BASE=.
ifdef BUILD
BUILD_POSSIBLE := $(BUILD) $(BASE)/$(BUILD)
else
-BUILD_POSSIBLE := $(BASE) $(BASE)/build-$(shell uname -m) $(BASE)/build
+BUILD_POSSIBLE := $(BASE) $(BASE)/build-$(shell uname --machine) $(BASE)/build
endif
BUILDX:= $(foreach i,$(BUILD_POSSIBLE),$(wildcard $(i)/environment.mak*))
BUILDX:= $(patsubst %/,%,$(firstword $(dir $(BUILDX))))
@@ -78,14 +78,14 @@ APT_UTILS=ftparchive sortpkgs extracttemplates
include buildlib/libversion.mak
# Determine which package we should provide in the control files
-LIBAPTPKG_PROVIDE=libapt-pkg$(LIBAPTPKG_MAJOR)
+LIBAPTPKG_PROVIDE= libapt-pkg$(LIBAPTPKG_MAJOR)
LIBAPTINST_PROVIDE=libapt-inst$(LIBAPTINST_MAJOR)
debian/shlibs.local: apt-pkg/makefile
# We have 3 shlibs.local files.. One for 'apt', one for 'apt-utils' and
# one for the rest of the packages. This ensures that each package gets
- # the right overrides..
- rm -rf $@ $@.apt $@.apt-utils
+ # the right overrides..
+ rm --recursive --force $@ $@.apt $@.apt-utils
echo "libapt-pkg $(LIBAPTPKG_MAJOR)" > $@.apt
echo "libapt-pkg $(LIBAPTPKG_MAJOR) $(LIBAPTPKG_PROVIDE)" > $@.apt-utils
@@ -94,8 +94,8 @@ debian/shlibs.local: apt-pkg/makefile
echo "libapt-pkg $(LIBAPTPKG_MAJOR) $(LIBAPTPKG_PROVIDE)" > $@
echo "libapt-inst $(LIBAPTINST_MAJOR) $(LIBAPTINST_PROVIDE)" >> $@
-build: build/build-stamp
-build-doc: build/build-doc-stamp
+build: build/build-stamp
+build-doc: build/build-doc-stamp
# Note that this is unconditionally done first as part of loading environment.mak
# The true is needed to force make to reload environment.mak after running
@@ -106,7 +106,7 @@ build/environment.mak: build/configure-stamp
configure: configure.in
build/configure-stamp: configure
dh_testdir
- -mkdir build
+ mkdir --parents build
cp COPYING debian/copyright
cd build && CXXFLAGS="$(confcxxflags)" ../configure $(confflags)
touch $@
@@ -124,10 +124,10 @@ build/build-doc-stamp: build/configure-stamp
clean:
dh_testdir
# dh_testroot
- [ -f Makefile ] && $(MAKE) clean
- [ -f Makefile ] && $(MAKE) distclean
- rm -rf build
+ [ ! -f Makefile ] || $(MAKE) clean distclean
+
+ rm --recursive --force build
# Add here commands to clean up after the build process.
dh_clean debian/copyright debian/shlibs.local debian/shlibs.local.apt debian/shlibs.local.apt-utils
@@ -141,10 +141,16 @@ libapt-pkg-doc: build-doc debian/shlibs.local
dh_installdirs -p$@
#
# libapt-pkg-doc install
-#
- dh_installdocs -p$@ $(BLD)/docs/cache* $(BLD)/docs/design* $(BLD)/docs/dpkg-tech* \
- $(BLD)/docs/files* $(BLD)/docs/method* \
- doc/libapt-pkg2_to_3.txt doc/style.txt
+#
+ dh_installdocs -p$@ \
+ $(BLD)/docs/cache* \
+ $(BLD)/docs/design* \
+ $(BLD)/docs/dpkg-tech* \
+ $(BLD)/docs/files* \
+ $(BLD)/docs/method* \
+ doc/libapt-pkg2_to_3.txt \
+ doc/style.txt
+
dh_installexamples -p$@
# dh_installmenu -p$@
# dh_installinit -p$@
@@ -170,8 +176,12 @@ apt-doc: build-doc
# apt-doc install
#
# Copy the guides
- dh_installdocs -p$@ $(BLD)/docs/guide*.text $(BLD)/docs/guide*.html \
- $(BLD)/docs/offline*.text $(BLD)/docs/offline*.html
+ dh_installdocs -p$@ \
+ $(BLD)/docs/guide*.text \
+ $(BLD)/docs/guide*.html \
+ $(BLD)/docs/offline*.text \
+ $(BLD)/docs/offline*.html
+
dh_installchangelogs -p$@
dh_compress -p$@
dh_fixperms -p$@
@@ -208,21 +218,21 @@ apt: build build-doc debian/shlibs.local
rm debian/$@/usr/lib/apt/methods/https
cp $(BLD)/scripts/dselect/* debian/$@/usr/lib/dpkg/methods/apt/
- cp -r $(BLD)/locale debian/$@/usr/share/
+ cp --recursive $(BLD)/locale debian/$@/usr/share/
cp debian/bugscript debian/$@/usr/share/bug/apt/script
cp debian/apt.logrotate debian/$@/etc/logrotate.d/apt
cp debian/apt.conf.autoremove debian/$@/etc/apt/apt.conf.d/01autoremove
-# head -n 500 ChangeLog > debian/ChangeLog
+# head --lines=500 ChangeLog > debian/ChangeLog
# copy lintian override
cp share/lintian-overrides debian/$@/usr/share/lintian/overrides/apt
# make rosetta happy and remove pot files in po/ (but leave stuff
# in po/domains/* untouched) and cp *.po into each domain dir
- rm -f build/po/*.pot
- rm -f po/*.pot
+ rm --force build/po/*.pot
+ rm --force po/*.pot
dh_installexamples -p$@ $(BLD)/docs/examples/*
dh_installman -p$@ $(wildcard $(patsubst %,doc/%.[158],$(apt_MANPAGES)) $(patsubst %,doc/*/%.*.[158],$(apt_MANPAGES)))
@@ -234,7 +244,7 @@ apt: build build-doc debian/shlibs.local
dh_fixperms -p$@
dh_makeshlibs -p$@ -m$(LIBAPTPKG_MAJOR) -V '$(LIBAPTPKG_PROVIDE)'
dh_installdeb -p$@
- dh_shlibdeps -p$@ -l`pwd`/debian/apt/usr/lib:`pwd`/debian/$@/usr/lib -- -Ldebian/shlibs.local.apt
+ dh_shlibdeps -p$@ -l$(pwd)/debian/apt/usr/lib:$(pwd)/debian/$@/usr/lib -- -Ldebian/shlibs.local.apt
dh_gencontrol -p$@ -u -Vlibapt-pkg:provides=$(LIBAPTPKG_PROVIDE)
dh_md5sums -p$@
dh_builddeb -p$@
@@ -273,7 +283,6 @@ apt-utils: build debian/shlibs.local
dh_testdir -p$@
dh_testroot -p$@
dh_clean -p$@ -k
- dh_installdirs -p$@
# install the shared libs
find $(BLD)/bin/ -type f -name "libapt-inst*.so.*" -exec cp -a "{}" debian/$@/usr/lib/ \;
@@ -290,9 +299,9 @@ apt-utils: build debian/shlibs.local
dh_strip -p$@
dh_compress -p$@
dh_fixperms -p$@
- dh_makeshlibs -m$(LIBAPTINST_MAJOR) -V '$(LIBAPTINST_PROVIDE)' -p$@
+ dh_makeshlibs --major=$(LIBAPTINST_MAJOR) --version-info='$(LIBAPTINST_PROVIDE)' -p$@
dh_installdeb -p$@
- dh_shlibdeps -p$@ -l`pwd`/debian/apt/usr/lib:`pwd`/debian/$@/usr/lib -- -Ldebian/shlibs.local.apt-utils
+ dh_shlibdeps -p$@ -l$(pwd)/debian/apt/usr/lib:$(pwd)/debian/$@/usr/lib -- -Ldebian/shlibs.local.apt-utils
dh_gencontrol -p$@ -u -Vlibapt-inst:provides=$(LIBAPTINST_PROVIDE)
dh_md5sums -p$@
dh_builddeb -p$@
@@ -304,7 +313,7 @@ apt-transport-https: build debian/shlibs.local libapt-pkg-dev
dh_installdirs -p$@
# install the method
- mkdir -p debian/$@/usr/lib/apt/methods
+ mkdir --parents debian/$@/usr/lib/apt/methods
cp $(BLD)/bin/methods/https debian/$@/usr/lib/apt/methods
dh_installdocs -p$@ debian/apt-transport-https.README
@@ -318,13 +327,14 @@ apt-transport-https: build debian/shlibs.local libapt-pkg-dev
dh_compress -p$@
dh_fixperms -p$@
dh_installdeb -p$@
- dh_shlibdeps -p$@ -l`pwd`/debian/apt/usr/lib:`pwd`/debian/$@/usr/lib
+ dh_shlibdeps -p$@ -l$(pwd)/debian/apt/usr/lib:$(pwd)/debian/$@/usr/lib
dh_gencontrol -p$@
dh_md5sums -p$@
dh_builddeb -p$@
source diff:
- @echo >&2 'source and diff are obsolete - use dpkg-source -b'; false
+ @echo 'source and diff are obsolete - use dpkg-source -b' >&2
+ false
# Update from CVS
l33ch: really-clean
@@ -341,16 +351,15 @@ l33ch-stamp: super-l33ch
touch $@
really-clean: clean
- -find -name Makefile.in -print0 | xargs -0r rm -f
- find -name ChangeLog | xargs rm -f
- rm -f l33ch-stamp
+ find . -name Makefile.in -print0 | xargs --null --no-run-if-empty rm --force
+ find . -name ChangeLog | xargs --no-run-if-empty rm --force
+ rm --force l33ch-stamp
binary: binary-indep binary-arch
.PHONY: build clean binary-indep binary-arch binary debian/shlibs.local
# Done by the uploader.
-#cvs update..
#edit debian/changelog
# configure.in has the version automatically updated now.
# edit configure.in
@@ -360,19 +369,24 @@ CVS_BUILDDIR=apt-$(APT_DEBVER)
CVS_ROOT=$(shell cat CVS/Root)
CVS_MODULE=$(shell cat CVS/Repository)
cvs-build:
- rm -rf debian/cvs-build
- mkdir -p debian/cvs-build
- (cd debian/cvs-build;cvs -d $(CVS_ROOT) export -r$(APT_CVSTAG) -d apt-$(APT_DEBVER) $(CVS_MODULE))
- $(MAKE) -C debian/cvs-build/$(CVS_BUILDDIR) startup doc
- (cd debian/cvs-build/$(CVS_BUILDDIR);$(DEB_BUILD_PROG))
+ rm --recursive --force debian/cvs-build
+ mkdir --parents debian/cvs-build
+ (cd debian/cvs-build; cvs -d $(CVS_ROOT) export -r$(APT_CVSTAG) -d apt-$(APT_DEBVER) $(CVS_MODULE))
+ $(MAKE) --directory=debian/cvs-build/$(CVS_BUILDDIR) startup doc
+ (cd debian/cvs-build/$(CVS_BUILDDIR); $(DEB_BUILD_PROG))
cvs-mkul:
- -mkdir -p ../upload-$(APT_DEBVER)
- cp `find debian/cvs-build -maxdepth 1 -type f` ../upload-$(APT_DEBVER)
+ mkdir --parents ../upload-$(APT_DEBVER)
+ cp $(find debian/cvs-build -maxdepth 1 -type f) ../upload-$(APT_DEBVER)
arch-build:
- rm -rf debian/arch-build
- mkdir -p debian/arch-build/apt-$(APT_DEBVER)
- tar -c --exclude=arch-build --no-recursion -f - `bzr inventory` | (cd debian/arch-build/$(PKG)-$(APT_DEBVER);tar xf -)
- $(MAKE) -C debian/arch-build/apt-$(APT_DEBVER) startup doc
- (cd debian/arch-build/apt-$(APT_DEBVER); $(DEB_BUILD_PROG); dpkg-genchanges -S > ../apt_$(APT_DEBVER)_source.changes)
+ rm --recursive --force debian/arch-build
+ mkdir --parents debian/arch-build/apt-$(APT_DEBVER)
+
+ tar --exclude=arch-build --no-recursion --create --file - $(bzr inventory) | \
+ tar --directory debian/arch-build/$(PKG)-$(APT_DEBVER) -x --file -
+
+ $(MAKE) --directory=debian/arch-build/apt-$(APT_DEBVER) startup doc
+
+ cd debian/arch-build/apt-$(APT_DEBVER); $(DEB_BUILD_PROG)
+ dpkg-genchanges -S > ../apt_$(APT_DEBVER)_source.changes
--
1.7.0
Reply to: