[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

Debbugs page

david larsson wrote 4 years ago
(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.
Tobias Geerinckx-Rice wrote 4 years ago
(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
Tobias Geerinckx-Rice wrote 4 years ago
(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
david larsson wrote 4 years ago
(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
Tobias Geerinckx-Rice wrote 4 years ago
(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
?
Your comment

This issue is archived.

To comment on this conversation send an email to 47495@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 47495
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help