[RFC,1/1] package/pkg-download: don't export the download command variables
diff mbox series

Message ID 20250212191015.3413693-1-dario.binacchi@amarulasolutions.com
State New
Headers show
Series
  • [RFC,1/1] package/pkg-download: don't export the download command variables
Related show

Commit Message

Dario Binacchi Feb. 12, 2025, 7:10 p.m. UTC 
Exporting the download variables can cause unfortunate name clashes, as
occurred with the SCP variable used by Binman for compiling U-Boot [1].
The patch removes the global export of the variables containing the
commands used for downloading the packages and passes them as
parameters, using the new -C option, to the dl-wrapper, which in turn
passes the appropriate download command to the specific script under the
support/download directory.

[1] https://lore.kernel.org/buildroot/a023971c7c8bfa4826a9a8721500c7ff@free.fr/T/
Cc: Julien Olivain <ju.o@free.fr>
Suggested-by: Peter Korsgaard <peter@korsgaard.com>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---
 package/pkg-download.mk     | 27 ++++++++++++++++++---------
 support/download/bzr        |  5 ++---
 support/download/curl       |  5 ++---
 support/download/cvs        |  5 ++---
 support/download/dl-wrapper | 27 +++++++++++++++++++++++----
 support/download/git        |  5 ++---
 support/download/hg         |  5 ++---
 support/download/scp        |  5 ++---
 support/download/sftp       |  5 ++---
 support/download/svn        |  5 ++---
 support/download/wget       |  5 ++---
 11 files changed, 59 insertions(+), 40 deletions(-)

Comments

Peter Korsgaard Feb. 12, 2025, 7:51 p.m. UTC | #1 
>>>>> "Dario" == Dario Binacchi <dario.binacchi@amarulasolutions.com> writes:

Hello Dario,

 > Exporting the download variables can cause unfortunate name clashes, as
 > occurred with the SCP variable used by Binman for compiling U-Boot [1].
 > The patch removes the global export of the variables containing the
 > commands used for downloading the packages and passes them as
 > parameters, using the new -C option, to the dl-wrapper, which in turn
 > passes the appropriate download command to the specific script under the
 > support/download directory.

Thanks for the patch, but is this -C option logic even needed? Couldn't
we just pass all of these in the environment to dl-wrapper?

 > [1] https://lore.kernel.org/buildroot/a023971c7c8bfa4826a9a8721500c7ff@free.fr/T/
 > Cc: Julien Olivain <ju.o@free.fr>
 > Suggested-by: Peter Korsgaard <peter@korsgaard.com>
 > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

Patch
diff mbox series 

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index ca01ff67a59e..580cc01cf882 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -8,15 +8,15 @@ 
 ################################################################################
 
 # 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))
-export BZR := $(call qstrip,$(BR2_BZR))
-export GIT := $(call qstrip,$(BR2_GIT))
-export HG := $(call qstrip,$(BR2_HG))
-export SCP := $(call qstrip,$(BR2_SCP))
-export SFTP := $(call qstrip,$(BR2_SFTP))
+CURL := $(call qstrip,$(BR2_CURL))
+WGET := $(call qstrip,$(BR2_WGET))
+SVN := $(call qstrip,$(BR2_SVN))
+CVS := $(call qstrip,$(BR2_CVS))
+BZR := $(call qstrip,$(BR2_BZR))
+GIT := $(call qstrip,$(BR2_GIT))
+HG := $(call qstrip,$(BR2_HG))
+SCP := $(call qstrip,$(BR2_SCP))
+SFTP := $(call qstrip,$(BR2_SFTP))
 export LOCALFILES := $(call qstrip,$(BR2_LOCALFILES))
 
 # Version of the format of the archives we generate in the corresponding
@@ -132,6 +132,15 @@  define DOWNLOAD
 		$(if $($(PKG)_GIT_SUBMODULES),-r) \
 		$(if $($(PKG)_GIT_LFS),-l) \
 		$(foreach uri,$(call DOWNLOAD_URIS,$(1),$(PKG)),-u $(uri)) \
+		-C '$(CURL)' \
+		-C '$(WGET)' \
+		-C '$(SVN)' \
+		-C '$(CVS)' \
+		-C '$(BZR)' \
+		-C '$(GIT)' \
+		-C '$(HG)' \
+		-C '$(SCP)' \
+		-C '$(SFTP)' \
 		$(2) \
 		$(QUIET) \
 		-- \
