[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#841222: Acknowledgement (RFS: patat)



Hello Félix,

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`.

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.

I wanted to confirm that you'd forwarded your --version patch, but I
couldn't without this header :)

> >>> 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!

> >>> 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: 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

> >>> 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.

> > 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?

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.

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.

> 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.

-- 
Sean Whitton


Reply to: