net-tools patch has no effect?

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.

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.

On Tue, Jul 25, 2017 at 12:12:25AM -0400, Leo Famulari wrote:

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:

Hi Leo,

Leo Famulari <leo@famulari.name> skribis:

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’.

On Tue, Jul 25, 2017 at 10:27:00AM +0200, Ludovic Courtès wrote:

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

on that today.

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.

On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:

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.

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?

From 5c738ab1ec5543981c9d5e512ee94d469bcca12b Mon Sep 17 00:00:00 2001

GnuTLS.

for the GnuTLS test suite, et cetera.

(net-tools): Inherit from 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(-)

On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:

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

change using it.

On Tue, Jul 25, 2017 at 02:03:35PM -0400, Leo Famulari wrote:

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.

Leo Famulari <leo@famulari.name> skribis:

‘hidden-package’ simply adds a ‘properties’ field; you could do that “by

hand” and make the property list of ‘net-tools’ empty.

Ludo’.

Leo Famulari <leo@famulari.name> skribis:

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

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.

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

OK with these changes, thank you!

Ludo’.

On Tue, Jul 25, 2017 at 11:33:08PM +0200, Ludovic Courtès wrote:

Ah, that's much better! Pushed as

f96dbf6b70364518072c63037d3fca105dfe6497.

On Tue, Jul 25, 2017 at 01:52:02PM -0400, Leo Famulari wrote:

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.

Leo Famulari <leo@famulari.name> skribis:

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

patches.

Thank you!

Ludo’.

On Wed, Jul 26, 2017 at 11:04:54PM +0200, Ludovic Courtès wrote:

Pushed as 6d31c175b2e7b062ce71648595c7af2e1381a036.