Hi, On 2016-10-30 10:14-0700, Sean Whitton wrote: > I noticed that you've published your patch-queue/master branch. Since > that is a branch you will rebase, it's not a good idea to publish it > (the gbp documentation recommends against publishing it). Anyone who > needs the branch can reconstruct it with `gbp pq import`. OK, I unpublished patch-queue/master. > Unfortunately, patat doesn't build against sid at present. Hopefully > this will be resolved within the next week or so. In the meantime, > there is some stuff you can improve: > > On Tue, Oct 25, 2016 at 10:34:11PM +0200, Félix Sipma wrote: >> On 2016-10-23 11:51-0700, Sean Whitton wrote: >>> You should use "Forwarded: not-needed" (see DEP-3). >> >> This does not seem to work with gbp-pq (see #785274), I propose to add this as >> soon as gbp-pq supports DEP-3. > > Indeed. Dmitry Bogatov pointed out to me that you can put the > Forwarded: header at the end of the patch description (just before the > ---) and then gbp won't remove it. It seems like gbp _does_ remove the Forwarded: header put just before the ---... > I wanted to confirm that you'd forwarded your --version patch, but I > couldn't without this header :) This particular patch is not needed anymore (fixed upstream). I pushed a new version to my repo, and will put this new version on mentors as soon as pandoc gets installable again. >>>>> 2. You can fix all of these Lintian tags, except possibly >>>>> hardening-no-fortify-functions. You should definitely deal with >>>>> the warnings. >>>>> >>>>> W: patat-dbgsym: debug-file-with-no-debug-symbols >>>> >>>> I've updated debian/rules to something matching >>>> stylish-haskell. > > Your d/rules is fine, though I think that the override_dh_compress stanza > is not needed: policy says you should only compress files above a > certain size, and presumably dh_compress isn't compressing the README > because it is smaller than that size. > > On the next upload of stylish-haskell I will probably remove that > stanza -- sorry to mislead you! OK, I removed the override_dh_compress stanza. >>>>> I: patat: spelling-error-in-binary usr/bin/patat Nam Name >>>>> I: patat: spelling-error-in-binary usr/bin/patat isn't isn't >>>>> I: patat: spelling-error-in-binary usr/bin/patat forward forward >>>>> I: patat: spelling-error-in-binary usr/bin/patat upto up to >>>>> I: patat: spelling-error-in-binary usr/bin/patat discontigous discontiguous >>>>> I: patat: spelling-error-in-binary usr/bin/patat uncomplete incomplete >>>>> I: patat: spelling-error-in-binary usr/bin/patat The The >>>> >>>> Not sure about this one... Is "patat" too generic for lintian? I've >>>> added this to debian/lintian-overrides. >>> >>> I don't understand. It is pointing out misspellings, such as >>> 'uncomplete', somewhere in the upstream source. You can add a quilt >>> patch to fix them, and forward it upstream. >> >> As I didn't found anything matching these errors in the source, I thought it >> was a generic error message concerning the binary name. >> >> Now, that I understood the purpose of this check, I can only found these >> mistakes in the binary itself, so I guess these are in the dependencies... > > Okay. In that case you should override them, with a comment in the > overrides file explaining why. I guess it is better to not override this warning, so that we don't forget that the dependencies needs to be fixed. >>>>> I: patat: hardening-no-bindnow usr/bin/patat I: patat: >>>>> hardening-no-pie usr/bin/patat >>>>> >>>>> I think that in order to pass hardening options to gcc, if you're >>>>> willing to work on that, you'll need to abandon the CDBS build system >>>>> you're using at present. See the Makefile for keysafe[1] (not yet in >>>>> Debian) to see how to pass the options, and the rules file for the >>>>> stylish-haskell package to see how to do without CDBS. >>>> >>>> After reading this Makefile, I'm not sure how keysafe avoids >>>> hardening-no-bindnow and hardening-no-pie... Do you have any clue? >>> >>> The Makefile propagates LDFLAGS, CFLAGS and CPPFLAGS through to ghc. >>> Then you enable all hardening in your d/rules,[1] and the right flags >>> get set by debhelper. >>> >>> [1] https://wiki.debian.org/Hardening >> >> I would like to wait a little before adding this: the default flags added to >> gcc seems quite new, so I propose to have a look again when things stabilize. > > Fair enough. > > FWIW keysafe's hardening is working fine, except for PIE, which has to > be disabled for Haskell atm. https://git.spwhitton.name/keysafe OK. >>>>> 3. Please run upstream's test suite during the package build. >>>> >>>> Should be done now, I'm not sure about how I run tests... See >>>> debian/rules override_dh_auto_test > > Okay. I can't test this atm because patat can't be built in sid, but > what you did looks sane. OK. >>> If help2man is insufficient, see again stylish-haskell where I use >>> asciidoctor. >> >> I'll try to add a manpage using help2man. > > A few of comments on your manpage: > > 1. Have you forwarded it upstream? No. > 2. Did you generate it with help2man, in the end? If so, there should > be a rule in d/rules to allow someone to regenerate the manpage for a > new upstream version (see the ocrmypdf package's rules file). If > upstream introduces a new upstream version it should be easy to update > the manpage. No. I did it by hand, help2man generated something ugly :-). > 3. It might be nice to add a reference to the file in > /usr/share/doc/patat/examples to the manpage. If I wanted to learn how > to use patat, the manpage alone wouldn't be much use. Upstream is working on a manpage (see https://github.com/jaspervdj/patat/issues/19 ). I'll add this manpage later, for now I would like to have patat in debian. This manpage stuff is not essential (and it takes time to work on it, that's why I didn't want to work on this), so I'd like to keep it like this, and update it as soon as upstream release a manpage. >> Concerning DHG's package-plan, I can't run the tests myself, ghc seems >> to be broken in my chroot due to hardening flag -pie (see #712228). So >> I propose to add patat later, when things calm down. > > Looks fine now -- good work. OK. patat is buildable again on sid. Thanks again for your reviews!
Attachment:
signature.asc
Description: PGP signature