Thanks for the quick review! Comments inline:

On Thu, Feb 22, 2018 at 9:31 AM, Chris Lamb <la...@debian.org> wrote:

> tags 891072 - patch
> thanks
>
> Dear Michael,
>
> > Please review and merge the attached patch. Thanks!
>
> Thank you so much for your patch. Can you fix up the following small
> things?  Naturally, please re-add the "patch" tag when ready :)
>
>
> +Tag: golang-missing-built-using
> +Tag: golang-built-using-on-arch-all
>
> These seem quite "clumsy" wordings and difficult to understand when
> out of context - can you try expanding them a little?
>

Can you make a suggestion as to how they would be clearer please?
I can’t come up with anything. Probably I’m stuck too deep in the subject
matter :).


>
> In addition, please make it very explicit about what them maintainer
> should do if they see this message. For example, something phrased
> along the lines of "please add..."
>

Done.


>
>
> +Info: This package builds a binary package which is not including
>                                                  ^^^^^^^^^^^^^^^^^
>                                                   does not include
>
> + ${misc:Built-Using} in its Built-Using control field.
>   ^^^^^^^^^^^^^^^^^^^        ^^^^^^^^^^^
>    <tt> … </tt>              <tt> … </tt>
>
>
>
Done.


> +            if ($arch eq 'all
>  […]
> +            if ($arch ne 'all'
>
> Is there a cleaner way of structuring these?
>

What would you consider cleaner? It seems fine to me.


>
> +++ a/t/tests/binaries-golang-built-using/desc
>               ^^^^^^^^^
> This probably should be prefixed with "control-file-" as that's the
> name of checks/foo.pm.
>

Done.


>
> Are we missing a Test-Depends in the "desc" file too? :)
>

Not sure what you mean?


>
> Thanks again!
>
>
> Regards,
>
> --
>       ,''`.
>      : :'  :     Chris Lamb
>      `. `'`      la...@debian.org / chris-lamb.co.uk
>        `-
>



-- 
Best regards,
Michael
From c04498f4c71d62cac9e3760c395ae847bf5f5040 Mon Sep 17 00:00:00 2001
From: Michael Stapelberg <stapelb...@debian.org>
Date: Thu, 22 Feb 2018 09:11:20 +0100
Subject: [PATCH] Add golang-missing-built-using and
 golang-built-using-on-arch-all

---
 checks/control-file.desc                           | 23 +++++++++++++
 checks/control-file.pm                             | 15 ++++++++
 t/tests/binaries-golang/desc                       |  1 +
 t/tests/binaries-golang/tags                       |  1 +
 .../debian/Makefile                                | 16 +++++++++
 .../control-file-golang-built-using/debian/basic.c | 12 +++++++
 .../debian/debian/control.in                       | 40 ++++++++++++++++++++++
 t/tests/control-file-golang-built-using/desc       |  6 ++++
 t/tests/control-file-golang-built-using/tags       |  1 +
 9 files changed, 115 insertions(+)
 create mode 100644 t/tests/control-file-golang-built-using/debian/Makefile
 create mode 100644 t/tests/control-file-golang-built-using/debian/basic.c
 create mode 100644 t/tests/control-file-golang-built-using/debian/debian/control.in
 create mode 100644 t/tests/control-file-golang-built-using/desc
 create mode 100644 t/tests/control-file-golang-built-using/tags

diff --git a/checks/control-file.desc b/checks/control-file.desc
index 2b2516bfc..a1399ec9a 100644
--- a/checks/control-file.desc
+++ b/checks/control-file.desc
@@ -350,3 +350,26 @@ Info: This package builds a binary package containing at least one path
  Please specify (eg.) <tt>Rules-Requires-Root: binary-targets</tt> in
  the <tt>debian/control</tt> source stanza.
 Ref: /usr/share/doc/dpkg-dev/rootless-builds.txt.gz
+
+Tag: golang-missing-built-using
+Severity: wishlist
+Certainty: certain
+Info: This package builds a binary package which does not include
+ <tt>${misc:Built-Using}</tt> in its <tt>Built-Using</tt> control field.
+ .
+ The <tt>${misc:Built-Using}</tt> substvar is populated by <tt>dh-golang(1)</tt>
+ and used for scheduling binNMUs.
+ .
+ Please add the following line to your package definition:
+ .
+ <tt>Built-Using: ${misc:Built-Using}</tt>
+
+Tag: golang-built-using-on-arch-all
+Severity: wishlist
+Certainty: certain
+Info: This package builds a binary arch:all package which incorrectly
+ specifies a Built-Using control field.
+ .
+ <tt>Built-Using</tt> only applies to architecture-specific packages.
+ .
+ Please remove the <tt>Built-Using</tt> line from your package definition.
\ No newline at end of file
diff --git a/checks/control-file.pm b/checks/control-file.pm
index f2a97b24d..8d8a1caac 100644
--- a/checks/control-file.pm
+++ b/checks/control-file.pm
@@ -427,6 +427,21 @@ sub run {
           unless $relation->implies('${gir:Depends}');
     }
 
+    # Verify that golang binary packages set Built-Using (except for arch:all
+    # library packages).
+    if ($info->relation('build-depends')->implies('golang-go | golang-any')) {
+        foreach my $bin (@package_names) {
+            my $bu = $info->binary_field($bin, 'built-using');
+            my $arch = $info->binary_field($bin, 'architecture');
+            if ($arch eq 'all' && defined($bu)) {
+                tag 'golang-built-using-on-arch-all', $bin;
+            }
+            if ($arch ne 'all' && (!defined($bu) || $bu !~ /\$\{misc:Built-Using\}/)) {
+                tag 'golang-missing-built-using', $bin;
+            }
+        }
+    }
+
     return;
 }
 
diff --git a/t/tests/binaries-golang/desc b/t/tests/binaries-golang/desc
index 0a13b1901..7cc04dd02 100644
--- a/t/tests/binaries-golang/desc
+++ b/t/tests/binaries-golang/desc
@@ -4,3 +4,4 @@ Extra-Build-Depends: golang-go (>= 2:1.1.1-4)
 Description: Misc errors related to golang binaries
 Architecture: any
 Test-Against: statically-linked-binary
+Test-For: golang-missing-built-using
diff --git a/t/tests/binaries-golang/tags b/t/tests/binaries-golang/tags
index e69de29bb..e287ae798 100644
--- a/t/tests/binaries-golang/tags
+++ b/t/tests/binaries-golang/tags
@@ -0,0 +1 @@
+I: binaries-golang source: golang-missing-built-using binaries-golang
diff --git a/t/tests/control-file-golang-built-using/debian/Makefile b/t/tests/control-file-golang-built-using/debian/Makefile
new file mode 100644
index 000000000..f04b342de
--- /dev/null
+++ b/t/tests/control-file-golang-built-using/debian/Makefile
@@ -0,0 +1,16 @@
+NOPIE_CFLAGS = $(filter-out -fPIE,$(CFLAGS))
+NOPIE_LDFLAGS = $(filter-out -fPIE -pie,$(LDFLAGS))
+COMPILE:= $(CC) $(NOPIE_CFLAGS) $(CPPFLAGS) $(NOPIE_LDFLAGS)
+
+all:
+	# static version
+	$(COMPILE) -static -o basic.static basic.c
+
+install:
+	install -d $(DESTDIR)/usr/lib/foo/
+	install -m 755 basic.static $(DESTDIR)/usr/lib/foo/static
+
+clean distclean:
+	rm -f basic
+
+check test:
diff --git a/t/tests/control-file-golang-built-using/debian/basic.c b/t/tests/control-file-golang-built-using/debian/basic.c
new file mode 100644
index 000000000..7dea5a030
--- /dev/null
+++ b/t/tests/control-file-golang-built-using/debian/basic.c
@@ -0,0 +1,12 @@
+#include <stdio.h>
+
+int
+main(void)
+{
+    char t[10];
+    printf("Hello world!\n");
+    /* Bad choice for reading from stdin, but it forces a stack
+       protector, so meh.
+     */
+    gets (t);
+}
diff --git a/t/tests/control-file-golang-built-using/debian/debian/control.in b/t/tests/control-file-golang-built-using/debian/debian/control.in
new file mode 100644
index 000000000..136a3cb65
--- /dev/null
+++ b/t/tests/control-file-golang-built-using/debian/debian/control.in
@@ -0,0 +1,40 @@
+Source: {$source}
+Section: net
+Priority: optional
+Maintainer: {$author}
+Standards-Version: {$standards_version}
+Build-Depends: {$build_depends},
+ golang-go (>= 2:1.1.1-4)
+
+Package: {$source}
+Architecture: {$architecture}
+Depends: $\{misc:Depends\}
+Built-Using: $\{misc:Built-Using\}
+Description: {$description}
+ This is a test package designed to exercise some feature or tag of
+ Lintian.  It is part of the Lintian test suite and may do very odd
+ things.  It should not be installed like a regular package.  It may
+ be an empty package.
+
+Package: {$source}-dev
+Architecture: all
+Depends: $\{misc:Depends\}
+Built-Using: $\{misc:Built-Using\}
+Description: {$description} (dev)
+ This is a test package designed to exercise some feature or tag of
+ Lintian.  It is part of the Lintian test suite and may do very odd
+ things.  It should not be installed like a regular package.  It may
+ be an empty package.
+ .
+ This package contains the source.
+
+Package: {$source}-clean-dev
+Architecture: all
+Depends: $\{misc:Depends\}
+Description: {$description} (clean dev)
+ This is a test package designed to exercise some feature or tag of
+ Lintian.  It is part of the Lintian test suite and may do very odd
+ things.  It should not be installed like a regular package.  It may
+ be an empty package.
+ .
+ This package contains the source, without a built-using tag.
diff --git a/t/tests/control-file-golang-built-using/desc b/t/tests/control-file-golang-built-using/desc
new file mode 100644
index 000000000..85118b21f
--- /dev/null
+++ b/t/tests/control-file-golang-built-using/desc
@@ -0,0 +1,6 @@
+Testname: binaries-golang-built-using
+Version: 1.0
+Description: False-positive test for Built-Using field
+Architecture: any
+Test-Against: golang-missing-built-using
+Test-For: golang-built-using-on-arch-all
diff --git a/t/tests/control-file-golang-built-using/tags b/t/tests/control-file-golang-built-using/tags
new file mode 100644
index 000000000..5724c0cd7
--- /dev/null
+++ b/t/tests/control-file-golang-built-using/tags
@@ -0,0 +1 @@
+I: binaries-golang-built-using source: golang-built-using-on-arch-all binaries-golang-built-using-dev
-- 
2.15.1

Reply via email to