Asbjørn,
On Sat, Jun 25, 2011 at 02:28:45PM +0000, Asbjørn Sloth Tønnesen wrote:
> On 06/25/2011 11:29 AM, Kilian Krause wrote:
[...]
> >- using dh-autoreconf feels like to wrong solution to a problem to me. Is
> > that really required to autoreconf? What for?
>
> This is because I'm using git-buildpackage, and pulling upstream
> through git, not release tarballs, as documented in the
> git-buildpackage documentation[1].
>
> I have the following branches in my git repository:
> - master - used for Debian packaging
> - upstream - upstream release I am working on (based on upstream tag)
> - upstream-head - upstream's current git HEAD
>
> I'm building with:
> git-buildpackage --git-export-dir=../build --git-export=HEAD -k5238B839
sounds sane to me. Just as there is a resulting diff with the upstream
tarball that's available on the website found by debian/watch please make
sure this is documented - like README.source or so. As you are
autoreconf'ing due to your Git-link anyway, why not ask upstream to tag
releases with identical contents than the release tarball and thus push this
task upstream? Or if you see a good benefit doing this as part of your
(re-)generation of the orig.tar.gz make a get-orig-source target in
debian/rules that will do this more transparent instead of having each and
every buildd force to autoreconf during the deb building. Most often
autoreconf in the automatic build will cause very funny effects that you can
easily avoid by fixing the orig.tar.gz and then just doing the regular
configure during build time.
> >- debian/changelog with
> > Format: http://anonscm.debian.org/viewvc/dep/web/deps/dep5.mdwn?revision=174&view=markup
> > is most probably too specific about which dep5 format you intended to
> > fulfill. Not exactly wrong just feels a bit "funny". ;-)
>
> Before the review session on IRC yesterday, I had:
> Format: http://dep.debian.net/deps/dep5
>
> It was pointed out that it was best practice to link to the revision:
>
> I of my 139 installed packages that have a DEP 5 copyright file:
> - 28 links to http://dep.debian.net/deps/dep5/
> - 111 links to http://svn.debian.org/wsvn/dep/web/deps/dep5.mdwn + rev
> - all dead links currently due to the websvn => viewvc change
>
> DEP 5 uses this link as an example:
> http://svn.debian.org/wsvn/dep/web/deps/dep5.mdwn?op=file&rev=REVISION
>
> liblucene2-java actually uses it directly. :( (Filing bug in a moment)
>
> I changed it to the anonscm.d.o link, because of this.
>
> After that I have through some more about it, now that DEP 5 is
> frozen I guess that, it might be better to just change it back to
> the dep.d.n link again.
That was exactly my point I had in mind. Referencing
http://dep.debian.net/deps/dep5/ will be a more static URL over time but as
I said, I'm not aware there's an official oppinion on this. So either one is
fine.
> >- The tarball doesn't match upsteam's - yet there's no notion of why this
> > difference exists:
> > -rw-rw-r-- 1 kk kk 51500 25. Jun 12:56 dhcpd-pools_2.15.orig.tar.gz
> > -rw-rw-r-- 1 kk kk 388230 25. Jun 13:04 tarballs/dhcpd-pools_2.15.orig.tar.gz
> > Either you seriously do a repack and alter the version number to ~dfsg (if
> > it's a DFSG-driven repack) or a ~debian if it's rather a cosmetic repack.
> >
> > From a first glance it seems that you're working around exactly that
> > autoreconf issue (reintroducing what you stripped out) and eventually some
> > Git that you could educate upstream to leave out of the RELEASE tarball.
> > Please get this sorted out in a clean way so that upstream's tarball is
> > used unchanged if you intend to maintain your current version number as
> > is.
>
> It is a purely cosmetic repack, based on upstreams release git tag.
> I wouldn't mind changing the version number to include ~debian, but
> in that case ~git would make more sense, as there are no debian
> specific changes in the repack, but ~git might also sound more scary
> than ~gittag. If ~debian is recommended/mandatory in package version
> where orig.tar.gz is generated by git-buildpackage, I would expect
> it to be mentioned in the git-buildpackage documentaion. If it is I
> would be happy to make patch against git-buildpackage adding that.
I'm fine with either ~debian or ~git. Just don't name it identical as
upstream uses on his website without avoiding a resulting diff.
If you have a resulting contents of the tarball but md5/sha1 diff due to
repacking your own tarball from Git I'd say that's ok if it's documented in
get-orig-source and README.source. And if you autoreconf due to upstream
doesn't export the tag properly reconf'd then use ~git or +git as version suffix.
> >- dpkg-gencontrol: warning: package dhcpd-pools: unused substitution variable ${perl:Depends}
> > should also be fixed - i.e. make sure it finds all *.pl files and does
> > look at them.
>
> Added it, the only perl file is contrib/snmptest.pl, and it doesn't
> have any non standard perl dependencies. So ${perl:Depends} only
> gets replaced with perl itself.
Good.
> >- I: dhcpd-pools: hyphen-used-as-minus-sign usr/share/man/man1/dhcpd-pools.1.gz:13
> > may also be worth fixing while we're at it
>
> I have ignored this, due to upstream having a commit[2] removing
> that line, scheduled for next release. Should I backport it?
If the next upstream release won't take ages it's not really necessary but
it won't hurt either.
> >- licensecheck still reports:
> > ./src/getopt.c: GPL (with incorrect FSF address)
> > ./src/getopt.h: GPL (with incorrect FSF address)
> > ./src/getopt1.c: GPL (with incorrect FSF address)
> > which you may want to tell upstream about.
>
> Will submit patch upstream. Have also added them to debian/copyright
> as GPL-2.0+.
>
> I assume that contrib/snmptest.pl is GPL-3.0+.
>
> > And just for the record ltmain.sh is GPLv2 (and isn't mentioned as
> > exception to the all-is-GPL3 in debian/copyright)
>
> ltmain.sh is created by autoreconf, and is distributed through that,
> it is neither a part of the orig.tar.gz, debian.tar.gz nor the
> resulting deb. It is through distributed in upstream release
> tarball.
Yes, right. I guess "autoreconf &&./configure --enable-maintainer-mode &&make
dist&&git-import-orig *.tar.gz" would be the most sane version to get only
the required files shipped.
> >- Spelling in debian/control should be fixed for at least:
> > accomodiate => accommodate
> >
> >- In debian/control you first talk about "ISC dhcp" and later about "ISC dhcpd"
> > meaning the same thing though - the DHCP server. Please use one wording
> > only.
> >
> >- CSV is an abbreviation that should be capitalized in the description
> >
> >- "Users of the command" most probably means "Users of this tool" I guess.
>
> Fixed, I took that description directly from upstream, will push the
> corrections upstream.
>
> [1] http://honk.sigxcpu.org/projects/git-buildpackage/manual-html/gbp.import.html#AEN195
> [2] http://git.asbjorn.biz/?p=debian/dhcpd-pools.git;a=commitdiff;h=2cb7369e
Nice.
If you can get the autoreconf fixed in a sane way I see no reason not to
sponsor your package.
--
Best regards,
Kilian
Attachment:
signature.asc
Description: Digital signature