Bug#886479: lintian: Please detect packages that currently needs R続!=no

[Niels Thykier]

Chris Lamb:
> 
> Hi Niels,
> 
> I think I reversed the root:root logic for testing and didn't save in my 
> editor. I am.on mobile now: can you just eq/ne (or the other way around) for 
> me? 
> 
> —lamby
> 
> [...]

Done. :)

Also, I love how your phone converted R³ to R続. :P


Thanks,
~Niels

Bug#886479: lintian: Please detect packages that currently needs R続!=no

[Chris Lamb]

Hi Niels,

I think I reversed the root:root logic for testing and didn't save in my 
editor. I am.on mobile now: can you just eq/ne (or the other way around) for 
me? 

—lamby

> Chris Lamb:
> > tags 886479 + pending
> > thanks
> > 
> > Thanks for the review Niels; glad I didn't push to master late
> > night :)
> > 
> > 
> > Best wishes,
> > 
> 
> So, I think we overlooked something in that patch.  All of the tests are
> failing with:
> 
> """
> > tests::nmu-space-around-maintainer: diff -u 
> > t/tests/nmu-space-around-maintainer/tags 
> > /tmp/testrun/debian/test-out/tests/nmu-space-around-maintainer/tags.nmu-space-around-maintainer
> > --- t/tests/nmu-space-around-maintainer/tags2018-01-07 
> > 20:41:40.599428401 +
> > +++ 
> > /tmp/testrun/debian/test-out/tests/nmu-space-around-maintainer/tags.nmu-space-around-maintainer
> >  2018-01-07 20:52:10.246286088 +
> > @@ -1 +1,2 @@
> > +I: nmu-space-around-maintainer source: should-specify-rules-requires-root 
> > nmu-space-around-maintainer usr/ (root:root)
> >  W: nmu-space-around-maintainer source: 
> > extra-whitespace-around-name-in-changelog-trailer
> > fail tests::nmu-space-around-maintainer: output differs!
> """
> 
> But that strongly smells like a clear cut false-positive.
> 
> Thanks,
> ~Niels
> 


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-

Bug#886479: lintian: Please detect packages that currently needs R続!=no

[Niels Thykier]

Chris Lamb:
> tags 886479 + pending
> thanks
> 
> Thanks for the review Niels; glad I didn't push to master late
> night :)
> 
> 
> Best wishes,
> 

So, I think we overlooked something in that patch.  All of the tests are
failing with:

"""
> tests::nmu-space-around-maintainer: diff -u 
> t/tests/nmu-space-around-maintainer/tags 
> /tmp/testrun/debian/test-out/tests/nmu-space-around-maintainer/tags.nmu-space-around-maintainer
> --- t/tests/nmu-space-around-maintainer/tags  2018-01-07 20:41:40.599428401 
> +
> +++ 
> /tmp/testrun/debian/test-out/tests/nmu-space-around-maintainer/tags.nmu-space-around-maintainer
>2018-01-07 20:52:10.246286088 +
> @@ -1 +1,2 @@
> +I: nmu-space-around-maintainer source: should-specify-rules-requires-root 
> nmu-space-around-maintainer usr/ (root:root)
>  W: nmu-space-around-maintainer source: 
> extra-whitespace-around-name-in-changelog-trailer
> fail tests::nmu-space-around-maintainer: output differs!
"""

But that strongly smells like a clear cut false-positive.

Thanks,
~Niels

Bug#886479: lintian: Please detect packages that currently needs R続!=no

[Chris Lamb]

tags 886479 + pending
thanks

Thanks for the review Niels; glad I didn't push to master late
night :)


Best wishes,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-

Bug#886479: lintian: Please detect packages that currently needs R続!=no

[Niels Thykier]

Chris Lamb:
> tags 886479 + patch
> thanks
> 
> Hi,
> 
>> lintian: Please detect packages that currently needs R!=no
> 
> Draft patch attached:
> 
>   commit 5a881408f5f5dc8a2dc69f25e0e3e1f01fd413e0
>   Author: Chris Lamb 
>   Date:   Sat Jan 6 23:00:35 2018 +
>   
>   Check for packages that should specify Rules-Require-Root. (Closes: 
> #886479)
>   
>checks/control-file.desc | 13 +
>checks/control-file.pm   | 19 ++-
>debian/changelog |  3 +++
>3 files changed, 34 insertions(+), 1 deletion(-)
> 
> 
> Regards,
> 

Hi,

Thanks for the draft. :)

