| Message ID | 1241025728.16780.21.camel@quest |
|---|---|
| State | Superseded, archived |
| Headers | show |
Scott, thanks for submitting these patches. Two quick comments:
1) It's greatly preferred if you submit each patch as separate e-mail
messages, with the one-line summary as the subject (as you might find
in a git submission or as described in sections 5.4 and 5.5 in the file
Documentation/development-process/5.Posting in the Linux kernel sources).
The reason for this is that the patchs will then be tracked in patchwork:
http://patchwork.ozlabs.org/project/linux-ext4/list/
and if it's accurately reflected there, it's much less likely that I
won't accidentally forget to act on your patches, especially when I
get busy/overloaded. The best thing to do is to use "git
format-patch" (which you apparenlty did to generate the patch), and
then use "git send-email" to send out the patches.
I find that this is in fact the fastest way for me to do work. So
after I commit two patches, I'll do something like this:
rm -rf /tmp/p
git format-patch -n -o /tmp/p -2
git send-email --to linux-ext4@vger.kernel.org /tmp/p
(and in fact I have an aliases file configured in ~/.gitconfig, so I
have sendemail.aliasesfile=/home/tytso/.mutt/aliases and
sendemail.aliasfiletype=mutt, in which case all I have to type is:
"git send-email --to linux-ext4 /tmp/p")
2) While lsb_release is mandatory on Ubuntu systems (there is a
dependency from ubuntu-minimal), it is not guaranteed to exist on
Debian systems. So how I already test for Ubuntu in the rules file is
via this construct:
if test -f /etc/lsb-release && \
grep -q DISTRIB_ID=Ubuntu /etc/lsb-release; then \
$(INSTALL) -p -m 0644 e2fsck/e2fsck.conf.ubuntu \
${debdir}/e2fsprogs/etc/e2fsck.conf; \
fi
Hmm, one of these days I should probably check and see if Ubuntu
finally fixed the installer and init scripts breakage which forced me
to set a special e2fsck.conf file just for Ubuntu. (See git commit
60702c26 in the e2fsprogs repository for more details.)
But in any case, I'd suggest checking for Ubuntu using the above
construct instead of trying to use the lsb_release command, since it's
not guaranteed to be there on Debian systems.
3) In the "nice to have" category, it would be nice if whether or not
blkid is built is controled via DEB_BUILD_OPTIONS flag instead of a
free-standard environment variable, but I won't insist on that.
- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2009-04-29 at 16:54 -0400, Theodore Tso wrote: > Scott, thanks for submitting these patches. Two quick comments: > > 1) It's greatly preferred if you submit each patch as separate e-mail > messages, with the one-line summary as the subject (as you might find > in a git submission or as described in sections 5.4 and 5.5 in the file > Documentation/development-process/5.Posting in the Linux kernel sources). > The reason for this is that the patchs will then be tracked in patchwork: > > http://patchwork.ozlabs.org/project/linux-ext4/list/ > > and if it's accurately reflected there, it's much less likely that I > won't accidentally forget to act on your patches, especially when I > get busy/overloaded. The best thing to do is to use "git > format-patch" (which you apparenlty did to generate the patch), and > then use "git send-email" to send out the patches. > At this point, consider these patches "for your review" ;) If you're happy with them, I'll submit them to the mailing list properly. > 2) While lsb_release is mandatory on Ubuntu systems (there is a > dependency from ubuntu-minimal), it is not guaranteed to exist on > Debian systems. > That's why I have "|| Debian" in there. This is the same rule we use in util-linux on Debian/Ubuntu. > So how I already test for Ubuntu in the rules file is > via this construct: > > if test -f /etc/lsb-release && \ > grep -q DISTRIB_ID=Ubuntu /etc/lsb-release; then \ > $(INSTALL) -p -m 0644 e2fsck/e2fsck.conf.ubuntu \ > ${debdir}/e2fsprogs/etc/e2fsck.conf; \ > fi > It's not guaranteed that /etc/lsb-release says DISTRIB_ID=Ubuntu - if we ever simply changed the defaults in lsb_release, then this would fail - no? > Hmm, one of these days I should probably check and see if Ubuntu > finally fixed the installer and init scripts breakage which forced me > to set a special e2fsck.conf file just for Ubuntu. (See git commit > 60702c26 in the e2fsprogs repository for more details.) > We spent a very long time on this - in Jaunty this should be rock-solidly correct <g> > 3) In the "nice to have" category, it would be nice if whether or not > blkid is built is controled via DEB_BUILD_OPTIONS flag instead of a > free-standard environment variable, but I won't insist on that. > Good idea. Scott
From 3607e808fd9fd340fdcb2b946fc3533b52cbd27a Mon Sep 17 00:00:00 2001 From: Scott James Remnant <scott@ubuntu.com> Date: Wed, 29 Apr 2009 18:18:06 +0100 Subject: [PATCH] debian: use external libblkid on Ubuntu Check the output of lsb_release -is to allow per-distro customisations. On Ubuntu, use the external libblkid. The advantage to doing this in the standard rules is that anybody may download a newer e2fsprogs tarball and have it build packages the same way as their distribution. Use ?= so it can still be overidden. --- debian/rules | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/debian/rules b/debian/rules index b798854..f60f0a9 100755 --- a/debian/rules +++ b/debian/rules @@ -12,6 +12,12 @@ export LC_ALL=C # no chance that pkg-create-dbgsym can cope with this package's manual construction of -dbg export NO_PKG_MANGLE=1 +# Allow distro-specific behaviour +DISTRO :=$(shell lsb_release -is 2>/dev/null || echo Debian) +ifeq ($(DISTRO),Ubuntu) +BUILD_BLKID ?= no +endif + # These are used for cross-compiling and for saving the configure script # from having to guess our platform (since we know it already) DEB_HOST_ARCH ?= $(shell dpkg-architecture -qDEB_HOST_ARCH) -- 1.6.0.5