diff --git a/support/download/bzr b/support/download/bzr
index 6f77bc286efb..2472e30ff3ab 100755
--- a/support/download/bzr
+++ b/support/download/bzr
@@ -11,9 +11,7 @@  set -e
 #   -u URI      Clone from repository at URI.
 #   -c CSET     Use changeset (or revision) CSET.
 #   -n NAME     Use basename NAME.
-#
-# Environment:
-#   BZR      : the bzr command to call
+#   -C          The bzr command to call.
 
 
 quiet=
@@ -24,6 +22,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     u)  uri="${OPTARG}";;
     c)  cset="${OPTARG}";;
     n)  basename="${OPTARG}";;
+    C)  BZR="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
     esac
diff --git a/support/download/curl b/support/download/curl
index 2124fe7cad95..65e2f976f790 100755
--- a/support/download/curl
+++ b/support/download/curl
@@ -10,9 +10,7 @@  set -e
 #   -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
+#   -C          The curl command to call.
 
 quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
@@ -21,6 +19,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     o)  output="${OPTARG}";;
     f)  filename="${OPTARG}";;
     u)  url="${OPTARG}";;
+    C)  CURL="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
     esac
diff --git a/support/download/cvs b/support/download/cvs
index a52e543e44c2..4e6ab8138997 100755
--- a/support/download/cvs
+++ b/support/download/cvs
@@ -12,9 +12,7 @@  set -e
 #   -c REV      Use revision REV.
 #   -N RAWNAME  Use rawname (aka module name) RAWNAME.
 #   -n NAME     Use basename NAME.
-#
-# Environment:
-#   CVS      : the cvs command to call
+#   -C          The cvs command to call.
 
 quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
@@ -25,6 +23,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     c)  rev="${OPTARG}";;
     N)  rawname="${OPTARG}";;
     n)  basename="${OPTARG}";;
+    C)  CVS="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
     esac
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 5445aad5a76a..f63d14ebf65a 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -20,17 +20,34 @@  set -e
 # shellcheck source=helpers source-path=SCRIPTDIR
 . "${0%/*}/helpers"
 
-export BR_BACKEND_DL_GETOPTS=":hc:d:o:n:N:H:lru:qf:e"
+export BR_BACKEND_DL_GETOPTS=":hc:d:o:n:C:N:H:lru:qf:e"
+
+dl_cmd() {
+    local match_cmd="$1"
+    shift
+    local cmds=("$@")
+
+    for cmd in "${cmds[@]}"; do
+        _cmd=$(echo "$cmd" | awk '{print $1}')
+        if [[ "$_cmd" == "$match_cmd" ]]; then
+            echo "$cmd"
+            return 0
+        fi
+    done
+
+    return 1
+}
 
 main() {
     local OPT OPTARG
-    local backend output large_file recurse quiet rc
-    local -a uris hfiles backend_opts post_process_opts
+    local backend backend_dl_cmd output large_file recurse quiet rc
+    local -a uris hfiles backend_opts post_process_opts dl_cmds
 
     # Parse our options; anything after '--' is for the backend
-    while getopts ":c:d:D:o:n:N:H:lrf:u:qp:P:" OPT; do
+    while getopts ":c:d:D:o:n:C:N:H:lrf:u:qp:P:" OPT; do
         case "${OPT}" in
         c)  cset="${OPTARG}";;
+        C)  dl_cmds+=( "${OPTARG}" );;
         d)  dl_dir="${OPTARG}";;
         D)  old_dl_dir="${OPTARG}";;
         o)  output="${OPTARG}";;
