Comments on module packaging

To: Victor Seva <linuxmaniac@torreviejawireless.org>
Cc: debian-kernel@lists.debian.org
Subject: Comments on module packaging
From: Jonas Smedegaard <dr@jones.dk>
Date: Wed, 9 Nov 2005 03:02:35 +0100
Message-id: <20051109030235.5448483d.dr@jones.dk>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Victor (And cc kernel team),

As promised on #debian-kernel is here some comments on your latest work
on linux-wlan-ng packaging.

Actually, What I primarily wanted to comment on, I now realize why you
didn't really understand on IRC: I insisted that the package use exact
same build rules for compiling modules as part of the packaging as is
used when a user compiles the modules. What I now realize is that this
is _exactly_ what you do already. You just do it in a very few lines of
shell code where I expected it couldn't be done without enough code
that I would stumble on it at a quick glance. Sorry - and thumbs up!

I have also added the above to the plan[1] (and made other major
changes so go read it again...). I promised Manoj later yesterday to
more generously add comments about the reasoning behind the various
points in there - I'll do that, but also invite all of you to go ahead
and add comments for the parts you know about (don't mind doing it
messy: I prefer cleaning up messy contributions to writing all
myself :-) ).


Looking through the code I did find some things I didn't like. A patche
is attached here. But I see nothing wrong in the overall build
structure. Also my other point on IRC about making it possible to build
single flavours only was bogus. So all in all it looks to me that you
are doing a great job, Victor :-)


I still want to do a cdbs snippet for kernel-module specific packaging
rules, but that's another story...


Oh, and one final comment: The package seems broken - fails in a pbuild
run with this (same error both with and without my patch applied):

