| Message ID | 834a3cc5e7e16f94691c1bb726f1dd1cc5edb6d0.1708638616.git.steve@futurile.net |
|---|---|
| State | New |
| Headers | show |
| Series | re: add x2goclient and nx-libs | expand |
Hi! Thank you Steve for shepherding this patch set! Here are some comments on minor issues. Steve George <steve@futurile.net> skribis: > From: nicodebo <nicolas.debonnaire@gmail.com> > > * gnu/packages/nx.scm: New file. > * gnu/packages/nx.scm (nx-libs): New variable. The second line is unnecessary. Please make sure to add the file to ‘gnu/local.mk’. > +++ b/gnu/packages/nx.scm > @@ -0,0 +1,141 @@ > +;;; This file is part of GNU Guix. Looks like a couple of lines are missing from the usual header. > + (add-before 'build 'replace-shebang-nxdialog > + (lambda _ > + (substitute* "nxdialog/bin/nxdialog" > + (("#!/usr/bin/env python") > + (string-append "#!" > + (which "python3")))))) ‘which’ returns ‘python3’ as found in $PATH—i.e., the native python. To do the right thing in a cross-compilation context (for example, pick the AArch64 ‘python3’ binary if you’re cross-compiling to AArch64), it should be written like this: (lambda* (#:key inputs #:allow-other-keys) (substitute* … (… … (search-input-file inputs "/bin/python3")))) > + (synopsis "NX remote desktop library") > + (description > + "NX technology also known as NX or NoMachine is a protocol that enables > +remote desktop access. It's maintained by the artica project, x2go > +developers and TheQVD.") Please describe that the package does; who maintains it is usually not relevant here: https://guix.gnu.org/manual/devel/en/html_node/Synopses-and-Descriptions.html Thanks, Ludo’.
Hi Steve and Ludovic, I had started reviewing this work as a diff with the intent to merge it before Ludovic tipped in; apologies for not signaling this earlier. Ludovic Courtès <ludo@gnu.org> writes: > Hi! > > Thank you Steve for shepherding this patch set! Here are some comments > on minor issues. > > Steve George <steve@futurile.net> skribis: > >> From: nicodebo <nicolas.debonnaire@gmail.com> >> >> * gnu/packages/nx.scm: New file. >> * gnu/packages/nx.scm (nx-libs): New variable. > > The second line is unnecessary. > > Please make sure to add the file to ‘gnu/local.mk’. Done. >> +++ b/gnu/packages/nx.scm >> @@ -0,0 +1,141 @@ >> +;;; This file is part of GNU Guix. > > Looks like a couple of lines are missing from the usual header. > >> + (add-before 'build 'replace-shebang-nxdialog >> + (lambda _ >> + (substitute* "nxdialog/bin/nxdialog" >> + (("#!/usr/bin/env python") >> + (string-append "#!" >> + (which "python3")))))) > > ‘which’ returns ‘python3’ as found in $PATH—i.e., the native python. > > To do the right thing in a cross-compilation context (for example, pick > the AArch64 ‘python3’ binary if you’re cross-compiling to AArch64), it > should be written like this: > > (lambda* (#:key inputs #:allow-other-keys) > (substitute* … > (… … (search-input-file inputs "/bin/python3")))) Done. >> + (synopsis "NX remote desktop library") >> + (description >> + "NX technology also known as NX or NoMachine is a protocol that enables >> +remote desktop access. It's maintained by the artica project, x2go >> +developers and TheQVD.") > > Please describe that the package does; who maintains it is usually not > relevant here: > > https://guix.gnu.org/manual/devel/en/html_node/Synopses-and-Descriptions.html Done. Here's the full diff from my changes from v1: --8<---------------cut here---------------start------------->8--- modified gnu/local.mk @@ -487,6 +487,7 @@ GNU_SYSTEM_MODULES = \ %D%/packages/ntp.scm \ %D%/packages/nutrition.scm \ %D%/packages/nvi.scm \ + %D%/packages/nx.scm \ %D%/packages/ocaml.scm \ %D%/packages/ocr.scm \ %D%/packages/openkinect.scm \ modified gnu/packages/nx.scm @@ -1,4 +1,6 @@ ;;; This file is part of GNU Guix. +;;; Copyright © 2024 Nicolas Debonnaire <nicolas.debonnaire@gmail.com> +;;; Copyright © 2024 Maxim Cournoyer <maxim.cournoyer@gmail.com> ;;; ;;; GNU Guix is free software; you can redistribute it and/or modify it ;;; under the terms of the GNU General Public License as published by @@ -14,13 +16,14 @@ ;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>. (define-module (gnu packages nx) - #:use-module (guix build utils) #:use-module (guix build-system gnu) #:use-module (guix download) + #:use-module (guix gexp) #:use-module (guix git-download) #:use-module ((guix licenses) #:prefix license:) #:use-module (guix packages) #:use-module (gnu packages autotools) + #:use-module (gnu packages base) #:use-module (gnu packages commencement) #:use-module (gnu packages cups) #:use-module (gnu packages glib) @@ -54,95 +57,104 @@ (define-public nx-libs (base32 "0ykbza39ksycpyydaiwwbp7hkmdk96v7b36pn989k39lhfwnn8kz")))) (build-system gnu-build-system) (arguments - `(#:tests? #f ;; no tests in this project - #:phases (modify-phases %standard-phases - (delete 'configure) - (add-before 'build 'replace-shebang-nxdialog - (lambda _ - (substitute* "nxdialog/bin/nxdialog" - (("#!/usr/bin/env python") - (string-append "#!" - (which "python3")))))) - (add-before 'build 'fix-makefile - (lambda* (#:key outputs #:allow-other-keys) - (let* ((out (assoc-ref outputs "out")) - (etc (string-append out "/etc"))) - (mkdir-p etc) - (substitute* "Makefile" - (("^gzip -f") "gzip -f -n") ;; reproducible build - (("/usr/local") - out) - (("/etc/nxagent") - etc) - (("./configure") - (string-append "CONFIG_SHELL=" - (which "sh") - " SHELL=" - (which "sh") - " " - (which "sh") - " ./configure"))) - (substitute* '("nx-X11/config/cf/site.def") - (("#include <host.def>") - (string-append "#include <host.def>" "\n" - "#define BourneShell " - (which "sh"))) - (("/usr/local") - out)) - ;; Workaround the following error during the build - ;; process: invalid input: ("_" #<procedure which (program)>) - (substitute* '("nx-X11/Makefile") - (("which") - "echo")) - #t))) - (replace 'build - (lambda _ - (invoke "make" "IMAKE_DEFINES=\"-DUseTIRPC=1\"") #t)) - (add-before 'validate-runpath 'link-libraries - (lambda* (#:key outputs #:allow-other-keys) - (let* ((out (assoc-ref outputs "out")) - (lib (string-append out "/lib")) - (lib-nx-x11 (string-append lib "/nx/X11"))) - ;; TODO: Should I use symlink instead of install-file ? - (for-each (lambda (file) - (install-file file lib-nx-x11)) - (list (string-append lib "/libXcomp.so.3") - (string-append lib - "/libXcompshad.so.3") - (string-append lib "/libNX_X11.so.6"))) - #t)))))) - (inputs (list perl - python - python-pygobject - gtk+)) - (native-inputs (list autoconf - automake - libtool - pkg-config - libpng - libjpeg-turbo - xkbcomp - libxfont - libxdamage - libxrandr - libxtst - libxext - libxcomposite - libxinerama - libxpm - libxml2 - font-util - pixman - gccmakedep - imake - quilt - libtirpc - gcc-toolchain)) - (synopsis "NX remote desktop library") - (description - "NX technology also known as NX or NoMachine is a protocol that enables -remote desktop access. It's maintained by the artica project, x2go -developers and TheQVD.") + (list + #:tests? #f ;no test suite + ;; The build randomly fails when run in parallel (see: + ;; https://github.com/ArcticaProject/nx-libs/issues/1072). + #:parallel-build? #f + #:make-flags #~(let ((sh (search-input-file %build-inputs "bin/sh"))) + (list (string-append "PREFIX=" #$output) + (string-append "ETCDIR_NX=" #$output "/etc") + (string-append "LOCAL_LDFLAGS=-Wl," + "-rpath=" #$output "/lib," + "-rpath=" #$output "/lib/nx/X11") + (string-append "IMAKE_DEFINES=-DUseTIRPC=1" + " -DBourneShell=" sh + " -DProjectRoot=" #$output + " -DDefaultUserPath=" + #$output "/bin") + (string-append "CONFIG_SHELL=" sh) + (string-append "SHELL=" sh) + ;; Autoreconf being run by Make, the generated + ;; configure script shebangs thus haven't been + ;; patched; workaround this by running explicitly + ;; via the shell. + (string-append "CONFIGURE=" sh " ./configure " + "--prefix=" #$output) + "VERBOSE=1")) + #:phases #~(modify-phases %standard-phases + (delete 'configure) + (add-before 'build 'adjust-Makefile + (lambda _ + ;; It's best to source a script via its absolute path, + ;; otherwise it's looked from PATH and fails for POSIX + ;; shells, such as our Bash-provided 'sh' (see: + ;; https://github.com/ArcticaProject/nx-libs/issues/1071). + (substitute* "Makefile" + (("\\. replace.sh") + ". ./replace.sh")))) + (add-after 'install 'wrap-nxdialog + (lambda* (#:key inputs #:allow-other-keys) + (wrap-program (string-append #$output "/bin/nxdialog") + `("GUIX_PYTHONPATH" = + (,(getenv "GUIX_PYTHONPATH"))) + ;; Ensure GObject Introspection typelibs are found. + `("GI_TYPELIB_PATH" ":" = + (,(getenv "GI_TYPELIB_PATH"))))))))) + (native-inputs + (list autoconf + automake + font-util + gccmakedep + imake + libtool + perl + pkg-config + quilt + which)) + (inputs + (list gtk+ + libjpeg-turbo + libtirpc + libxcomposite + libxdamage + libxext + libxfont + libxinerama + libxml2 + libxpm + libxrandr + libxtst + pixman + python-pygobject + python-wrapper + xkbcomp)) + (propagated-inputs + (list libpng)) ;in Requires of nxcomp.pc + (synopsis "NX X11 protocol compression libraries") + (description "NX is a software suite which implements very efficient +compression of the X11 protocol. This increases performance when using X +applications over a network, especially a slow one. This package provides the +following libraries: +@table @code +@item NX_X11 +NX's modified X Window System (X11) library +@item Xcomp +NX differential compression library for X11 +@item Xcompshad +Session shadowing library +@end table + +The following commands are also provided: + +@table @command +@item nxagent +Agent providing NX transport of X sessions +@item nxproxy +The NX proxy (client) binary +@item nxdialog +Helper script +@end table") (home-page "https://github.com/ArcticaProject/nx-libs") (license license:gpl2))) --8<---------------cut here---------------end--------------->8--- Changes summary: 1. Move most libraries to inputs. 2. Remove gcc-toolchain from native-inputs. 3. Use gexps / search-input-file instead of 'which' 4. Expound description 5. Wrap Python script with needed environment variables 6. Disable parallel build, which was randomly causing build failures 7. Use make flags instead of modifying the Makefile. I think that's about it! I'll push shortly. Thanks to Steve for the initial review, and for Ludo for providing extra tips.
diff --git a/gnu/packages/nx.scm b/gnu/packages/nx.scm new file mode 100644 index 00000000000..92542c02a7b --- /dev/null +++ b/gnu/packages/nx.scm @@ -0,0 +1,141 @@ +;;; This file is part of GNU Guix. +;;; +;;; GNU Guix is free software; you can redistribute it and/or modify it +;;; under the terms of the GNU General Public License as published by +;;; the Free Software Foundation; either version 3 of the License, or (at +;;; your option) any later version. +;;; +;;; GNU Guix is distributed in the hope that it will be useful, but +;;; WITHOUT ANY WARRANTY; without even the implied warranty of +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;;; GNU General Public License for more details. +;;; +;;; You should have received a copy of the GNU General Public License +;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>. + +(define-module (gnu packages nx) + #:use-module (guix build utils) + #:use-module (guix build-system gnu) + #:use-module (guix git-download) + #:use-module ((guix licenses) #:prefix license:) + #:use-module (guix packages) + #:use-module (gnu packages autotools) + #:use-module (gnu packages commencement) + #:use-module (gnu packages glib) + #:use-module (gnu packages gtk) + #:use-module (gnu packages image) + #:use-module (gnu packages onc-rpc) + #:use-module (gnu packages patchutils) + #:use-module (gnu packages perl) + #:use-module (gnu packages pkg-config) + #:use-module (gnu packages python) + #:use-module (gnu packages xdisorg) + #:use-module (gnu packages xml) + #:use-module (gnu packages xorg)) + +(define-public nx-libs + (package + (name "nx-libs") + (version "3.5.99.27") + (source + (origin + (method git-fetch) + (uri (git-reference + (url "https://github.com/ArcticaProject/nx-libs") + (commit version))) + (file-name (git-file-name name version)) + (sha256 + (base32 "0ykbza39ksycpyydaiwwbp7hkmdk96v7b36pn989k39lhfwnn8kz")))) + (build-system gnu-build-system) + (arguments + `(#:tests? #f ;; no tests in this project + #:phases (modify-phases %standard-phases + (delete 'configure) + (add-before 'build 'replace-shebang-nxdialog + (lambda _ + (substitute* "nxdialog/bin/nxdialog" + (("#!/usr/bin/env python") + (string-append "#!" + (which "python3")))))) + (add-before 'build 'fix-makefile + (lambda* (#:key outputs #:allow-other-keys) + (let* ((out (assoc-ref outputs "out")) + (etc (string-append out "/etc"))) + (mkdir-p etc) + (substitute* "Makefile" + (("^gzip -f") "gzip -f -n") ;; reproducible build + (("/usr/local") + out) + (("/etc/nxagent") + etc) + (("./configure") + (string-append "CONFIG_SHELL=" + (which "sh") + " SHELL=" + (which "sh") + " " + (which "sh") + " ./configure"))) + (substitute* '("nx-X11/config/cf/site.def") + (("#include <host.def>") + (string-append "#include <host.def>" "\n" + "#define BourneShell " + (which "sh"))) + (("/usr/local") + out)) + ;; Workaround the following error during the build + ;; process: invalid input: ("_" #<procedure which (program)>) + (substitute* '("nx-X11/Makefile") + (("which") + "echo")) + #t))) + (replace 'build + (lambda _ + (invoke "make" "IMAKE_DEFINES=\"-DUseTIRPC=1\"") #t)) + (add-before 'validate-runpath 'link-libraries + (lambda* (#:key outputs #:allow-other-keys) + (let* ((out (assoc-ref outputs "out")) + (lib (string-append out "/lib")) + (lib-nx-x11 (string-append lib "/nx/X11"))) + ;; TODO: Should I use symlink instead of install-file ? + (for-each (lambda (file) + (install-file file lib-nx-x11)) + (list (string-append lib "/libXcomp.so.3") + (string-append lib + "/libXcompshad.so.3") + (string-append lib "/libNX_X11.so.6"))) + #t)))))) + (inputs (list perl + python + python-pygobject + gtk+)) + (native-inputs (list autoconf + automake + libtool + pkg-config + libpng + libjpeg-turbo + xkbcomp + libxfont + libxdamage + libxrandr + libxtst + libxext + libxcomposite + libxinerama + libxpm + libxml2 + font-util + pixman + gccmakedep + imake + quilt + libtirpc + gcc-toolchain)) + (synopsis "NX remote desktop library") + (description + "NX technology also known as NX or NoMachine is a protocol that enables +remote desktop access. It's maintained by the artica project, x2go +developers and TheQVD.") + (home-page "https://github.com/ArcticaProject/nx-libs") + (license license:gpl2)))