Bug#902225: RFS: ii/1.8-1

2018-08-08 Thread Niels Thykier
itd:
> Hi,
> 
> thanks for the review!
> 
> Niels Thykier:
>> # The remark:
> Oops! Fixed, thanks. (Updated Build-Dependency on debhelper.)
> 

Ack. :)

>>  * Relicense the patches to the same as upstream (requires approval from
>>the previous maintainer).  We already spoke about this on IRC.
> Previous maintainer approves (see #902225 message #23). (@Nico: Thanks!)
> (Updated d/copyright.)
> 

Excellent.  For good measure, please document this in the changelog.
Mostly so it is obvious to people who were not following along on these
bugs (and to help ourselves if we are asked to explain it later)

On a related note, there is a minor but relevant mistake in the
debian/copyright file:

"""
Files: *
   debian/patches/*
[...]
License: MIT

Files: debian/*
[...]
License: GPL-2
"""

The problem is that the "debian/*" overshadows the debian/patches/*
entry.  According to the copyright file, debian/patches/* are still
under the GPL-2.
  This is caused by how the copyright file is read.  I a bit surprised
that lintian did not give any warnings about this (it does for some
similar cases), so I have filed #905747 to improve the situation.

>> [... other suggestions ...]
> Done.
> 

:)

>>  * Have you considered setting up a git repository for the packaging on
>>salsa.debian.org?
> Yes. I'm currently using https://salsa.debian.org/itd-guest/ii.
> It has restricted ("internal") visibility, since it's not official. I
> couldn't find a previous repository. "gbp import-dscs --debsnap ii" was
> used to import previous package versions (IIRC).
> 

A good idea for bootstrapping a git repo. :)  Given that you are (about
to be) the official maintainer for ii, please make it public and then
you can add the Vcs-Git + Vcs-Browser fields in debian/control as well. :)
  Alternatively, we can add a "debian/ii" repo that you can work in if
you think that would be better.  (But I do not think it is worth
stalling the upload for that.  Uploads are cheap so we can always do
another one for that)

On a related note, as Axel mentioned, Nico has retired from Debian.
Please replace him with yourself in the Maintainers field (and drop the
Uploaders field).  :)

These bits are less critical and can wait till a later upload if you prefer.

> # Other changes
> * use libbsd-dev to provide strlcpy(3) and exclude "strlcpy.c"
> * apply upstream patch that adds additional user input validation
> 

Excellent. :)

> Thanks again!
> 
> Regards,
> itd
> --
> $ head -n 17 debian/changelog
> [...]
> 


Once again, thanks for working on improving Debian, :)
~Niels



Bug#902225: RFS: ii/1.8-1

2018-08-08 Thread Axel Beckert
Hi,

itd wrote:
> Yes. I'm currently using https://salsa.debian.org/itd-guest/ii.
> It has restricted ("internal") visibility, since it's not official. I
> couldn't find a previous repository. "gbp import-dscs --debsnap ii" was
> used to import previous package versions (IIRC).

That's a good method, thanks!

One thing which nobody seems to have noticed in this bug report yet:
Nico Golde has retired, see https://nm.debian.org/person/nion

So you should probably take over the package completely (i.e. list
yourself in the Maintainer field) instead of adding yourself to
Uploaders.

Regards, Axel
-- 
 ,''`.  |  Axel Beckert , https://people.debian.org/~abe/
: :' :  |  Debian Developer, ftp.ch.debian.org Admin
`. `'   |  4096R: 2517 B724 C5F6 CA99 5329  6E61 2FF9 CD59 6126 16B5
  `-|  1024D: F067 EA27 26B9 C3FC 1486  202E C09E 1D89 9593 0EDE


signature.asc
Description: Digital signature


Bug#902225: RFS: ii/1.8-1

2018-08-08 Thread Nico Golde
Hi itd,
the license change for debian patches is fine!

Cheers,
Nico

-- 
Nico Golde - XMPP: n...@jabber.ccc.de - GPG: 0xA0A0


signature.asc
Description: PGP signature


Bug#902225: RFS: ii/1.8-1

2018-08-08 Thread Niels Thykier
Control: tags -1 moreinfo -pending

On Sat, 23 Jun 2018 15:09:00 + itd  wrote:
> Package: sponsorship-requests
> Severity: normal
> 
> Dear mentors,
> 
> @Nico: I added myself to Uploaders. Please shout, if that is not what
> you meant in #890995 [1].
> 
> [...]
> 
> Changes since the last upload:
> 
>   * New upstream release (Closes: #890995).
>   * Fix watchfile (upstream URL changed).
>   * Bump compat level and debhelper dependency to 11.
>   * Bump standards version to 4.1.4.
>   * Update patch respect_dpkg_buildflags.patch.
>   * Enable hardening (requires dpkg-dev >= 1.16.1.1).
>   * Switch to machine-readable copyright file.
>   * Experimental autopkgtest smoke-test.
>   * Switch to HTTPS upstream links.
>   * Add myself to Uploaders.
> 
> 
> Regards,
> itd
> 
> [1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890995#10
> 

Hi,

Thanks for working on improving Debian. :)

I had a look at reviewing your package; I got one question/remark before
I sponsor it and then some optional suggestions for improvements.

# The remark:

In debian/rules, I noticed the following line:
"""
  rm -f =$(CURDIR)/debian/ii/usr/share/doc/ii/CHANGES
"""

It seems strange that there would be a valid path starting with "=", so
I am guessing that is a typo (AFAICT, left-over from previous versions
of ii).

To be honest, I think the line can be removed if you call
dh_installchangelogs *without* the CHANGES argument and bump the
Build-Dependency on debhelper to "debhelper (>= 11.3.2~)".  As of that
version of debhelper, dh_installchangelogs should do what you want out
of the box.  (This fix though is optional; fixing the above typo is also
a valid solution)

# Optional suggestions:

These are suggestions that I think would make your package better,
easier to maintain in the long turn or otherwise improve it.  These can
be done in a separate upload.

 * Relicense the patches to the same as upstream (requires approval from
   the previous maintainer).  We already spoke about this on IRC.
   - If / when the previous maintainer approves it, it is simply a
 question of updating the debian/copyright to reflect the special
 license for debian/patches/*

 * Set [Rules-Requires-Root] to "no" as the source package does not need
   (fake)root to build the .debs.  It is a one-line change to the
   debian/control file.
   (see attached debdiff)

 * Use debhelper's dh_auto_* tools instead of calling $(MAKE) to support
   cross-building.  Literally no additional changes will be required
   (see attached debdiff).

 * Consider rewriting debian/rules into a "dh7" style rules file.
   (see [rules.tiny] for a basis example to build on).  This will have
   the advantage that dh can skip some helpers automatic if there is
   nothing to do and new helpers will be enabled automatically in new
   compat levels.
   - It might be useful to postpone this change to a later revision.
 Especially if you have never worked with dh(1) before.

 * Have you considered setting up a git repository for the packaging on
   salsa.debian.org?

# Summary:

Once you have resolved the remark about a typo in d/rules, I am happy to
sponsor the package.  The optional suggestions can be done either now or
in later revisions.

Thanks,
~Niels

[Rules-Requires-Root]:
https://www.debian.org/doc/debian-policy/ch-controlfields.html#rules-requires-root

[rules.tiny]:
/usr/share/doc/debhelper/examples/rules.tiny

You will probably also need to read "man dh" to learn about "OVERRIDE
TARGETS".  The dh manpage contains examples for how to use these
override targets.

diff -Nru ii-1.8/debian/control ii-1.8/debian/control
--- ii-1.8/debian/control   2018-06-23 10:49:42.0 +0200
+++ ii-1.8/debian/control   2018-08-08 11:16:33.0 +0200
@@ -6,6 +6,7 @@
 Build-Depends: debhelper (>= 11~), dpkg-dev (>= 1.16.1.1)
 Standards-Version: 4.2.0
 Homepage: https://tools.suckless.org/ii/
+Rules-Requires-Root: no
 
 Package: ii
 Architecture: any
diff -Nru ii-1.8/debian/rules ii-1.8/debian/rules
--- ii-1.8/debian/rules 2018-06-23 10:49:42.0 +0200
+++ ii-1.8/debian/rules 2018-08-08 11:16:33.0 +0200
@@ -12,7 +12,8 @@
dh_testdir
 
# Building package
-   CFLAGS="$(CFLAGS)" $(MAKE) PREFIX=/usr
+   #CFLAGS="$(CFLAGS)" $(MAKE) PREFIX=/usr
+   dh_auto_build
 
touch build-stamp
 
@@ -22,7 +23,8 @@
rm -f build-stamp
 
# Cleaning package
-   [ ! -f Makefile ] || $(MAKE) clean
+   #[ ! -f Makefile ] || $(MAKE) clean
+   dh_auto_clean
 
dh_clean
 
@@ -33,7 +35,8 @@
dh_installdirs
 
# Installing package
-   $(MAKE) install DESTDIR=$(CURDIR)/debian/ii PREFIX=/usr
+   #$(MAKE) install DESTDIR=$(CURDIR)/debian/ii PREFIX=/usr
+   dh_auto_install -- PREFIX=/usr
rm -f =$(CURDIR)/debian/ii/usr/share/doc/ii/CHANGES
 
# Removing double files