| Message ID | 9acaf4118aa1e3ba7ad9e0516a78273af7d05d3c.1716063903.git.yann.morin.1998@free.fr |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | support/download: fix running on hosts with wget2 (branch yem/dl-curl) | expand |
All, On 2024-05-18 22:25 +0200, Yann E. MORIN spake thusly: > Recent versions of wget, starting with wget 2.0, aka wget2 thereafter, > no longer support FTP (nor FTPS, aka FTP-over-SSL). wget2 is packaged in > Fedora 40, recently released; F40 does not even have the old wget > available in its repository anymore. [--SNIP--] > diff --git a/Config.in b/Config.in > index b5a94325c4..534efa4050 100644 > --- a/Config.in > +++ b/Config.in > @@ -103,6 +103,10 @@ menu "Build options" > > menu "Commands" > > +config BR2_CURL > + string "Curl command" > + default "curl -q --ftp-pasv --retry 3" > + > config BR2_WGET > string "Wget command" > default "wget --passive-ftp -nd -t 3" ^^^^^^^^^^^^^ I forgot to drop that in the series... I'll sit on it for a little while, and respin later with that fixed... Regards, Yann E. MORIN.
Hi Yann > -----Original Message----- > From: buildroot <buildroot-bounces@buildroot.org> On Behalf Of Yann E. > MORIN > Sent: Saturday, May 18, 2024 3:25 PM > To: buildroot@buildroot.org > Cc: Yann E. MORIN <yann.morin.1998@free.fr> > Subject: [External] [Buildroot] [PATCH 1/2] support/download: introduce curl > backend for FTP transfers > > Recent versions of wget, starting with wget 2.0, aka wget2 thereafter, > no longer support FTP (nor FTPS, aka FTP-over-SSL). wget2 is packaged in > Fedora 40, recently released; F40 does not even have the old wget > available in its repository anymore. > > Introduce cURL as a download backend, that we use for FTP and FPTS > protocols. > > Note that the -q flag does not means being quiet; it means that a curlrc > file should not be parsed. The long option is --disable, which meaning > is not much more obivous than the short -q. It also has to be the first > option on the command line. > > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> > --- > Config.in | 4 ++++ > docs/manual/prerequisite.adoc | 1 + > package/pkg-download.mk | 1 + > package/pkg-generic.mk | 2 ++ > support/download/curl | 45 > +++++++++++++++++++++++++++++++++++ > support/download/dl-wrapper | 1 + > 6 files changed, 54 insertions(+) > create mode 100755 support/download/curl > > diff --git a/Config.in b/Config.in > index b5a94325c4..534efa4050 100644 > --- a/Config.in > +++ b/Config.in > @@ -103,6 +103,10 @@ menu "Build options" > > menu "Commands" > > +config BR2_CURL > + string "Curl command" > + default "curl -q --ftp-pasv --retry 3" In my testing I found we need `-L`/`--location` which allows curl to follow redirects. Otherwise, some URLs like github tarballs will silently fail to download. Also I'd suggest `-f`/`--fail` which allows curl to detect some HTTP 4XX errors and print them to stderr. Thanks, Brandon Maier
Brandon, All, On 2024-05-21 16:19 +0000, Maier, Brandon L Collins spake thusly: > > From: buildroot <buildroot-bounces@buildroot.org> On Behalf Of Yann E. > > MORIN [--SNIP--] > > +config BR2_CURL > > + string "Curl command" > > + default "curl -q --ftp-pasv --retry 3" > > In my testing I found we need `-L`/`--location` which allows curl to > follow redirects. Otherwise, some URLs like github tarballs will silently > fail to download. > > Also I'd suggest `-f`/`--fail` which allows curl to detect some HTTP 4XX > errors and print them to stderr. The curl backend is only supposed to handle ftp and ftps URIs, not http or https. See that part in the download wrapper: 92 case "${backend}" in 93 git|svn|cvs|bzr|file|scp|hg|sftp) ;; 94 ftp|ftps) backend="curl" ;; 95 *) backend="wget" ;; 96 esac So, unless I borked something, curl should not be used for http or https. If you have a reproducer, I'm all eyes to test here and fix the breakage. Thanks for the feedback, much appreciated! 👍 Regards, Yann E. MORIN.
Yann, > -----Original Message----- > From: Yann E. MORIN <yann.morin.1998@free.fr> > Sent: Tuesday, May 21, 2024 12:08 PM > To: Maier, Brandon L Collins <Brandon.Maier@collins.com> > Cc: buildroot@buildroot.org > Subject: Re: [External] [Buildroot] [PATCH 1/2] support/download: introduce > curl backend for FTP transfers > > Brandon, All, > > On 2024-05-21 16:19 +0000, Maier, Brandon L Collins spake > thusly: > > > From: buildroot <buildroot-bounces@buildroot.org> On Behalf Of Yann E. > > > MORIN > [--SNIP--] > > > +config BR2_CURL > > > + string "Curl command" > > > + default "curl -q --ftp-pasv --retry 3" > > > > In my testing I found we need `-L`/`--location` which allows curl to > > follow redirects. Otherwise, some URLs like github tarballs will silently > > fail to download. > > > > Also I'd suggest `-f`/`--fail` which allows curl to detect some HTTP 4XX > > errors and print them to stderr. > > The curl backend is only supposed to handle ftp and ftps URIs, not http > or https. See that part in the download wrapper: > > 92 case "${backend}" in > 93 git|svn|cvs|bzr|file|scp|hg|sftp) ;; > 94 ftp|ftps) backend="curl" ;; > 95 *) backend="wget" ;; > 96 esac > > So, unless I borked something, curl should not be used for http or > https. > > If you have a reproducer, I'm all eyes to test here and fix the > breakage. > > Thanks for the feedback, much appreciated! 👍 Oops, you are correct I missed the "FTP" in your subject line. My comments do not apply here. I have a different issue where wget2 isn't working with our corporate proxy and assumed this was related. :) Thanks, Brandon Maier
diff --git a/Config.in b/Config.in index b5a94325c4..534efa4050 100644 --- a/Config.in +++ b/Config.in @@ -103,6 +103,10 @@ menu "Build options" menu "Commands" +config BR2_CURL + string "Curl command" + default "curl -q --ftp-pasv --retry 3" + config BR2_WGET string "Wget command" default "wget --passive-ftp -nd -t 3" diff --git a/docs/manual/prerequisite.adoc b/docs/manual/prerequisite.adoc index 262a5153f5..846a7482ac 100644 --- a/docs/manual/prerequisite.adoc +++ b/docs/manual/prerequisite.adoc @@ -75,6 +75,7 @@ packages using any of these methods, you will need to install the corresponding tool on the host system: + ** +bazaar+ +** +curl+ ** +cvs+ ** +git+ ** +mercurial+ diff --git a/package/pkg-download.mk b/package/pkg-download.mk index 4be45c9d12..455443c164 100644 --- a/package/pkg-download.mk +++ b/package/pkg-download.mk @@ -8,6 +8,7 @@ ################################################################################ # Download method commands +export CURL := $(call qstrip,$(BR2_CURL)) export WGET := $(call qstrip,$(BR2_WGET)) export SVN := $(call qstrip,$(BR2_SVN)) export CVS := $(call qstrip,$(BR2_CVS)) diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index a2749320c3..e1c16b7343 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -1253,6 +1253,8 @@ else ifeq ($$($(2)_SITE_METHOD),hg) DL_TOOLS_DEPENDENCIES += hg else ifeq ($$($(2)_SITE_METHOD),cvs) DL_TOOLS_DEPENDENCIES += cvs +else ifneq ($(filter ftp ftps,$$($(2)_SITE_METHOD)),) +DL_TOOLS_DEPENDENCIES += curl endif # SITE_METHOD # cargo/go vendoring (may) need git diff --git a/support/download/curl b/support/download/curl new file mode 100755 index 0000000000..bea4485a6c --- /dev/null +++ b/support/download/curl @@ -0,0 +1,45 @@ +#!/usr/bin/env bash + +# We want to catch any unexpected failure, and exit immediately +set -e + +# Download helper for curl, to be called from the download wrapper script +# +# Options: +# -q Be quiet. +# -o FILE Save into file FILE. +# -f FILENAME The filename of the tarball to get at URL +# -u URL Download file at URL. +# +# Environment: +# CURL : the curl command to call + +quiet= +while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do + case "${OPT}" in + q) quiet=-s;; + o) output="${OPTARG}";; + f) filename="${OPTARG}";; + u) url="${OPTARG}";; + :) printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;; + \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;; + esac +done + +shift $((OPTIND-1)) # Get rid of our options + +# Caller needs to single-quote its arguments to prevent them from +# being expanded a second time (in case there are spaces in them) +_curl() { + if [ -z "${quiet}" ]; then + printf '%s ' "${CURL}" "${@}"; printf '\n' + fi + _plain_curl "$@" +} +# Note: please keep command below aligned with what is printed above +_plain_curl() { + # shellcheck disable=SC2086 # We want splitting + eval ${CURL} "${@}" +} + +_curl ${quiet} "${@}" --output "'${output}'" "'${url}/${filename}'" diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper index 35428faeef..069b2c1c21 100755 --- a/support/download/dl-wrapper +++ b/support/download/dl-wrapper @@ -91,6 +91,7 @@ main() { backend="${backend_urlencode%|*}" case "${backend}" in git|svn|cvs|bzr|file|scp|hg|sftp) ;; + ftp|ftps) backend="curl" ;; *) backend="wget" ;; esac uri=${uri#*+}
Recent versions of wget, starting with wget 2.0, aka wget2 thereafter, no longer support FTP (nor FTPS, aka FTP-over-SSL). wget2 is packaged in Fedora 40, recently released; F40 does not even have the old wget available in its repository anymore. Introduce cURL as a download backend, that we use for FTP and FPTS protocols. Note that the -q flag does not means being quiet; it means that a curlrc file should not be parsed. The long option is --disable, which meaning is not much more obivous than the short -q. It also has to be the first option on the command line. Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> --- Config.in | 4 ++++ docs/manual/prerequisite.adoc | 1 + package/pkg-download.mk | 1 + package/pkg-generic.mk | 2 ++ support/download/curl | 45 +++++++++++++++++++++++++++++++++++ support/download/dl-wrapper | 1 + 6 files changed, 54 insertions(+) create mode 100755 support/download/curl