Subject: Re: bug#58341: [PATCH] Add x11vnc recipe
From: Maxim Cournoyer <[email-masked]>
To: Mehmet Tekman <[email-masked]>
Cc: [email-masked]
Date: Mon, 31 Oct 2022 11:36:37 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi Mehmet,

Mehmet Tekman <[email-masked]> writes:

> Hello, thank you for the review!

Thanks for the prompt reply, and sorry for the delays on my part!

> I've put the recipe into vnc.scm and built to a newer commit that
> makes the patches redundant (thanks for the tip, I was basing my
> recipe off one from another distro).
>
> I've fixed the commit message and linted and styled the vnc.scm file.
>
> I created a new branch and put my changes there, so please disregard
> the above changes.
>
> I attach here the patch file, as I don't quite trust myself to paste
> the changes into this email.

That's fine.  It looks very good!  I've made the following
cosmetic/nitpicking changes:

--8<---------------cut here---------------start------------->8---
modified   gnu/packages/vnc.scm
@@ -568,11 +568,10 @@ (define-public x11vnc
   ;; The release version of 0.9.16 requires patches to work, so we pin to the
   ;; latest working commit
   (let ((commit "3e4dc8ef2985a6e670e1d9649fe55395c2b31039")
-        (version "0.9.16")
-        (revision "1"))
+        (revision "0"))
     (package
       (name "x11vnc")
-      (version (git-version version revision commit))
+      (version (git-version "0.9.16" revision commit))
       (source (origin
                 (method git-fetch)
                 (uri (git-reference
@@ -584,14 +583,15 @@ (define-public x11vnc
                   "0a120gv9h3whiznlddl0j3nz3400jjgl97znaincm5i2m5pnjifs"))))
       (build-system gnu-build-system)
       (arguments
-       '(#:phases (modify-phases %standard-phases
-                    (add-before 'bootstrap 'delete-premature-configure
-                      (lambda _
-                        (substitute* "./autogen.sh"
-                          ((".*/configure")
-                           "")))))))
+       (list #:phases #~(modify-phases %standard-phases
+                          (add-before 'bootstrap 'delete-premature-configure
+                            (lambda _
+                              (substitute* "./autogen.sh"
+                                ((".*/configure")
+                                 "")))))))
       (native-inputs (list autoconf automake autobuild pkg-config))
       (inputs (list avahi
+                    libjpeg-turbo
                     libvnc
                     libx11
                     libxcomposite
@@ -604,9 +604,8 @@ (define-public x11vnc
                     libxtst
                     openssl
                     xdpyinfo
-                    zlib
-                    libjpeg-turbo
-                    xf86-video-dummy))
+                    xf86-video-dummy
+                    zlib))
       (synopsis "VNC server for real X displays")
       (home-page "https://github.com/LibVNC/x11vnc")
       (description
@@ -615,7 +614,6 @@ (define-public x11vnc
 mouse) with any VNC viewer.")
       (license license:gpl2))))
 
-
 (define-public libvnc
   (package
     (name "libvnc")
--8<---------------cut here---------------end--------------->8---

1. Do not bind a "version" variable in let; it's bound by the version
field of the package record at definition (see: define-record-type* from
(guix records)).

2. Used a plain list with a gexp for the phases, which is more
forward-looking.

3. Sorted the inputs.

4. Removed an extra newline at the end (we use only one newline between
definitions).

Before pushing, I've also double checked the license and found that it
should be gpl2+, as the COPYING file contains the "any later version"
text.  x11vnc.h also has in the license notice:

    x11vnc 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 2 of the License, or (at
    your option) any later version.

I made these changes and pushed to master!

Thanks again,

Maxim