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

Bug#963832: RFS: iotop-c/1.0-1 [ITP] -- iotop-c - simple top-like I/O monitor (implemented in C)



Hi Paul,

On Wed, 2020-07-08 at 03:22 +0000, Paul Wise wrote:
> Some issues that I think need to be fixed before uploading:
> 
> 
> Please comply with Debian Policy about build verbosity:
> 
> https://www.debian.org/doc/debian-policy/ch-source.html#main-building-script-debian-rules

Sure, fixed.

> Please fix these lintian warnings:
> 
> I: iotop-c: hardening-no-bindnow usr/sbin/iotop-c

For this part I have confirmed that LDFLAGS=-Wl,-z,relro is taken into
account and used, I suppose that it was not in the proper place -
linker options do different things depending on their position.

Checking further I see that in order to enable this protection, a -Wl,-
z,now needs to be added to LDFLAGS and dpkg-buildflags does not include
it (at least on my sid install). For the sake of test, I have added it
in Makefile and the warning have disappeared. I do not think that this
should be fixed in the package - that option should come from dpkg-
buildflags.

> Please fix the cppcheck warning-level complaints. While looking at
> the
> pw_name warning, I noticed that the same pattern occurs in both
> view_batch and view_curses but cppcheck doesn't detect the two
> occurrences in view_curses for some reason.

That is perfectly safe - all recent (last 20 years) glibc versions
allow passing NULL for %s and print (null)... Anyways it is better to
check it explicitly and spare some time for the people who are going to
look into that some day.

> Some issues that would be nice to fix:
> 
> I know that GNU Make has some default rules that could make most of
> the Makefile unnecessary, but I wonder if other Make implementations
> also have enough of them so that you could actually rely on the
> default rules.

I do not get that - the current Makefile uses GNU make extensions and
wildcards, but not a single implicit rule. Please clarify which part
can be improved.

> Please wrap and sort the debian/ packaging files, this makes
> packaging
> diffs easier to read.
> 
> wrap-and-sort --short-indent --wrap-always --sort-binary-packages
> --trailing-comma

OK, fixed.

> Please include copyright holder information and license grants in
> each
> of the source files. SPDX is a standard machine-readable way to do
> that.
> 
> https://lu.is/blog/2012/03/17/on-the-importance-of-per-file-license-information/
> https://spdx.org/

Will do

> Please replace COPYING with the canonical version of the GPLv2:
> 
> https://www.gnu.org/licenses/old-licenses/gpl-2.0.txt

Semantically it was the same, so I see no issue in replacing it. Done.

> Please fix any of the easy-to-fix lintian complaints. I'm not sure
> what is correct for the conflicts, but you could test using piuparts
> if using breaks is enough or if using conflicts is really needed. I
> assume you'll want to keep debhelper compat 12 for backportability.
> Look up the tag names on lintian.d.o or using `lintian-info -t` for
> descriptions of the tags.
> 
> I: iotop-c: conflicts-with-version iotop (<= 0.6-24-g733f3f8-1)

I think this is the only way, in case there is a system with this
package and older iotop package, the only way to prevent installing
files in the same place is conflicts. Maybe it would be nice if there
was a way to tell lintian to silence the warning...

> I: iotop-c source: debian-watch-contains-dh_make-template <project>

Found the problem and fixed it.

> I: iotop-c source: testsuite-autopkgtest-missing

Can't fix that.

> X: iotop-c source: debian-watch-does-not-check-gpg-signature

There is no signature upstream, can't fix that too.

> P: iotop-c source: maintainer-manual-page debian/iotop-
> c.alternatives-dh13.1

I have renamed the file to decrease the noise.

> P: iotop-c source: maintainer-manual-page debian/iotop.8
> P: iotop-c source: package-uses-old-debhelper-compat-version 12
> X: iotop-c source: upstream-metadata-file-is-missing
> 
> The following tools run by check-all-the-things produce some probably
> actionable complaints:
> 
> blhc
> mandoc
> wrap-and-sort
> cppcheck

Thanks for the suggestions!

I will be waiting for your feedback on Makefile implict rules and will
reupload it.

With best regards,
b.


Reply to: