[PATCH] gnu: vsftpd: Use CentOS version and patches.

  • Done
  • quality assurance status badge
Details
2 participants
  • david larsson
  • Tobias Geerinckx-Rice
Owner
unassigned
Submitted by
david larsson
Severity
normal
D
D
D
david larsson wrote on 30 Mar 2021 11:20
(address . 47495@debbugs.gnu.org)(name . Guix-patches)(address . guix-patches-bounces+david.larsson=selfhosted.xyz@gnu.org)
08d5f3aefaeff390aa73a1e88bd64e13@selfhosted.xyz
On 2021-03-30 09:52, david larsson wrote:
Toggle quote (5 lines)
> Hi,
> the attached patch updates vsftpd so it can use tlsv1.2 etc.
>
> //David

Sorry, that was the wrong patch that got attached. I have attached the
correct one now, and pasted below:

From 10868d1d6e705abc9e1d5744f6eea321f3dafc64 Mon Sep 17 00:00:00 2001
From: methuselah-0 <david.larsson@selfhosted.xyz>
Date: Tue, 30 Mar 2021 11:18:09 +0200
Subject: [PATCH] gnu: vsftpd: Use CentOS version and patches.

* gnu/packages/ftp.scm (vftpd): Use CentOS version and patches.
---
gnu/packages/ftp.scm | 185 +++++++++++++++++++++++++++++++++++--------
1 file changed, 150 insertions(+), 35 deletions(-)

Toggle diff (450 lines)
diff --git a/gnu/packages/ftp.scm b/gnu/packages/ftp.scm
index b178063556..1c2c8119c7 100644
--- a/gnu/packages/ftp.scm
+++ b/gnu/packages/ftp.scm
@@ -28,18 +28,21 @@
#:use-module (gnu packages)
#:use-module (gnu packages autotools)
#:use-module (gnu packages check)
+ #:use-module (gnu packages cpio)
#:use-module (gnu packages compression)
#:use-module (gnu packages freedesktop)
#:use-module (gnu packages gettext)
#:use-module (gnu packages glib)
#:use-module (gnu packages gtk)
#:use-module (gnu packages libidn)
+ #:use-module (gnu packages linux)
#:use-module (gnu packages ncurses)
#:use-module (gnu packages nettle)
#:use-module (gnu packages pkg-config)
#:use-module (gnu packages readline)
#:use-module (gnu packages sqlite)
#:use-module (gnu packages tls)
+ #:use-module (gnu packages version-control)
#:use-module (gnu packages wxwidgets)
#:use-module (gnu packages xml))