mv: cannot stat `debian/modules-build-powerpc/modules/*.deb': No such
file or directory

But I guess that's just because the package is work-in-progress, right?


 - Jonas

[1] http://wiki.debian.org/KernelModulesPackaging

- -- 
* Jonas Smedegaard - idealist og Internet-arkitekt
* Tlf.: +45 40843136  Website: http://dr.jones.dk/

 - Enden er nær: http://www.shibumi.org/eoti.htm
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQFDcVi7n7DbMsAkQLgRAiBaAJ4s9WeYAyIS4cjFNgj5BpQuBFXsrACfQACE
rIrNRo49/jo8OrTZ30aQ8jM=
=JmmI
-----END PGP SIGNATURE-----

diff -ruN debian/changelog /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/changelog
--- debian/changelog	2005-11-08 13:51:44.000000000 +0100
+++ /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/changelog	2005-11-08 17:35:05.000000000 +0100
@@ -1,3 +1,15 @@
+linux-wlan-ng (0.2.2+dfsg-5.0.jones1) unstable; urgency=low
+
+  * Improve helper scripts fetch-and-clean-upstream-from-hex.sh and
+    linux-wlan-ng-build-firmware-deb:
+    + Fix downloading firmware with curl.
+    + Quote strings.
+  * Fix misspellings of prism2 (not prims2).
+  * Fail (not only warn) if any *.hex (not just one test) is in source.
+  * Silence comments in rule files (by not indenting them).
+
+ -- Jonas Smedegaard <dr@jones.dk>  Tue,  8 Nov 2005 13:57:10 +0100
+
 linux-wlan-ng (0.2.2+dfsg-5) unstable; urgency=low
 
   * the -source package now produces modules that do not depend on debian
diff -ruN debian/fetch-and-clean-upstream-from-hex.sh /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/fetch-and-clean-upstream-from-hex.sh
--- debian/fetch-and-clean-upstream-from-hex.sh	2005-11-08 13:51:44.000000000 +0100
+++ /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/fetch-and-clean-upstream-from-hex.sh	2005-11-08 17:30:33.000000000 +0100
@@ -22,18 +22,18 @@
 
 #set -x
 
-BASEURL=ftp://ftp.linux-wlan.org/pub/linux-wlan-ng/
-BASENAME=linux-wlan-ng-
-EXTENSION=.tar.gz
+BASEURL"=ftp://ftp.linux-wlan.org/pub/linux-wlan-ng/";
+BASENAME="linux-wlan-ng-"
+EXTENSION=".tar.gz"
 RC=""
 
 function tempdir {
-	local NAME=/tmp/linux-wlan-ng-$1-`date +%s`
-	mkdir $NAME
+	local NAME"=/tmp/linux-wlan-ng-$1-`date +%s`"
+	mkdir "$NAME"
 	if [ $? != 0 ]; then
 		return 1
 	else
-		RC=$NAME
+		RC="$NAME"
 		return 0
 	fi
 }
@@ -41,83 +41,84 @@
 # $1: the version you want to fetch
 function fetch {
 	local DIR=""
-	local FILENAME=$BASENAME$1$EXTENSION
-	local URL=$BASEURL$FILENAME
+	local FILENAME="$BASENAME$1$EXTENSION"
+	local URL="$BASEURL$FILENAME"
 	local DOWNLOADER=""
 
 	tempdir fetch
 	if [ $? = 0 ]; then
-		DIR=$RC
+		DIR="$RC"
 	else
 		echo "Unable to create a temporary directory"
 		return 1
 	fi
 	if [ ! -z "`which wget`" ]; then
-		DOWNLOADER=wget
+		DOWNLOADER="wget"
 	elif [ ! -z "`which curl`" ]; then
-		DOWNLOADER=curl
+		#FIXME: options are not quoted when $DOWNLAODER is used!
+		DOWNLOADER="curl -o $FILENAME"
 	else
 		echo "I need wget or curl to fetch the upstream tarball"
-		rm -rf $DIR
+		rm -rf "$DIR"
 		return 1
 	fi
-	cd $DIR
-	$DOWNLOADER $URL
+	cd "$DIR"
+	$DOWNLOADER "$URL"
 	if [ $? != 0 ]; then
 		echo "$DOWNLOADER returned an error"
-		rm -rf $DIR
+		rm -rf "$DIR"
 		cd -
 		return 1
 	fi
 	cd - >&/dev/null
-	RC=$DIR/$FILENAME
+	RC="$DIR/$FILENAME"
 	return 0
 } 
 
 # $1: the tarball
 function remove-hex {
 	local DIR=""
-	local FILENAME=`basename $1`
+	local FILENAME="`basename "$1"`"
 	
 	tempdir remove-hex
 	if [ $? = 0 ]; then
-		DIR=$RC
+		DIR="$RC"
 	else
 		echo "Unable to create a temporary directory"
 		return 1
 	fi
 	
-	tar -xzf "$1" -C $DIR
+	tar -xzf "$1" -C "$DIR"
 	if [ $? != 0 ]; then
 		echo "Unable to untar original file"
-		rm -rf $DIR
+		rm -rf "$DIR"
 		return 1
 	fi
 	
-	find $DIR -name \*.hex -exec rm \{\} \;
+	find "$DIR" -name \*.hex -exec rm \{\} \;
 	if [ $? != 0 ]; then
 		echo "Unable to remove hex files"
-		rm -rf $DIR
+		rm -rf "$DIR"
 		return 1
 	fi
 
-	cd $DIR
-	tar -czf $DIR/$FILENAME *
+	cd "$DIR"
+	tar -czf "$DIR/$FILENAME" *
 	if [ $? != 0 ]; then
 		echo "Unable to repack cleaned sources"
-		rm -rf $DIR
+		rm -rf "$DIR"
 		cd -
 		return 1
 	fi
 	cd - >&/dev/null
 
-	RC=$DIR/$FILENAME
+	RC="$DIR/$FILENAME"
 	return 0
 }
 
 if [ -z "$1" ]; then
 	echo 
-	echo "usage: `basename $0` version"
+	echo "usage: `basename "$0"` version"
 	echo
 	echo "Will produce the following files in the current directory:"
 	echo -e "\tlinux-wlan-ng-version.tar.gz"
@@ -126,20 +127,20 @@
 	echo
 	exit 1
 fi
-VERSION=$1
+VERSION="$1"
 
-fetch $VERSION
+fetch "$VERSION"
 if [ $? != 0 ]; then
 	exit 1
 fi
-UPSTREAM=$RC
-remove-hex $RC
+UPSTREAM="$RC"
+remove-hex "$RC"
 if [ $? != 0 ]; then
 	exit 2
 fi
-CLEANED=$RC
-cp $UPSTREAM $BASENAME$VERSION$EXTENSION
-cp $CLEANED linux-wlan-ng_$VERSION.orig$EXTENSION
-rm -rf `dirname $UPSTREAM` `dirname $CLEANED`
+CLEANED="$RC"
+cp "$UPSTREAM" "$BASENAME$VERSION$EXTENSION"
+cp "$CLEANED" "linux-wlan-ng_$VERSION.orig$EXTENSION"
+rm -rf "`dirname "$UPSTREAM"`" "`dirname "$CLEANED"`"
 
 #eof
diff -ruN debian/linux-wlan-ng-build-firmware-deb /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/linux-wlan-ng-build-firmware-deb
--- debian/linux-wlan-ng-build-firmware-deb	2005-11-08 13:51:44.000000000 +0100
+++ /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/linux-wlan-ng-build-firmware-deb	2005-11-08 17:32:53.000000000 +0100
@@ -19,23 +19,23 @@
 #
 # See /usr/share/common-licenses/GPL for the full text of the GPL.
 
-VERSION=@@VERSION@@
+VERSION="@@VERSION@@"
 
 #set -x
 
-BASEURL=ftp://ftp.linux-wlan.org/pub/linux-wlan-ng/
-BASENAME=linux-wlan-ng-
-EXTENSION=.tar.gz
+BASEURL="ftp://ftp.linux-wlan.org/pub/linux-wlan-ng/";
+BASENAME="linux-wlan-ng-"
+EXTENSION=".tar.gz"
 RC=""
-DEBIAN=/usr/share/linux-wlan-ng-firmware/debian
+DEBIAN="/usr/share/linux-wlan-ng-firmware/debian"
 
 function tempdir {
-	local NAME=/tmp/linux-wlan-ng-$1-`date +%s`
-	mkdir $NAME
+	local NAME="/tmp/linux-wlan-ng-$1-`date +%s`"
+	mkdir "$NAME"
 	if [ $? != 0 ]; then
 		return 1
 	else
-		RC=$NAME
+		RC="$NAME"
 		return 0
 	fi
 }
@@ -43,36 +43,37 @@
 # $1: the version you want to fetch
 function fetch {
 	local DIR=""
-	local FILENAME=$BASENAME$1$EXTENSION
-	local URL=$BASEURL$FILENAME
+	local FILENAME="$BASENAME$1$EXTENSION"
+	local URL="$BASEURL$FILENAME"
 	local DOWNLOADER=""
 
 	tempdir fetch
 	if [ $? = 0 ]; then
-		DIR=$RC
+		DIR="$RC"
 	else
 		echo "Unable to create a temporary directory"
 		return 1
 	fi
 	if [ ! -z "`which curl`" ]; then
-		DOWNLOADER=curl
+		# FIXME: options are unquoted when $DOWNLOADER is used
+		DOWNLOADER="curl -o $FILENAME"
 	elif [ ! -z "`which wget`" ]; then
-		DOWNLOADER=wget
+		DOWNLOADER="wget"
 	else
 		echo "I need wget or curl to fetch the upstream tarball"
-		rm -rf $DIR
+		rm -rf "$DIR"
 		return 1
 	fi
-	cd $DIR
-	$DOWNLOADER $URL
+	cd "$DIR"
+	$DOWNLOADER "$URL"
 	if [ $? != 0 ]; then
 		echo "$DOWNLOADER returned an error"
-		rm -rf $DIR
+		rm -rf "$DIR"
 		cd -
 		return 1
 	fi
 	cd - >&/dev/null
-	RC=$DIR/$FILENAME
+	RC="$DIR/$FILENAME"
 	return 0
 } 
 
@@ -86,25 +87,25 @@
 	exit 1
 fi
 
-OLDDIR=`pwd`
-UPSTREAM_VERSION=`echo $VERSION | cut -d - -f 1 | cut -d + -f 1`
+OLDDIR="`pwd`"
+UPSTREAM_VERSION="`echo "$VERSION" | cut -d - -f 1 | cut -d + -f 1`"
 
-fetch $UPSTREAM_VERSION
+fetch "$UPSTREAM_VERSION"
 if [ $? != 0 ]; then
 	exit 1
 fi
 
-UPSTREAM=$RC
-DIRNAME=`dirname $UPSTREAM`
-cd $DIRNAME
-tar -xzf `basename $UPSTREAM`
-cd $BASENAME$UPSTREAM_VERSION
-cp -r $DEBIAN .
+UPSTREAM="$RC"
+DIRNAME="`dirname "$UPSTREAM"`"
+cd "$DIRNAME"
+tar -xzf "`basename "$UPSTREAM"`"
+cd "$BASENAME$UPSTREAM_VERSION"
+cp -r "$DEBIAN" .
 chmod a+rx debian/rules
 dpkg-buildpackage -rfakeroot -us -uc
 cd ..
-mv linux-wlan-ng-firmware-files_${VERSION}_all.deb $OLDDIR
-cd $OLDDIR
-rm -rf `dirname $UPSTREAM`
+mv "linux-wlan-ng-firmware-files_${VERSION}_all.deb" "$OLDDIR"
+cd "$OLDDIR"
+rm -rf "`dirname "$UPSTREAM"`"
 
 #eof
diff -ruN debian/patches/DOC_man_prism2dl_1.dpatch /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/patches/DOC_man_prism2dl_1.dpatch
--- debian/patches/DOC_man_prism2dl_1.dpatch	2005-11-08 13:51:44.000000000 +0100
+++ /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/patches/DOC_man_prism2dl_1.dpatch	2005-11-08 14:01:15.000000000 +0100
@@ -11,11 +11,11 @@
 @@ -0,0 +1,93 @@
 +.TH PRISM2DL 8
 +.SH NAME
-+prims2dl \- 802.11 frame dump utility
++prism2dl \- 802.11 frame dump utility
 +.SH SYNOPSIS
 +.B prism2dl [OPTIONS] devname
 +.SH DESCRIPTION
-+.B prims2dl
++.B prism2dl
 +User utility for downloading prism2 images
 +.SH OPTIONS
 +.TP
diff -ruN debian/patches/SRC_src_prism2_driver_prism2_cs_c.dpatch /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/patches/SRC_src_prism2_driver_prism2_cs_c.dpatch
--- debian/patches/SRC_src_prism2_driver_prism2_cs_c.dpatch	2005-11-08 13:51:44.000000000 +0100
+++ /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/patches/SRC_src_prism2_driver_prism2_cs_c.dpatch	2005-11-08 14:02:35.000000000 +0100
@@ -16,7 +16,7 @@
 +#if (LINUX_VERSION_CODE <= KERNEL_VERSION(2,4,27))
 +MODULE_PARM( irq_mask, "i");
 +MODULE_PARM( irq_list, "1-4i");
-+MODULE_PARM( prims2_ignorevcc, "i");
++MODULE_PARM( prism2_ignorevcc, "i");
 +#else
 +#if (LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,8))
  static int numlist = 4;
diff -ruN debian/rules /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/rules
--- debian/rules	2005-11-08 13:51:44.000000000 +0100
+++ /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/rules	2005-11-09 02:35:20.000000000 +0100
@@ -106,24 +106,25 @@
 		$(TMPDIR)/modules/$(LWNS)/debian
 	install -m755 debian/rules.modules \
 		$(TMPDIR)/modules/$(LWNS)/debian/rules
-	# rm firmware files, but THEY SHOULD NOT BE THERE!!!!
-	if test -e src/prism2/af010104.hex; then \
+# fail if firmware files found: THEY SHOULD NOT BE THERE!!!!
+	if [ -n "`find -name '*.hex'`" ]; then \
 		echo -en "\n\n\n***** $(WARN_MSG) *****\n\n\n"; \
+		exit 1; \
 	fi
-	rm -f $(TMPDIR)/modules/$(LWNS)/src/prism2/*.hex
+#	rm -f $(TMPDIR)/modules/$(LWNS)/src/prism2/*.hex
 	touch build-stamp-indep
 
 # creates the tarball
 install-stamp-indep: build-stamp-indep
-	# install the source package for debian-kernels
+# install the source package for debian-kernels
 	cd $(TMPDIR);\
 		tar zcf ../$(LWNS_K).tar.gz modules
-	# install the source package (for any kernel, even no make-kpkg)
+# install the source package (for any kernel, even no make-kpkg)
 	cp debian/control.source.in \
 		$(TMPDIR)/modules/$(LWNS)/debian/control.modules.in
 	cd $(TMPDIR);\
 		tar zcf ../$(LWNS).tar.gz modules
-	# install the firmware stuff
+# install the firmware stuff
 	TGT=debian/tmp/usr/share/linux-wlan-ng-firmware/debian/;\
 	mkdir -p $$TGT;\
 	mkdir -p debian/tmp/usr/bin/;\
@@ -173,7 +174,7 @@
 	dh_clean -k $(DH_OPTIONS)
 	dh_installdirs $(DH_OPTIONS)
 	
-	# install utils and docs
+# install utils and docs
 	$(MAKE) install $(TO_NULL)
 	dh_install $(DH_OPTIONS)
 	touch install-stamp
@@ -207,7 +208,7 @@
 	echo -en "\n\n\n***** $(UTILS_BUILD_MSG) *****\n\n\n"
 	$(MAKE) all $(TO_NULL)
 
-	# prism2dl binary ( linux-wlan-ng package )
+# prism2dl binary ( linux-wlan-ng package )
 	$(MAKE) -C src/mkmeta $(TO_NULL)
 	$(MAKE) -C src/prism2/download $(TO_NULL)
 
@@ -248,7 +249,7 @@
 binary-arch: binary-arch-modules install-stamp
 	echo -en "\n\n\n***** $(UTILS_DEB_MSG) *****\n\n\n"
 
-	# this is done two times ( install-stamp already done it)
+# this is done two times ( install-stamp already done it)
 	dh_installdirs $(DH_OPTIONS)
 	dh_install $(DH_OPTIONS) -Xrc.d
 
@@ -280,15 +281,15 @@
 	$(MAKE) -C src/prism2/download clean $(TO_NULL)
 	dh_clean debian/postinst
 
-	# stamps
+# stamps
 	$(RM) configure-stamp build-stamp install-stamp \
 		build-stamp-modules build-stamp-indep install-stamp-indep \
 		install-firmware-stamp
 	
-	# temporary stuff 
+# temporary stuff 
 	rm -rf $(TMPDIR) $(MOD_BUILD_DIR)-* $(LWNS_K).tar.gz $(LWNS).tar.gz
 	
-	# firmware
+# firmware
 	rm -rf .firmware
 
 ########################## MAIN ##################################
diff -ruN debian/rules.firmware /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/rules.firmware
--- debian/rules.firmware	2005-11-08 13:51:44.000000000 +0100
+++ /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/rules.firmware	2005-11-08 16:53:49.000000000 +0100
@@ -19,13 +19,13 @@
 	dh_clean -k
 	dh_installdirs
 
-	# firmware files	
+# firmware files	
 	mkdir -p $(TARGET_DIR)
 	cp src/prism2/*pda $(TARGET_DIR)
 	for x in src/prism2/*.hex ; do \
 		if [ -f "$$x" ]; then \
 			SUFFIX=`echo $$x |  cut -c12-13`;\
-			cp $$x $(TARGET_DIR)prims2_$$SUFFIX.hex ; \
+			cp $$x $(TARGET_DIR)prism2_$$SUFFIX.hex ; \
 		fi; \
 	done
 	touch install-stamp
@@ -56,7 +56,7 @@
 clean:
 	dh_clean debian/postinst
 
-	# stamps
+# stamps
 	$(RM) build-stamp install-stamp 
 	
 ########################## MAIN ##################################
diff -ruN debian/rules.modules /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/rules.modules
--- debian/rules.modules	2005-11-08 13:51:44.000000000 +0100
+++ /home/jonas/pub/debian-auryn/tmp/linux-wlan-ng/linux-wlan-ng-0.2.2+dfsg/debian/rules.modules	2005-11-08 16:54:04.000000000 +0100
@@ -60,7 +60,7 @@
 		echo 'LWN_VERSION_MAJOR' $(LWN_VERSION_MAJOR) ;\
 		echo 'KLINUX' $(KLINUX) ;\
 	fi
-	# generate control file suitable for modules package
+# generate control file suitable for modules package
 	cat debian/control.modules.in | \
 		sed 's/$${lwnversmajor}/$(LWN_VERSION_MAJOR)/g' | \
 		sed 's/$${lwnvers}/$(LWN_VERSION)/g' | \

Reply to:

Prev by Date: Bug#338089: New aic7xxx driver fails spectacularly on 2940UW
Next by Date: Re: version/infrastructure separation test ...
Previous by thread: Bug#338228: marked as done (yaird error: unrecognised line in /proc/bus/input/devices: S: Sysfs=/class/input/input0 (fatal))
Next by thread: Bug#336471: still appears broken.
Index(es):
- Date
- Thread