| Message ID | CAGNyvehn=j+kGhwQ-8E0qOocfdYUDvnXU0tCYnaB2v9847QuLQ@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#53319] gnu: Add n2n. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
Hello, 路辉 <luhux76@gmail.com> writes: > Subject: [PATCH] gnu: Add n2n. Thank you. Some comments follow. > +(define-public n2n-2 I think the variable should be "n2n" only. > + (native-inputs (list autoconf automake)) > + (arguments > + `(#:make-flags (list (string-append "PREFIX=" %output) "CC=gcc") CC=gcc is not cross-compilation friendly. Also, %output is being phased out. I suggest using G-expressions: (arguments (list #:make-flags #~(list (string-append "PREFIX=" #$output) #$(string-append "CC=" (cc-for-target))) ...)) > + #:phases > + (modify-phases %standard-phases If you use G-expressions, you'll need to start with: #~(modify-phases %standard-phases ...) > + (add-before 'configure 'fix-configure > + (lambda* (#:key inputs #:allow-other-keys) > + (substitute* "configure" > + (("/bin/sh") (which "sh")))))) Instead of using `which', you can use `search-input-file': (("/bin/sh") (search-input-file inputs "/bin/sh")) > + #:tests? #f)) ;there is no check target > + (home-page "https://github.com/ntop/n2n") > + (synopsis "Peer-to-peer VPN client and server") > + (description > + "A light VPN software which makes it > +easy to create virtual networks bypassing intermediate firewalls.") Description should consist of full sentences. I suggest: n2n is a light VPN software which makes it easy to create virtual networks bypassing intermediate firewalls. Also, the package brings third-party software: libnatpmp and libupnp. Would it be possible to unbundle them, since Guix already ships both? Could you send an updated patch? Regards,
Hello, 路辉 <luhux76@gmail.com> writes: > Subject: [PATCH] gnu: Add n2n. Thank you. I applied your patch with the changes below. > > +(define-public n2n-2 I renamed it to n2n. > + (native-inputs (list autoconf automake)) I added pkg-config and bash-minimal. > + (arguments > + `(#:make-flags (list (string-append "PREFIX=" %output) "CC=gcc") Using G-expressions I wrote #:make-flags #~(list (string-append "PREFIX=" #$output) (string-append "CC=" #$(cc-for-target))) ... > + (substitute* "configure" > + (("/bin/sh") (which "sh")))))) Here I wrote (("/bin/sh") (search-inputs-file input "/bin/sh")) > + #:tests? #f)) ;there is no check target > + (home-page "https://github.com/ntop/n2n") > + (synopsis "Peer-to-peer VPN client and server") > + (description > + "A light VPN software which makes it I turned the description into complete sentences. Regards,
Nicolas Goaziou schreef op vr 28-01-2022 om 11:10 [+0100]: > > + (add-before 'configure 'fix-configure > > + (lambda* (#:key inputs #:allow-other-keys) > > + (substitute* "configure" > > + (("/bin/sh") (which "sh")))))) > > Instead of using `which', you can use `search-input-file': > > (("/bin/sh") (search-input-file inputs "/bin/sh")) 'configure' is run during build, so for cross-compilation, a sh from 'native-inputs' shoud be used instead of 'inputs': (("/bin/sh") (search-input-file (or native-inputs inputs) "/bin/sh")) or simpler: (("/bin/sh") (which "sh")) Also, this package definition packages version 2.8. Why not package the latest version instead? In the latest version, 'autogen.sh' does not run "./configure" and hence 'move-configure' and 'fix-configure' should not be necessary. Also, looking at <https://github.com/ntop/n2n/blob/472a9878f72299466ddbce2a232ea9e081159fa9/configure.seed#L94>, it seems that n2n might not be bit-for-bit reproducible. Greetings, Maxime
Hello, Maxime Devos <maximedevos@telenet.be> writes: > 'configure' is run during build, so for cross-compilation, a sh from > 'native-inputs' shoud be used instead of 'inputs': > > (("/bin/sh") (search-input-file (or native-inputs inputs) "/bin/sh")) True, I keep forgetting about this. It would be more natural to use (search-input-file native-inputs "/bin/sh"), but, IIRC, native-inputs may be empty if we are not cross-compiling. I will fix it. > or simpler: > > (("/bin/sh") (which "sh")) IIUC, search-input-file is a replacement for `which', so that seems to be going backwards. Of course, if `which' is the preferred solution for package style, I'd love to hear a confirmation about it. > Also, this package definition packages version 2.8. Why not package > the latest version instead? The OP waited one month without any feedback. I consider this is more respectful to apply the patch in its current version rather than requesting more changes now. YMMV. Of course, the update can happen in a later, very welcome, patch. > Also, looking at > <https://github.com/ntop/n2n/blob/472a9878f72299466ddbce2a232ea9e081159fa9/configure.seed#L94>, > it seems that n2n might not be bit-for-bit reproducible. I agree this package has room for improvement. Hopefully, 路辉 can have a look at it. Regards,
diff --git a/gnu/packages/vpn.scm b/gnu/packages/vpn.scm index 4ad555ef1b..542d6518fd 100644 --- a/gnu/packages/vpn.scm +++ b/gnu/packages/vpn.scm @@ -18,6 +18,7 @@ ;;; Copyright © 2021 Domagoj Stolfa <ds815@gmx.com> ;;; Copyright © 2021 Raghav Gururajan <rg@raghavgururajan.name> ;;; Copyright © 2021 jgart <jgart@dismail.de> +;;; Copyright © 2022 Lu hui <luhux76@gmail.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -1093,3 +1094,39 @@ (define-public xl2tpd "xl2tpd is an implementation of the Layer 2 Tunnelling Protocol (RFC 2661). L2TP allows you to tunnel PPP over UDP.") (license license:gpl2))) + +(define-public n2n-2 + (package + (name "n2n") + (version "2.8") + (source (origin + (method git-fetch) + (uri (git-reference + (url "https://github.com/ntop/n2n") + (commit version))) + (file-name (git-file-name name version)) + (sha256 + (base32 + "1ph2npvnqh1xnmkp96pdzpxm033jkb8zznd3nc59l9arhn0pq4nv")))) + (build-system gnu-build-system) + (native-inputs (list autoconf automake)) + (arguments + `(#:make-flags (list (string-append "PREFIX=" %output) "CC=gcc") + #:phases + (modify-phases %standard-phases + (add-before 'bootstrap 'move-configure + ;; don't execute configure script in bootstrap + (lambda* (#:key inputs #:allow-other-keys) + (substitute* "autogen.sh" + (("./configure") "")))) + (add-before 'configure 'fix-configure + (lambda* (#:key inputs #:allow-other-keys) + (substitute* "configure" + (("/bin/sh") (which "sh")))))) + #:tests? #f)) ;there is no check target + (home-page "https://github.com/ntop/n2n") + (synopsis "Peer-to-peer VPN client and server") + (description + "A light VPN software which makes it +easy to create virtual networks bypassing intermediate firewalls.")
From c9d69917251e377c3291443dda0090cfa5e46956 Mon Sep 17 00:00:00 2001 From: Lu Hui <luhux76@gmail.com> Date: Mon, 17 Jan 2022 10:48:44 +0800 Subject: [PATCH] gnu: Add n2n. * gnu/packages/vpn.scm (n2n-2): New variable --- gnu/packages/vpn.scm | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) + (license license:gpl3+)))