@@ -251,40 +254,152 @@ directory comparison and more.")
(properties '((upstream-name . "FileZilla")))))

(define-public vsftpd
- (package
- (name "vsftpd")
- (version "3.0.3")
- (source (origin
- (method url-fetch)
- (uri (string-append
"https://security.appspot.com/downloads/"
- name "-" version ".tar.gz"))
- (sha256
- (base32
-
"1xsyjn68k3fgm2incpb3lz2nikffl9by2safp994i272wvv2nkcx"))))
- (build-system gnu-build-system)
- (arguments
- `(#:make-flags '("LDFLAGS=-lcrypt")
- #:tests? #f ; No tests exist.
- #:phases
- (modify-phases %standard-phases
- (add-after 'unpack 'patch-installation-directory
- (lambda* (#:key outputs #:allow-other-keys)
- (substitute* "Makefile"
- (("/usr") (assoc-ref outputs "out")))
- #t))
- (add-before 'install 'mkdir
- (lambda* (#:key outputs #:allow-other-keys)
- (let ((out (assoc-ref outputs "out")))
- (mkdir-p out)
- (mkdir (string-append out "/sbin"))
- (mkdir (string-append out "/man"))
- (mkdir (string-append out "/man/man5"))
- (mkdir (string-append out "/man/man8"))
- #t)))
- (delete 'configure))))
- (synopsis "vsftpd FTP daemon")
- (description "@command{vsftpd} is a daemon that listens on a TCP
socket
+ (let ((version "3.0.3")
+ (revision "32")
+ (centos-version "8.3.2011"))
+ (package
+ (name "vsftpd")
+ (version version)
+ (source (origin
+ (method url-fetch)
+ (uri (string-append
+ "https://vault.centos.org/centos/" centos-version
+ "/AppStream/Source/SPackages/vsftpd-" version "-"
+ revision ".el8.src.rpm"))
+ (sha256
+ (base32
+
"1xl0kqcismf82hl99klqbvvpylpyk1yr1qjy5hd8f80cj4lyl0f4"))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:make-flags '("LDFLAGS=-lcrypt -lssl -pie")
+ #:tests? #f ; No tests exist.
+ #:phases
+ (modify-phases %standard-phases
+ (add-after 'unpack 'patch-installation-directory
+ (lambda* (#:key outputs #:allow-other-keys)
+ (substitute* "Makefile"
+ (("/usr") (assoc-ref outputs "out")))
+ #t))
+ (replace 'unpack
+ (lambda* (#:key source #:allow-other-keys)
+ (let ((version "3.0.3")
+ (revision "32")
+ (centos-version "8.3.2011"))
+
+ (invoke "7z" "e" source (string-append "-o"
"./vsftpd-"
+ version "-"
+ revision
".el8.src.cpio"))
+ (chdir (string-append "./vsftpd-" version "-"
+ revision ".el8.src.cpio"))
+ (invoke "cpio" "-idmv" (string-append
"--file=./vsftpd-"
+ version "-"
+ revision
".el8.src.cpio"))
+ (invoke "tar" "xvf" (string-append "./vsftpd-"
version ".tar.gz"))
+ (let ((patches
+
'("0001-Don-t-use-the-provided-script-to-locate-libraries.patch"
+ "0002-Enable-build-with-SSL.patch"
+ "0003-Enable-build-with-TCP-Wrapper.patch"
+
"0004-Use-etc-vsftpd-dir-for-config-files-instead-of-etc.patch"
+
"0005-Use-hostname-when-calling-PAM-authentication-module.patch"
+
"0006-Close-stdin-out-err-before-listening-for-incoming-co.patch"
+ "0007-Make-filename-filters-smarter.patch"
+ "0008-Write-denied-logins-into-the-log.patch"
+
"0009-Trim-whitespaces-when-reading-configuration.patch"
+ "0010-Improve-daemonizing.patch"
+ "0011-Fix-listing-with-more-than-one-star.patch"
+
"0012-Replace-syscall-__NR_clone-.-with-clone.patch"
+ "0013-Extend-man-pages-with-systemd-info.patch"
+
"0014-Add-support-for-square-brackets-in-ls.patch"
+ "0015-Listen-on-IPv6-by-default.patch"
+
"0016-Increase-VSFTP_AS_LIMIT-from-200UL-to-400UL.patch"
+
"0017-Fix-an-issue-with-timestamps-during-DST.patch"
+
"0018-Change-the-default-log-file-in-configuration.patch"
+
"0019-Introduce-reverse_lookup_enable-option.patch"
+
"0020-Use-unsigned-int-for-uid-and-gid-representation.patch"
+
"0021-Introduce-support-for-DHE-based-cipher-suites.patch"
+
"0022-Introduce-support-for-EDDHE-based-cipher-suites.patch"
+
"0023-Add-documentation-for-isolate_-options.-Correct-defa.patch"
+ "0024-Introduce-new-return-value-450.patch"
+ "0025-Improve-local_max_rate-option.patch"
+ "0026-Prevent-hanging-in-SIGCHLD-handler.patch"
+ "0027-Delete-files-when-upload-fails.patch"
+ "0028-Fix-man-page-rendering.patch"
+ "0029-Fix-segfault-in-config-file-parser.patch"
+
"0030-Fix-logging-into-syslog-when-enabled-in-config.patch"
+
"0031-Fix-question-mark-wildcard-withing-a-file-name.patch"
+
"0032-Propagate-errors-from-nfs-with-quota-to-client.patch"
+
"0033-Introduce-TLSv1.1-and-TLSv1.2-options.patch"
+
"0034-Turn-off-seccomp-sandbox-because-it-is-too-strict.patch"
+
"0035-Modify-DH-enablement-patch-to-build-with-OpenSSL-1.1.patch"
+ "0036-Redefine-VSFTP_COMMAND_FD-to-1.patch"
+
"0037-Document-the-relationship-of-text_userdb_names-and-c.patch"
+
"0038-Document-allow_writeable_chroot-in-the-man-page.patch"
+
"0039-Improve-documentation-of-ASCII-mode-in-the-man-page.patch"
+ "0040-Use-system-wide-crypto-policy.patch"
+
"0041-Document-the-new-default-for-ssl_ciphers-in-the-man-.patch"
+
"0042-When-handling-FEAT-command-check-ssl_tlsv1_1-and-ssl.patch"
+ "0043-Enable-only-TLSv1.2-by-default.patch"
+
"0044-Disable-anonymous_enable-in-default-config-file.patch"
+
"0045-Expand-explanation-of-ascii_-options-behaviour-in-ma.patch"
+
"0046-vsftpd.conf-Refer-to-the-man-page-regarding-the-asci.patch"
+ "0047-Disable-tcp_wrappers-support.patch"
+
"0048-Fix-default-value-of-strict_ssl_read_eof-in-man-page.patch"
+
"0049-Add-new-filename-generation-algorithm-for-STOU-comma.patch"
+ "0050-Don-t-link-with-libnsl.patch"
+
"0051-Improve-documentation-of-better_stou-in-the-man-page.patch"
+ "0052-Fix-rDNS-with-IPv6.patch"
+ "0053-Always-do-chdir-after-chroot.patch"
+
"0054-vsf_sysutil_rcvtimeo-Check-return-value-of-setsockop.patch"
+
"0055-vsf_sysutil_get_tz-Check-the-return-value-of-syscall.patch"
+ "0056-Log-die-calls-to-syslog.patch"
+
"0057-Improve-error-message-when-max-number-of-bind-attemp.patch"
+
"0058-Make-the-max-number-of-bind-retries-tunable.patch"
+
"0059-Fix-SEGFAULT-when-running-in-a-container-as-PID-1.patch"
+
"0001-Move-closing-standard-FDs-after-listen.patch"
+ "0002-Prevent-recursion-in-bug.patch"
+
"0001-Set-s_uwtmp_inserted-only-after-record-insertion-rem.patch"
+
"0002-Repeat-pututxline-if-it-fails-with-EINTR.patch"
+
"0003-Repeat-pututxline-until-it-succeeds-if-it-fails-with.patch"
+ "0001-Fix-timestamp-handling-in-MDTM.patch"
+
"0001-Remove-a-hint-about-the-ftp_home_dir-SELinux-boolean.patch")))
+ (map (lambda (x) (invoke "mv" (string-append "./"
x)
+ (string-append "vsftpd-"
version "/")))
+ patches)
+ (chdir (string-append "./vsftpd-" version))
+ (invoke "git" "init" ".")
+ (invoke "git" "config" "user.email"
"you@example.com")
+ (invoke "git" "config" "user.name" "Your Name" )
+ (invoke "git" "add" ".")
+ (invoke "git" "commit" "-m" "first")
+ (map (lambda (x) (invoke "git" "am" (string-append
"./" x))) patches)
+ (map (lambda (x) (invoke "rm" (string-append "./"
x))) patches)
+ (invoke "rm" "-rf" "./.git")
+ (chdir "../")
+ (invoke "mv" (string-append "./vsftpd-" version)
"../")
+ (chdir "../")
+ (invoke "rm" "-rf" (string-append "./vsftpd-"
version "-"
+ revision
".el8.src.cpio"))
+ (chdir (string-append "./vsftpd-" version)))
+ #t)))
+ (add-before 'install 'mkdirFrom
10868d1d6e705abc9e1d5744f6eea321f3dafc64 Mon Sep 17 00:00:00 2001
From: methuselah-0 <david.larsson@selfhosted.xyz>
Date: Tue, 30 Mar 2021 11:18:09 +0200
Subject: [PATCH] gnu: vsftpd: Use CentOS version and patches.

* gnu/packages/ftp.scm (vftpd): Use CentOS version and patches.
---
gnu/packages/ftp.scm | 185 +++++++++++++++++++++++++++++++++++--------
1 file changed, 150 insertions(+), 35 deletions(-)

diff --git a/gnu/packages/ftp.scm b/gnu/packages/ftp.scm
index b178063556..1c2c8119c7 100644
--- a/gnu/packages/ftp.scm
+++ b/gnu/packages/ftp.scm
@@ -28,18 +28,21 @@
#:use-module (gnu packages)
#:use-module (gnu packages autotools)
#:use-module (gnu packages check)
+ #:use-module (gnu packages cpio)
#:use-module (gnu packages compression)
#:use-module (gnu packages freedesktop)
#:use-module (gnu packages gettext)
#:use-module (gnu packages glib)
#:use-module (gnu packages gtk)
#:use-module (gnu packages libidn)
+ #:use-module (gnu packages linux)
#:use-module (gnu packages ncurses)
#:use-module (gnu packages nettle)
#:use-module (gnu packages pkg-config)
#:use-module (gnu packages readline)
#:use-module (gnu packages sqlite)
#:use-module (gnu packages tls)
+ #:use-module (gnu packages version-control)
#:use-module (gnu packages wxwidgets)
#:use-module (gnu packages xml))

@@ -251,40 +254,152 @@ directory comparison and more.")
(properties '((upstream-name . "FileZilla")))))

(define-public vsftpd
- (package
- (name "vsftpd")
- (version "3.0.3")
- (source (origin
- (method url-fetch)
- (uri (string-append
"https://security.appspot.com/downloads/"
- name "-" version ".tar.gz"))
- (sha256
- (base32
-
"1xsyjn68k3fgm2incpb3lz2nikffl9by2safp994i272wvv2nkcx"))))
- (build-system gnu-build-system)
- (arguments
- `(#:make-flags '("LDFLAGS=-lcrypt")
- #:tests? #f ; No tests exist.
- #:phases
- (modify-phases %standard-phases
- (add-after 'unpack 'patch-installation-directory
- (lambda* (#:key outputs #:allow-other-keys)
- (substitute* "Makefile"
- (("/usr") (assoc-ref outputs "out")))
- #t))
- (add-before 'install 'mkdir
- (lambda* (#:key outputs #:allow-other-keys)
- (let ((out (assoc-ref outputs "out")))
- (mkdir-p out)
- (mkdir (string-append out "/sbin"))
- (mkdir (string-append out "/man"))
- (mkdir (string-append out "/man/man5"))
- (mkdir (string-append out "/man/man8"))
- #t)))
- (delete 'configure))))
- (synopsis "vsftpd FTP daemon")
- (description "@command{vsftpd} is a daemon that listens on a TCP
socket
+ (let ((version "3.0.3")
+ (revision "32")
+ (centos-version "8.3.2011"))
+ (package
+ (name "vsftpd")
+ (version version)
+ (source (origin
+ (method url-fetch)
+ (uri (string-append
+ "https://vault.centos.org/centos/" centos-version
+ "/AppStream/Source/SPackages/vsftpd-" version "-"
+ revision ".el8.src.rpm"))
+ (sha256
+ (base32
+
"1xl0kqcismf82hl99klqbvvpylpyk1yr1qjy5hd8f80cj4lyl0f4"))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:make-flags '("LDFLAGS=-lcrypt -lssl -pie")
+ #:tests? #f ; No tests exist.
+ #:phases
+ (modify-phases %standard-phases
+ (add-after 'unpack 'patch-installation-directory
+ (lambda* (#:key outputs #:allow-other-keys)
+ (substitute* "Makefile"
+ (("/usr") (assoc-ref outputs "out")))
+ #t))
+ (replace 'unpack
+ (lambda* (#:key source #:allow-other-keys)
+ (let ((version "3.0.3")
+ (revision "32")
+ (centos-version "8.3.2011"))
+
+ (invoke "7z" "e" source (string-append "-o"
"./vsftpd-"
+ version "-"
+ revision
".el8.src.cpio"))
+ (chdir (string-append "./vsftpd-" version "-"
+ revision ".el8.src.cpio"))
+ (invoke "cpio" "-idmv" (string-append
"--file=./vsftpd-"
+ version "-"
+ revision
".el8.src.cpio"))
+ (invoke "tar" "xvf" (string-append "./vsftpd-"
version ".tar.gz"))
+ (let ((patches
+
'("0001-Don-t-use-the-provided-script-to-locate-libraries.patch"
+ "0002-Enable-build-with-SSL.patch"
+ "0003-Enable-build-with-TCP-Wrapper.patch"
+
"0004-Use-etc-vsftpd-dir-for-config-files-instead-of-etc.patch"
+
"0005-Use-hostname-when-calling-PAM-authentication-module.patch"
+
"0006-Close-stdin-out-err-before-listening-for-incoming-co.patch"
+ "0007-Make-filename-filters-smarter.patch"
+ "0008-Write-denied-logins-into-the-log.patch"
+
"0009-Trim-whitespaces-when-reading-configuration.patch"
+ "0010-Improve-daemonizing.patch"
+ "0011-Fix-listing-with-more-than-one-star.patch"
+
"0012-Replace-syscall-__NR_clone-.-with-clone.patch"
+ "0013-Extend-man-pages-with-systemd-info.patch"
+
"0014-Add-support-for-square-brackets-in-ls.patch"
+ "0015-Listen-on-IPv6-by-default.patch"
+
"0016-Increase-VSFTP_AS_LIMIT-from-200UL-to-400UL.patch"
+
"0017-Fix-an-issue-with-timestamps-during-DST.patch"
+
"0018-Change-the-default-log-file-in-configuration.patch"
+
"0019-Introduce-reverse_lookup_enable-option.patch"
+
"0020-Use-unsigned-int-for-uid-and-gid-representation.patch"
+
"0021-Introduce-support-for-DHE-based-cipher-suites.patch"
+
"0022-Introduce-support-for-EDDHE-based-cipher-suites.patch"
+
"0023-Add-documentation-for-isolate_-options.-Correct-defa.patch"
+ "0024-Introduce-new-return-value-450.patch"
+ "0025-Improve-local_max_rate-option.patch"
+ "0026-Prevent-hanging-in-SIGCHLD-handler.patch"
+ "0027-Delete-files-when-upload-fails.patch"
+ "0028-Fix-man-page-rendering.patch"
+ "0029-Fix-segfault-in-config-file-parser.patch"
+
"0030-Fix-logging-into-syslog-when-enabled-in-config.patch"
+
"0031-Fix-question-mark-wildcard-withing-a-file-name.patch"
+
"0032-Propagate-errors-from-nfs-with-quota-to-client.patch"
+
"0033-Introduce-TLSv1.1-and-TLSv1.2-options.patch"
+
"0034-Turn-off-seccomp-sandbox-because-it-is-too-strict.patch"
+
"0035-Modify-DH-enablement-patch-to-build-with-OpenSSL-1.1.patch"
+ "0036-Redefine-VSFTP_COMMAND_FD-to-1.patch"
+
"0037-Document-the-relationship-of-text_userdb_names-and-c.patch"
+
"0038-Document-allow_writeable_chroot-in-the-man-page.patch"
+
"0039-Improve-documentation-of-ASCII-mode-in-the-man-page.patch"
+ "0040-Use-system-wide-crypto-policy.patch"
+
"0041-Document-the-new-default-for-ssl_ciphers-in-the-man-.patch"
+
"0042-When-handling-FEAT-command-check-ssl_tlsv1_1-and-ssl.patch"
+ "0043-Enable-only-TLSv1.2-by-default.patch"
+
"0044-Disable-anonymous_enable-in-default-config-file.patch"
+
"0045-Expand-explanation-of-ascii_-options-behaviour-in-ma.patch"
+
"0046-vsftpd.conf-Refer-to-the-man-page-regarding-the-asci.patch"
+ "0047
This message was truncated. Download the full message here.
T
T
Tobias Geerinckx-Rice wrote on 30 Mar 2021 17:32
(name . david larsson)(address . david.larsson@selfhosted.xyz)
87y2e4hd2z.fsf@nckx
David,

david larsson writes:
Toggle quote (3 lines)
> Hi,
> the attached patch updates vsftpd so it can use tlsv1.2 etc.

Wow. Thanks!

As indicated on IRC I've made some changes to the patch, mainly to
avoid hard-coding all patches. The result is attached. Let me
know what you think.

Further random comments below:

Toggle quote (6 lines)
> From: methuselah-0 <david.larsson@selfhosted.xyz>
> Date: Tue, 30 Mar 2021 11:18:09 +0200
> Subject: [PATCH] gnu: vsftpd: Use CentOS version and patches.
>
> * gnu/packages/ftp.scm (vftpd): Use CentOS version and
> patches.
^^^^

This is what happens when you copy commit messages from git and
paste them right back in :-) In that case, remove the four
leading spaces.

Toggle quote (2 lines)
> + (let ((version "3.0.3")

I renamed this to UPSTREAM-VERSION, so we can show a more specific
VERSION field in the Guix UI. What we offer isn't ‘3.0.3’ any
more.

Toggle quote (2 lines)
> + (revision "32")

I subjectively added ‘.el8’ here, mainly to factor it out below.
Neither of us knows what it means, though...

Toggle quote (6 lines)
> + (add-after 'unpack 'patch-installation-directory
> + (lambda* (#:key outputs #:allow-other-keys)
> + (substitute* "Makefile"
> + (("/usr") (assoc-ref outputs "out")))
> + #t))

Moved below the redefined 'unpack phase for clarity.

Toggle quote (6 lines)
> + (replace 'unpack
> + (lambda* (#:key source #:allow-other-keys)
> + (let ((version "3.0.3")
> + (revision "32")
> + (centos-version "8.3.2011"))

OK, so, as mentioned on IRC this can be avoided by quasiquoting
<arguments> (as it already was, here) and using ,version instead.

Quoting is probably the most confusing-yet-basic concept in
Scheme.

Toggle quote (23 lines)
> +
> + (invoke "7z" "e" source (string-append "-o"
> "./vsftpd-"
> +
> version "-"
> +
> revision
> ".el8.src.cpio"))
> + (chdir (string-append "./vsftpd-" version
> "-"
> + revision
> ".el8.src.cpio"))
> + (invoke "cpio" "-idmv" (string-append
> "--file=./vsftpd-"
> +
> version "-"
> +
> revision
> ".el8.src.cpio"))
> + (invoke "tar" "xvf" (string-append
> "./vsftpd-"
> version ".tar.gz"))

This dance had a few steps too many IMO, so I simplified it. It's
OK to keep the unpacked steps around during the (short) build
process; they are tiny by today's standards.

Toggle quote (2 lines)
> + (let ((patches

I understand the reason for this: the patches need to be applied
in this order, or patching will appear to succeed but result in
unbuildable source. A simple FIND-FILES is right out.

However, since the order is specified in vsftpd.spec, it's safer,
shorter, and simply more fun to parse it ourselves.

Toggle quote (29 lines)
> + (chdir (string-append "./vsftpd-"
> version))
> + (invoke "git" "init" ".")
> + (invoke "git" "config" "user.email"
> "you@example.com")
> + (invoke "git" "config" "user.name" "Your
> Name" )
> + (invoke "git" "add" ".")
> + (invoke "git" "commit" "-m" "first")
> + (map (lambda (x) (invoke "git" "am"
> (string-append
> "./" x))) patches)
> + (map (lambda (x) (invoke "rm"
> (string-append "./"
> x))) patches)
> + (invoke "rm" "-rf" "./.git")
> + (chdir "../")
> + (invoke "mv" (string-append "./vsftpd-"
> version)
> "../")
> + (chdir "../")
> + (invoke "rm" "-rf" (string-append
> "./vsftpd-"
> version "-"
> + revision
> ".el8.src.cpio"))
> + (chdir (string-append "./vsftpd-"
> version)))

You lost me here. Why all the git? I removed all mention of git
from the package, since it didn't seem necessary, but please
correct me if needful.

Toggle quote (2 lines)
> + #t)))

Whilst Guix on master still complains about ‘missing’ #Ts, they
are a moribund relic and I've secretly started forgetting the odd
#t on master already...

Toggle quote (7 lines)
> + (native-inputs `(("openssl" ,openssl)
> + ("linux-pam" ,linux-pam)
> + ("p7zip" ,p7zip)
> + ("cpio" ,cpio)
> + ("git" ,git-minimal)
> + ("libcap" ,libcap)))

These are *all* new, correct? I removed git and added them all to
the commit message (check it out).

Thanks again for your work!

T G-R
T
T
Tobias Geerinckx-Rice wrote on 30 Mar 2021 17:34
(name . david larsson)(address . david.larsson@selfhosted.xyz)
87v998hczb.fsf@nckx
Tobias Geerinckx-Rice forgot to write:
Toggle quote (2 lines)
> I've also added a copyright line for you.

Kind regards,

T G-R
D
D
david larsson wrote on 30 Mar 2021 20:38
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)
ce5f489144a0f59ce59f84f2f54a4aa8@selfhosted.xyz
On 2021-03-30 17:32, Tobias Geerinckx-Rice wrote:

Toggle quote (4 lines)
> As indicated on IRC I've made some changes to the patch, mainly to
> avoid hard-coding all patches. The result is attached. Let me know
> what you think.

It looks great! Especially nice to see that you separated the patch and
unpack phases - it looks much better now.

Toggle quote (8 lines)
>>
>> * gnu/packages/ftp.scm (vftpd): Use CentOS version and
>> patches.
> ^^^^
>
> This is what happens when you copy commit messages from git and paste
> them right back in :-) In that case, remove the four leading spaces.

Yep, thats what I did :-) will fix next time!

Reg. why to use the significantly patched CentOS variant (asked in your
updated patch's comments): the email passwords thing was a mistake to
mention by me in IRC - that feature was probably already there -
however, the tlsv1.2 was the main reason for switching to the CentOS
version - other features added by the whole patch-set I don't know much
about except from glancing over them and it looks mostly like bug and
security fixes to me.

Toggle quote (6 lines)
>
>> + (let ((version "3.0.3")
>
> I renamed this to UPSTREAM-VERSION, so we can show a more specific
> VERSION field in the Guix UI. What we offer isn't ‘3.0.3’ any more.

Ok, I think I understand.

Toggle quote (5 lines)
>> + (revision "32")
>
> I subjectively added ‘.el8’ here, mainly to factor it out below.
> Neither of us knows what it means, though...

That is fine with me.

Toggle quote (9 lines)
>
>> + (add-after 'unpack 'patch-installation-directory
>> + (lambda* (#:key outputs #:allow-other-keys)
>> + (substitute* "Makefile"
>> + (("/usr") (assoc-ref outputs "out")))
>> + #t))
>
> Moved below the redefined 'unpack phase for clarity.

Great! I had in mind to do the same myself, but didn't due to a
combination of a lack of Guile/Guix coding skills and time.

Toggle quote (11 lines)
>> + (replace 'unpack
>> + (lambda* (#:key source #:allow-other-keys)
>> + (let ((version "3.0.3")
>> + (revision "32")
>> + (centos-version "8.3.2011"))
>
> OK, so, as mentioned on IRC this can be avoided by quasiquoting
> <arguments> (as it already was, here) and using ,version instead.
>
> Quoting is probably the most confusing-yet-basic concept in Scheme.

Looks good to me! I am actually quite familiar with unquoting, including
g-exp unquoting things, and I somehow missed that I was in a quasiquote
context from after "arguments"... I intend to improve!

Toggle quote (19 lines)
>
>> +
>> + (invoke "7z" "e" source (string-append "-o"
>> "./vsftpd-"
>> + version "-"
>> + revision ".el8.src.cpio"))
>> + (chdir (string-append "./vsftpd-" version "-"
>> + revision ".el8.src.cpio"))
>> + (invoke "cpio" "-idmv" (string-append
>> "--file=./vsftpd-"
>> + version "-"
>> + revision ".el8.src.cpio"))
>> + (invoke "tar" "xvf" (string-append "./vsftpd-"
>> version ".tar.gz"))
>
> This dance had a few steps too many IMO, so I simplified it. It's OK
> to keep the unpacked steps around during the (short) build process;
> they are tiny by today's standards.

Agreed. I was not very happy with this myself. Thanks for fixing!

Toggle quote (36 lines)
>
>> + (let ((patches
>
> I understand the reason for this: the patches need to be applied in
> this order, or patching will appear to succeed but result in
> unbuildable source. A simple FIND-FILES is right out.
>
> However, since the order is specified in vsftpd.spec, it's safer,
> shorter, and simply more fun to parse it ourselves.
>
>> + (chdir (string-append "./vsftpd-" version))
>> + (invoke "git" "init" ".")
>> + (invoke "git" "config" "user.email"
>> "you@example.com")
>> + (invoke "git" "config" "user.name" "Your Name" )
>> + (invoke "git" "add" ".")
>> + (invoke "git" "commit" "-m" "first")
>> + (map (lambda (x) (invoke "git" "am"
>> (string-append "./" x))) patches)
>> + (map (lambda (x) (invoke "rm" (string-append
>> "./" x))) patches)
>> + (invoke "rm" "-rf" "./.git")
>> + (chdir "../")
>> + (invoke "mv" (string-append "./vsftpd-" version)
>> "../")
>> + (chdir "../")
>> + (invoke "rm" "-rf" (string-append "./vsftpd-"
>> version "-"
>> + revision
>> ".el8.src.cpio"))
>> + (chdir (string-append "./vsftpd-" version)))
>
> You lost me here. Why all the git? I removed all mention of git from
> the package, since it didn't seem necessary, but please correct me if
> needful.

I am, or was, simply unfamiliar with the simplicity of just using
"patch". I tried git am which failed and reported errors that was solved
by the additional git commands. Your replacement is exactly what I need
to learn more about, and looks great, thanks!

Toggle quote (11 lines)
>
>> + (native-inputs `(("openssl" ,openssl)
>> + ("linux-pam" ,linux-pam)
>> + ("p7zip" ,p7zip)
>> + ("cpio" ,cpio)
>> + ("git" ,git-minimal)
>> + ("libcap" ,libcap)))
>
> These are *all* new, correct? I removed git and added them all to the
> commit message (check it out).

Yep!

Toggle quote (5 lines)
>
> Thanks again for your work!
>
> T G-R

Well..., thank you for your work! You made this patch a lot better! :-)

Best regards,
David Larsson
T
T
Tobias Geerinckx-Rice wrote on 30 Mar 2021 21:41
(name . david larsson)(address . david.larsson@selfhosted.xyz)(address . 47495-done@debbugs.gnu.org)
87sg4ch1iz.fsf@nckx
David,

Toggle quote (2 lines)
> + (native-inputs `(("openssl" ,openssl)

Not sure how I missed this -- actually I do, considering the three
empty champagne bottles now adorning our wall -- but the first
three should be regular inputs, not native, as they are legitimate
references of the resulting package ($ guix gc --references).

Native inputs run only during the build. The distinction matters
during cross-compilation, when the build-time native-inputs may be
a different (say, x86_64) architecture from the output package and
its inputs (both identical: say, aarch64).

Toggle quote (4 lines)
> It looks great! Especially nice to see that you separated the
> patch
> and unpack phases - it looks much better now.

Thank you :-) Pushed as 634d9845a6b4e362f32ba369ae42851719455ba3.

Kind regards,

T G-R
Closed
?