I got a few remarks:

 * check_rules_requires_root appears to be a dead sub (probably
   left-over code in your prototype)
 * I would probably use $group->get_binary_processables as that
   also includes udebs (which technically also triggers the
   requirement).
 * I think Severity should be at most wishlist for packages without
   an explicit Rules-Requires-Root.  For packages with an explicit
   R³ field, I could agree that it would make sense to have it higher
   (except that would almost certainly FTBFS so I doubt the incorrect
value would survive very long anyway)
 * I will leave the choice of Certainty to you.  That said, "wishlist"
   is not valid.
 * The tag description should probably mention that we are doing this to
   be able to change the default for "Rules-Requires-Root" as the vast
   majority of packages are assumed not to require (fake)root.

Thanks,
~Niels

Bug#886479: lintian: Please detect packages that currently needs R続!=no

[Chris Lamb]

tags 886479 + patch
thanks

Hi,

> lintian: Please detect packages that currently needs R!=no

Draft patch attached:

  commit 5a881408f5f5dc8a2dc69f25e0e3e1f01fd413e0
  Author: Chris Lamb 
  Date:   Sat Jan 6 23:00:35 2018 +
  
  Check for packages that should specify Rules-Require-Root. (Closes: 
#886479)
  
   checks/control-file.desc | 13 +
   checks/control-file.pm   | 19 ++-
   debian/changelog |  3 +++
   3 files changed, 34 insertions(+), 1 deletion(-)


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-
>From 5a881408f5f5dc8a2dc69f25e0e3e1f01fd413e0 Mon Sep 17 00:00:00 2001
From: Chris Lamb 
Date: Sat, 6 Jan 2018 23:00:35 +
Subject: [PATCH] Check for packages that should specify Rules-Require-Root.
 (Closes: #886479)

---
 checks/control-file.desc | 13 +
 checks/control-file.pm   | 19 ++-
 debian/changelog |  3 +++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/checks/control-file.desc b/checks/control-file.desc
index 000f148f9..a63932a7c 100644
--- a/checks/control-file.desc
+++ b/checks/control-file.desc
@@ -330,3 +330,16 @@ Severity: classification
 Certainty: certain
 Info: Package requires fakeroot or similar to build binary targets and explicitly declares it.
 Ref: /usr/share/doc/dpkg-dev/rootless-builds.txt.gz
+
+Tag: should-specify-rules-requires-root
+Severity: normal
+Certainty: wishlist
+Info: This package builds at least one binary package containing at
+ least one path with a UNIX ownership other than "root:root".
+ .
+ It therefore requires fakeroot(1) or similar to build its
+ binary targets.
+ .
+ Please specify (eg.) Rules-Requires-Root: binary-targets in
+ the debian/control source stanza.
+Ref: /usr/share/doc/dpkg-dev/rootless-builds.txt.gz
diff --git a/checks/control-file.pm b/checks/control-file.pm
index 097696435..45f118a45 100644
--- a/checks/control-file.pm
+++ b/checks/control-file.pm
@@ -47,7 +47,7 @@ my $KNOWN_DBG_PACKAGE = Lintian::Data->new(
 });
 
 sub run {
-my ($pkg, undef, $info) = @_;
+my ($pkg, undef, $info, undef, $group) = @_;
 my $debian_dir = $info->index_resolved_path('debian/');
 my $dcontrol;
 $dcontrol = $debian_dir->child('control') if $debian_dir;
@@ -381,6 +381,18 @@ sub run {
 tag 'rules-requires-root-implicitly';
 }
 
+if ($info->source_field('rules-requires-root', 'no') eq 'no') {
+  BINARY:
+foreach my $proc ($group->get_processables('binary')) {
+foreach my $file ($proc->info->sorted_index) {
+my $owner = $file->owner . ':' . $file->group;
+next if $owner eq 'root:root';
+tag 'should-specify-rules-requires-root', $file, "($owner)";
+last BINARY;
+}
+}
+}
+
 # find binary packages that Pre-Depend on multiarch-support without going
 # via ${misc:Pre-Depends}
 if ($info->source_field('build-depends')) {
@@ -529,6 +541,11 @@ sub check_relation {
 return;
 }
 
+sub check_rules_requires_root {
+my ($info, $group) = @_;
+
+}
+
 1;
 
 # Local Variables:
diff --git a/debian/changelog b/debian/changelog
index 36178feb8..514930601 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -9,6 +9,9 @@ lintian (2.5.68) UNRELEASED; urgency=medium
   when matching autotools-pkg-config-macro-not-cross-compilation-safe
   by skipping comment lines.  (Closes: #886297)
 + [FL] Fix version parsing for native packages.
+  * checks/control-file.{desc.pm}:
++ [CL] Check for packages that should specify Rules-Require-Root.
+  (Closes: #886479)
   * checks/debconf.pm:
 + [CL] Don't warn about unknown template type "entropy" when a package
   depends on cdebconf.  (Closes: #677870)
-- 
2.15.1