GNU bug report logs - #27811
net-tools patch has no effect?

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27811 in the body.
You can then email your comments to 27811 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: bug-guix <at> gnu.org
Subject: net-tools patch has no effect?
Date: Mon, 24 Jul 2017 18:02:01 -0400

[Message part 1 (text/plain, inline)]

While looking into an update of the Debian patch that we apply to
net-tools, I noticed that the patch is not being applied meaningfully.

After the 'patch' phase, there is an empty file called 'the-patch' in
the root of the source tree, but otherwise there are no changes to the
unpacked tarball.

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: 27811 <at> debbugs.gnu.org
Subject: Re: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 00:12:25 -0400

[Message part 1 (text/plain, inline)]

The problem is that, in the patch phase, the patch file is referred to
as 'patch', but the package for the `patch` program is what that
variable resolves to:

(let ((patch.gz (assoc-ref inputs "patch")))

After fixing that by renaming the variable for the patch file to
something unique, the build fails like this:

------
gcc -D_GNU_SOURCE -O2 -Wall -g  -I. -idirafter ./include/ -Ilib   -c -o ipmaddr.o ipmaddr.c
ipmaddr.c: In function ‘multiaddr_modify’:
ipmaddr.c:351:25: warning: pointer targets in passing argument 2 of ‘parse_lla’ differ in signedness [-Wpointer-sign]
    if (parse_lla(*argv, ifr.ifr_hwaddr.sa_data) < 0)
                         ^
ipmaddr.c:78:12: note: expected ‘unsigned char *’ but argument is of type ‘char *’
 static int parse_lla(char *str, unsigned char *addr)
            ^
gcc  -Llib -o ipmaddr ipmaddr.o -lnet-tools 
gcc -D_GNU_SOURCE -O2 -Wall -g  -I. -idirafter ./include/ -Ilib   -c -o mii-tool.o mii-tool.c
mii-tool.c: In function ‘main’:
mii-tool.c:432:29: error: too few arguments to function ‘parse_media’
  case 'A': nway_advertise = parse_media(optarg); break;
                             ^
mii-tool.c:163:12: note: declared here
 static int parse_media(char *arg, unsigned *bmcr2)
            ^
mii-tool.c:433:26: error: too few arguments to function ‘parse_media’
  case 'F': fixed_speed = parse_media(optarg); break;
                          ^
mii-tool.c:163:12: note: declared here
 static int parse_media(char *arg, unsigned *bmcr2)
            ^
mii-tool.c:456:2: warning: implicit declaration of function ‘version’ [-Wimplicit-function-declaration]
  version();
  ^
mii-tool.c: At top level:
mii-tool.c:32:13: warning: ‘Version’ defined but not used [-Wunused-variable]
 static char Version[] = "$Id: mii-tool.c,v 1.9 2006/09/27 20:59:18 ecki Exp $\n(Author: David Hinds based on Donald Becker's mii-diag)";
             ^
make: *** [<builtin>: mii-tool.o] Error 1
phase `build' failed after 7.5 seconds
------

This problem with the wrong number of arguments being passed to
parse_media() is introduced in the Debian patch, so we should check if
there is a better version of the patch or if fixes are being applied
upstream.

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: 27811 <at> debbugs.gnu.org
Subject: Re: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 00:59:47 -0400

[Message part 1 (text/plain, inline)]

On Tue, Jul 25, 2017 at 12:12:25AM -0400, Leo Famulari wrote:
> This problem with the wrong number of arguments being passed to
> parse_media() is introduced in the Debian patch, so we should check if
> there is a better version of the patch or if fixes are being applied
> upstream.

Actually, the first issue is that the patch doesn't apply to the source
fully. I tried a later patch from Debian, but it also doesn't apply. I
haven't been able to try fetching a later revision of the net-tools Git
repo, because I first have to make Git not depend on net-tools.

This patch will try to apply the patch and handle the exit codes of
gunzip and patch correctly:

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 456909a94..cab6abb32 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -1220,14 +1217,15 @@ messages and are accompanied by a set of manpages.")
                      (zero? (system* "patch" "-p1" "--force"
                                      "--input" file)))
 
-                   (let ((patch.gz (assoc-ref inputs "patch")))
+                   (let ((patch.gz (assoc-ref inputs "debian-patch")))
                      (format #t "applying Debian patch set '~a'...~%"
                              patch.gz)
-                     (system (string-append "gunzip < " patch.gz " > the-patch"))
-                     (and (apply-patch "the-patch")
-                          (for-each apply-patch
-                                    (find-files "debian/patches"
-                                                "\\.patch")))))
+                     (and
+                       (zero? (system (string-append "gunzip < " patch.gz " > debian-patch")))
+                       (and (apply-patch "debian-patch")
+                            (every apply-patch
+                                      (find-files "debian/patches"
+                                                  "\\.patch"))))))
                  (alist-replace
                   'configure
                   (lambda* (#:key outputs #:allow-other-keys)
@@ -1270,7 +1268,7 @@ messages and are accompanied by a set of manpages.")
 
     ;; Use the big Debian patch set (the thing does not even compile out of
     ;; the box.)
-    (inputs `(("patch" ,(origin
+    (inputs `(("debian-patch" ,(origin
                          (method url-fetch)
                          (uri
                           "http://ftp.de.debian.org/debian/pool/main/n/net-tools/net-tools_1.60-24.2.diff.gz")

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Leo Famulari <leo <at> famulari.name>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 10:27:00 +0200

Hi Leo,

Leo Famulari <leo <at> famulari.name> skribis:

> On Tue, Jul 25, 2017 at 12:12:25AM -0400, Leo Famulari wrote:
>> This problem with the wrong number of arguments being passed to
>> parse_media() is introduced in the Debian patch, so we should check if
>> there is a better version of the patch or if fixes are being applied
>> upstream.
>
> Actually, the first issue is that the patch doesn't apply to the source
> fully. I tried a later patch from Debian, but it also doesn't apply. I
> haven't been able to try fetching a later revision of the net-tools Git
> repo, because I first have to make Git not depend on net-tools.
>
> This patch will try to apply the patch and handle the exit codes of
> gunzip and patch correctly:

Good catch, this LGTM.  I wonder how this went unnoticed for so long!

As for the actual update, the intent is to add a second ‘net-tools’
variant, right?

Thanks for looking into this!

Ludo’.

Message #17 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 12:10:19 -0400

[Message part 1 (text/plain, inline)]

On Tue, Jul 25, 2017 at 10:27:00AM +0200, Ludovic Courtès wrote:
> > This patch will try to apply the patch and handle the exit codes of
> > gunzip and patch correctly:
> 
> Good catch, this LGTM.  I wonder how this went unnoticed for so long!

By handling the exit codes properly, the build fails ;) But I'm working
on that today.

> As for the actual update, the intent is to add a second ‘net-tools’
> variant, right?

Right. I want to be able to update net-tools without rebuilding
everything. So, GnuTLS will use a special variant of net-tools for its
test suite, and the built GnuTLS will not keep any references to
net-tools after it is built.

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 13:53:24 -0400

[Message part 1 (text/plain, inline)]

On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:
> I've attached patches to fix this. First, a patch to create an otherwise
> equivalent package variant and to use it in GnuTLS, and a second patch
> to actually update the public net-tools variable.

I forgot to mention that I've tested these changes by rebuilding, on
x86_64-linux, every package that refers directly to net-tools, and by
reconfiguring my GuixSD system to test the effect of the new net-tools
on wicd. Wicd is still working.

[signature.asc (application/pgp-signature, inline)]

Message #23 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 13:52:02 -0400

[Message part 1 (text/plain, inline)]

I've attached patches to fix this. First, a patch to create an otherwise
equivalent package variant and to use it in GnuTLS, and a second patch
to actually update the public net-tools variable.

Is this the right way to create the test-only package variant for now?

[0001-gnu-net-tools-Add-a-test-only-variant-and-use-it-for.patch (text/plain, attachment)]

[0002-gnu-net-tools-Update-to-1.60-0.90da8a0.patch (text/plain, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #26 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 14:03:35 -0400

[Message part 1 (text/plain, inline)]

On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:
> Is this the right way to create the test-only package variant for now?

I was reminded of the hidden-package procedure. I'll try rewriting this
change using it.

[signature.asc (application/pgp-signature, inline)]

Message #29 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 14:08:08 -0400

[Message part 1 (text/plain, inline)]

On Tue, Jul 25, 2017 at 02:03:35PM -0400, Leo Famulari wrote:
> On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:
> > Is this the right way to create the test-only package variant for now?
> 
> I was reminded of the hidden-package procedure. I'll try rewriting this
> change using it.

I think the order of package inheritance between net-tools and
net-tools-for-tests is the opposite of what's needed for hidden-package.
I'll add a note about using hidden-package in the next core-updates
cycle.

[signature.asc (application/pgp-signature, inline)]

Message #32 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Leo Famulari <leo <at> famulari.name>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 23:30:50 +0200

Leo Famulari <leo <at> famulari.name> skribis:

> On Tue, Jul 25, 2017 at 02:03:35PM -0400, Leo Famulari wrote:
>> On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:
>> > Is this the right way to create the test-only package variant for now?
>> 
>> I was reminded of the hidden-package procedure. I'll try rewriting this
>> change using it.
>
> I think the order of package inheritance between net-tools and
> net-tools-for-tests is the opposite of what's needed for hidden-package.
> I'll add a note about using hidden-package in the next core-updates
> cycle.

‘hidden-package’ simply adds a ‘properties’ field; you could do that “by
hand” and make the property list of ‘net-tools’ empty.

Ludo’.

Message #35 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Leo Famulari <leo <at> famulari.name>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Tue, 25 Jul 2017 23:33:08 +0200

Leo Famulari <leo <at> famulari.name> skribis:

> From 5c738ab1ec5543981c9d5e512ee94d469bcca12b Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo <at> famulari.name>
> Date: Tue, 25 Jul 2017 00:19:56 -0400
> Subject: [PATCH 1/2] gnu: net-tools: Add a test-only variant and use it for
>  GnuTLS.
>
> * gnu/packages/linux.scm (net-tools-for-tests): Add net-tools variant
> for the GnuTLS test suite, et cetera.
> (net-tools): Inherit from net-tools-for-tests.
> * gnu/packages/tls.scm (gnutls)[native-inputs]: Use net-tools-for-tests
> instead of net-tools.
> ---
>  gnu/packages/linux.scm | 5 ++++-
>  gnu/packages/tls.scm   | 4 +++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 456909a94..4f9fa6d25 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -1187,7 +1187,7 @@ consists of several tools, of which the most important are @command{ip} and
>  messages and are accompanied by a set of manpages.")
>      (license license:gpl2+)))
>  
> -(define-public net-tools
> +(define net-tools-for-tests

Use ‘define-public’ here, and perhaps add a comment explaining the intent.

>    ;; XXX: This package is basically unmaintained, but it provides a few
>    ;; commands not yet provided by Inetutils, such as 'route', so we have to
>    ;; live with it.
> @@ -1288,6 +1288,9 @@ network hardware types (plipconfig, slattach) and advanced aspects of IP
>  configuration (iptunnel, ipmaddr).")
>      (license license:gpl2+)))
>  
> +(define-public net-tools
> +  (package (inherit net-tools-for-tests)))

I would tend to do it the other way around, and simply start with:

  (define-public net-tools-for-tests
     net-tools)

and change that whenever we need it.

> +;; TODO Add net-tools to #:disallowed-references when we can afford to rebuild
> +;; GnuTLS.
>  (define-public gnutls
>    (package
>      (name "gnutls")
> @@ -194,7 +196,7 @@ living in the same process.")
>                 "debug"
>                 "doc"))                            ;4.1 MiB of man pages
>      (native-inputs
> -     `(("net-tools" ,net-tools)
> +     `(("net-tools" ,(@@ (gnu packages linux) net-tools-for-tests))

Remove the “@@” once ‘net-tools-for-tests’ is public.

OK with these changes, thank you!

Ludo’.

Message #38 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Wed, 26 Jul 2017 12:08:37 -0400

[Message part 1 (text/plain, inline)]

On Tue, Jul 25, 2017 at 11:33:08PM +0200, Ludovic Courtès wrote:
> Leo Famulari <leo <at> famulari.name> skribis:
> > From 5c738ab1ec5543981c9d5e512ee94d469bcca12b Mon Sep 17 00:00:00 2001
> > From: Leo Famulari <leo <at> famulari.name>
> > Date: Tue, 25 Jul 2017 00:19:56 -0400
> > Subject: [PATCH 1/2] gnu: net-tools: Add a test-only variant and use it for
> >  GnuTLS.
> >
> > * gnu/packages/linux.scm (net-tools-for-tests): Add net-tools variant
> > for the GnuTLS test suite, et cetera.
> > (net-tools): Inherit from net-tools-for-tests.
> > * gnu/packages/tls.scm (gnutls)[native-inputs]: Use net-tools-for-tests
> > instead of net-tools.
> 
> OK with these changes, thank you!

Ah, that's much better! Pushed as
f96dbf6b70364518072c63037d3fca105dfe6497.

[signature.asc (application/pgp-signature, inline)]

Message #41 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Wed, 26 Jul 2017 12:11:46 -0400

[Message part 1 (text/plain, inline)]

On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:
> From f57732d79ac265917f154da5791ff203c82c9733 Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo <at> famulari.name>
> Date: Tue, 25 Jul 2017 01:46:38 -0400
> Subject: [PATCH 2/2] gnu: net-tools: Update to 1.60-0.90da8a0.
> 
> Fixes <https://bugs.gnu.org/27811>.
> 
> * gnu/packages/linux.scm (net-tools): Update to 1.60-0.90da8a0.
> [inputs]: Remove the Debian patch.
> [arguments]: Use modify-phases. Remove the 'patch' phase. Disable
> SELINUX and AFBLUETOOTH from build configuration.

Revisions!

The first patch is the same as before, but the second one uses git-fetch
and builds from the latest upstream commit. I'll squash them if we
choose to build from Git.

[0001-gnu-net-tools-Update-to-1.60-0.90da8a0.patch (text/plain, attachment)]

[0002-gnu-net-tools-Update-to-1.60-1.479bb4a7.patch (text/plain, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #44 received at 27811 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Leo Famulari <leo <at> famulari.name>
Cc: 27811 <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Wed, 26 Jul 2017 23:04:54 +0200

Leo Famulari <leo <at> famulari.name> skribis:

> On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:
>> From f57732d79ac265917f154da5791ff203c82c9733 Mon Sep 17 00:00:00 2001
>> From: Leo Famulari <leo <at> famulari.name>
>> Date: Tue, 25 Jul 2017 01:46:38 -0400
>> Subject: [PATCH 2/2] gnu: net-tools: Update to 1.60-0.90da8a0.
>> 
>> Fixes <https://bugs.gnu.org/27811>.
>> 
>> * gnu/packages/linux.scm (net-tools): Update to 1.60-0.90da8a0.
>> [inputs]: Remove the Debian patch.
>> [arguments]: Use modify-phases. Remove the 'patch' phase. Disable
>> SELINUX and AFBLUETOOTH from build configuration.
>
> Revisions!
>
> The first patch is the same as before, but the second one uses git-fetch
> and builds from the latest upstream commit. I'll squash them if we
> choose to build from Git.
>
> From 6cd4159e355b9414b0678608e74fa53b8e173834 Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo <at> famulari.name>
> Date: Tue, 25 Jul 2017 20:33:27 -0400
> Subject: [PATCH 1/2] gnu: net-tools: Update to 1.60-0.90da8a0.
>
> This leaves net-tools-for-tests unchanged.
>
> Fixes <https://bugs.gnu.org/27811>.
>
> * gnu/packages/linux.scm (net-tools): Update to 1.60-0.90da8a0.
> [inputs]: Remove the Debian patch.
> [arguments]: Use modify-phases. Remove the 'patch' phase. Disable
> SELINUX and AFBLUETOOTH from build configuration.

LGTM.  Using ‘git-fetch’ sounds preferable, so better squash both
patches.

Thank you!

Ludo’.

Message #49 received at 27811-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27811-done <at> debbugs.gnu.org
Subject: Re: bug#27811: net-tools patch has no effect?
Date: Thu, 27 Jul 2017 13:41:58 -0400

[Message part 1 (text/plain, inline)]

On Wed, Jul 26, 2017 at 11:04:54PM +0200, Ludovic Courtès wrote:
> > On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:
> >> From f57732d79ac265917f154da5791ff203c82c9733 Mon Sep 17 00:00:00 2001
> >> From: Leo Famulari <leo <at> famulari.name>
> >> Date: Tue, 25 Jul 2017 01:46:38 -0400
> >> Subject: [PATCH 2/2] gnu: net-tools: Update to 1.60-0.90da8a0.
> >> 
> >> Fixes <https://bugs.gnu.org/27811>.
> >> 
> >> * gnu/packages/linux.scm (net-tools): Update to 1.60-0.90da8a0.
> >> [inputs]: Remove the Debian patch.
> >> [arguments]: Use modify-phases. Remove the 'patch' phase. Disable
> >> SELINUX and AFBLUETOOTH from build configuration.
> >
> > Revisions!
> >
> > The first patch is the same as before, but the second one uses git-fetch
> > and builds from the latest upstream commit. I'll squash them if we
> > choose to build from Git.
> >
> > From 6cd4159e355b9414b0678608e74fa53b8e173834 Mon Sep 17 00:00:00 2001
> > From: Leo Famulari <leo <at> famulari.name>
> > Date: Tue, 25 Jul 2017 20:33:27 -0400
> > Subject: [PATCH 1/2] gnu: net-tools: Update to 1.60-0.90da8a0.
> >
> > This leaves net-tools-for-tests unchanged.
> >
> > Fixes <https://bugs.gnu.org/27811>.
> >
> > * gnu/packages/linux.scm (net-tools): Update to 1.60-0.90da8a0.
> > [inputs]: Remove the Debian patch.
> > [arguments]: Use modify-phases. Remove the 'patch' phase. Disable
> > SELINUX and AFBLUETOOTH from build configuration.
> 
> LGTM.  Using ‘git-fetch’ sounds preferable, so better squash both
> patches.
> 
> Thank you!

Pushed as 6d31c175b2e7b062ce71648595c7af2e1381a036.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.