@@ -101,6 +118,7 @@  main() {
         esac
         uri=${uri#*+}
 
+        backend_dl_cmd=$(dl_cmd "${backend}" "${dl_cmds[@]}")
         urlencode=${backend_urlencode#*|}
         # urlencode must be "urlencode"
         [ "${urlencode}" != "urlencode" ] && urlencode=""
@@ -134,6 +152,7 @@  main() {
         # cleanup and exit.
         if ! "${OLDPWD}/support/download/${backend}" \
                 -c "${cset}" \
+                -C "${backend_dl_cmd}" \
                 -d "${dl_dir}" \
                 -n "${raw_base_name}" \
                 -N "${base_name}" \
diff --git a/support/download/git b/support/download/git
index 8123bc31531c..59e12cba8855 100755
--- a/support/download/git
+++ b/support/download/git
@@ -17,9 +17,7 @@  set -e
 #   -u URI      Clone from repository at URI.
 #   -c CSET     Use changeset CSET.
 #   -n NAME     Use basename NAME.
-#
-# Environment:
-#   GIT      : the git command to call
+#   -C          The git command to call.
 
 # shellcheck disable=SC1090 # Only provides mk_tar_gz()
 # shellcheck disable=SC1091
@@ -65,6 +63,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     c)  cset="${OPTARG}";;
     d)  dl_dir="${OPTARG}";;
     n)  basename="${OPTARG}";;
+    C)  GIT="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
     esac
diff --git a/support/download/hg b/support/download/hg
index 768a27e06fd0..bc133ac2c2ff 100755
--- a/support/download/hg
+++ b/support/download/hg
@@ -11,9 +11,7 @@  set -e
 #   -u URI      Clone from repository at URI.
 #   -c CSET     Use changeset (or revision) CSET.
 #   -n NAME     Use basename NAME.
-#
-# Environment:
-#   HG       : the hg command to call
+#   -C          The hg command to call.
 
 quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
@@ -23,6 +21,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     u)  uri="${OPTARG}";;
     c)  cset="${OPTARG}";;
     n)  basename="${OPTARG}";;
+    C)  HG="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
     esac
diff --git a/support/download/scp b/support/download/scp
index 14e768b601d0..ac88ea9e6285 100755
--- a/support/download/scp
+++ b/support/download/scp
@@ -10,9 +10,7 @@  set -e
 #   -o FILE     Copy to local file FILE.
 #   -f FILE     Copy from remote file FILE.
 #   -u URI      Download file at URI.
-#
-# Environment:
-#   SCP       : the scp command to call
+#   -C          The scp command to call.
 
 quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
@@ -21,6 +19,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     o)  output="${OPTARG}";;
     f)  filename="${OPTARG}";;
     u)  uri="${OPTARG}";;
+    C)  SCP="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
     esac
diff --git a/support/download/sftp b/support/download/sftp
index 0a44be7e618f..3bddc115fc64 100755
--- a/support/download/sftp
+++ b/support/download/sftp
@@ -10,9 +10,7 @@  set -e
 #   -o FILE     Copy to local file FILE.
 #   -f FILE     Copy from remote file FILE.
 #   -u URI      Download file at URI.
-#
-# Environment:
-#   SFTP      : the sftp command to call
+#   -C          The sftp command to call.
 
 quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
@@ -21,6 +19,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     o)  output="${OPTARG}";;
     f)  filename="${OPTARG}";;
     u)  uri="${OPTARG}";;
+    C)  SFTP="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
     esac
diff --git a/support/download/svn b/support/download/svn
index ff33f1fa39af..829e6bb6f226 100755
--- a/support/download/svn
+++ b/support/download/svn
@@ -17,9 +17,7 @@  set -e
 #   -c REV      Use revision REV.
 #   -n NAME     Use basename NAME.
 #   -r          Recursive, i.e. use externals
-#
-# Environment:
-#   SVN      : the svn command to call
+#   -C          The svn command to call.
 
 # shellcheck disable=SC1090 # Only provides mk_tar_gz()
 # shellcheck disable=SC1091
@@ -35,6 +33,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     c)  rev="${OPTARG}";;
     n)  basename="${OPTARG}";;
     r)  externals=;;
+    C)  SVN="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
     esac
diff --git a/support/download/wget b/support/download/wget
index 68bd0b13c870..be10e123410f 100755
--- a/support/download/wget
+++ b/support/download/wget
@@ -11,9 +11,7 @@  set -e
 #   -f FILENAME The filename of the tarball to get at URL
 #   -u URL      Download file at URL.
 #   -e ENCODE   Tell wget to urlencode the filename passed to it
-#
-# Environment:
-#   WGET     : the wget command to call
+#   -C          The wget command to call.
 
 quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
@@ -23,6 +21,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     f)  filename="${OPTARG}";;
     u)  url="${OPTARG}";;
     e)  encode="-e";;
+    C)  WGET="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
     esac