| Message ID | b1388fcaee6d79d0845a49632b5b9326050ed219.1688559056.git.yann.morin@orange.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] support/download: fix shellcheck errors in svn backend | expand |
All, On 2023-07-05 14:11 +0200, Yann E. MORIN spake thusly: > Like git which can have submodules, subversion can have externals. The > default behaviour for subversion is to retrieve all the externals, > unless told otherwise. > > For some repositories, the externals may be huge (e.g. a dataset or some > assets) and may not be required for building the package. In such a > case, retrieving the externals is both a waste of network bandwitdh and > time, and a waste of disk storage. > > Like for git submodules and git lfs, add an option that packages can set > to specify whether they want externals or not. > > Since we've so far been retrieving externals, we keep that the default, > and packages can opt-out (rather than the opt-in for git submodules or > git lfs). > > Signed-off-by: Yann E. MORIN <yann.morin@orange.com> > --- [--SNIP--] > diff --git a/package/pkg-download.mk b/package/pkg-download.mk > index 0718f21aad..5a311a95c6 100644 > --- a/package/pkg-download.mk > +++ b/package/pkg-download.mk > @@ -119,6 +119,7 @@ define DOWNLOAD > -n '$($(2)_BASENAME_RAW)' \ > -N '$($(2)_RAWNAME)' \ > -o '$($(2)_DL_DIR)/$(notdir $(1))' \ > + $(if $(filter YES,$($(2)_SVN_EXTERNALS)),-r) \ Take note of this line, and continue reading, below... > $(if $($(2)_GIT_SUBMODULES),-r) \ > $(if $($(2)_GIT_LFS),-l) \ > $(foreach uri,$(call DOWNLOAD_URIS,$(1),$(2)),-u $(uri)) \ > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index 5d1c1da128..2be3e1fed9 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -643,6 +643,15 @@ ifndef $(2)_GIT_SUBMODULES > endif > endif > > +ifndef $(2)_SVN_EXTERNALS > + ifdef $(3)_SVN_EXTERNALS > + $(2)_SVN_EXTERNALS = $$($(3)_SVN_EXTERNALS) > + else > + # Legacy: we used to always use externals by default > + $(2)_SVN_EXTERNALS = YES This causes _SVN_EXTERNALS to be set even for non-svn packages, so a git-hosted package will get _SVN_EXTERNALS=YES, and so this is will pass -r in the line will get -r passed, even if not using submodules. So, setting _SVN_EXTERNALS = YES by default should only be done for svn-hosted pacakges. I'll mark the seris as changes-requested and respin this shortly. Regards, Yann E. MORIN. > + endif > +endif > + > # Do not accept to download git submodule if not using the git method > ifneq ($$($(2)_GIT_SUBMODULES),) > ifneq ($$($(2)_SITE_METHOD),git) > diff --git a/support/download/svn b/support/download/svn > index 3e08a0ef36..1decb2310b 100755 > --- a/support/download/svn > +++ b/support/download/svn > @@ -16,6 +16,7 @@ set -e > # -u URI Checkout from repository at URI. > # -c REV Use revision REV. > # -n NAME Use basename NAME. > +# -r Recursive, i.e. use externals > # > # Environment: > # SVN : the svn command to call > @@ -24,6 +25,7 @@ set -e > . "${0%/*}/helpers" > > quiet= > +externals=--ignore-externals > while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do > case "${OPT}" in > q) quiet=-q;; > @@ -31,6 +33,7 @@ while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do > u) uri="${OPTARG}";; > c) rev="${OPTARG}";; > n) basename="${OPTARG}";; > + r) externals=;; > :) printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;; > \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;; > esac > @@ -52,7 +55,8 @@ _plain_svn() { > eval ${SVN} "${@}" > } > > -_svn export --ignore-keywords ${quiet} "${@}" "'${uri}@${rev}'" "'${basename}'" > +# shellcheck disable=SC2086 # externals and quiet may be empty > +_svn export --ignore-keywords ${quiet} ${externals} "${@}" "'${uri}@${rev}'" "'${basename}'" > > # For 'svn info', we only need the credentials, if any; other options > # would be invalid, as they are intended for 'svn export'. > -- > 2.34.1 >
diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt index fbe37f9ca9..299017f9d2 100644 --- a/docs/manual/adding-packages-generic.txt +++ b/docs/manual/adding-packages-generic.txt @@ -347,6 +347,12 @@ not and can not work as people would expect it should: Git LFS to store large files out of band. This is only available for packages downloaded with git (i.e. when +LIBFOO_SITE_METHOD=git+). ++ +LIBFOO_SVN_EXTERNAL+ can be set to +YES+ (the default) to specify + whether to retrieve the svn external references, or to +NO+ to avoid + retrieving those externals. Note that, contrary to other similar + options like +LIBFOO_GIT_SUBMODULES+ or +LIBFOO_GIT_LFS+, the default + here is to actually retrieve the externals; this is a legacy heritage. + * +LIBFOO_STRIP_COMPONENTS+ is the number of leading components (directories) that tar must strip from file names on extraction. The tarball for most packages has one leading component named diff --git a/package/pkg-download.mk b/package/pkg-download.mk index 0718f21aad..5a311a95c6 100644 --- a/package/pkg-download.mk +++ b/package/pkg-download.mk @@ -119,6 +119,7 @@ define DOWNLOAD -n '$($(2)_BASENAME_RAW)' \ -N '$($(2)_RAWNAME)' \ -o '$($(2)_DL_DIR)/$(notdir $(1))' \ + $(if $(filter YES,$($(2)_SVN_EXTERNALS)),-r) \ $(if $($(2)_GIT_SUBMODULES),-r) \ $(if $($(2)_GIT_LFS),-l) \ $(foreach uri,$(call DOWNLOAD_URIS,$(1),$(2)),-u $(uri)) \ diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index 5d1c1da128..2be3e1fed9 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -643,6 +643,15 @@ ifndef $(2)_GIT_SUBMODULES endif endif +ifndef $(2)_SVN_EXTERNALS + ifdef $(3)_SVN_EXTERNALS + $(2)_SVN_EXTERNALS = $$($(3)_SVN_EXTERNALS) + else + # Legacy: we used to always use externals by default + $(2)_SVN_EXTERNALS = YES + endif +endif + # Do not accept to download git submodule if not using the git method ifneq ($$($(2)_GIT_SUBMODULES),) ifneq ($$($(2)_SITE_METHOD),git) diff --git a/support/download/svn b/support/download/svn index 3e08a0ef36..1decb2310b 100755 --- a/support/download/svn +++ b/support/download/svn @@ -16,6 +16,7 @@ set -e # -u URI Checkout from repository at URI. # -c REV Use revision REV. # -n NAME Use basename NAME. +# -r Recursive, i.e. use externals # # Environment: # SVN : the svn command to call @@ -24,6 +25,7 @@ set -e . "${0%/*}/helpers" quiet= +externals=--ignore-externals while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do case "${OPT}" in q) quiet=-q;; @@ -31,6 +33,7 @@ while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do u) uri="${OPTARG}";; c) rev="${OPTARG}";; n) basename="${OPTARG}";; + r) externals=;; :) printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;; \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;; esac @@ -52,7 +55,8 @@ _plain_svn() { eval ${SVN} "${@}" } -_svn export --ignore-keywords ${quiet} "${@}" "'${uri}@${rev}'" "'${basename}'" +# shellcheck disable=SC2086 # externals and quiet may be empty +_svn export --ignore-keywords ${quiet} ${externals} "${@}" "'${uri}@${rev}'" "'${basename}'" # For 'svn info', we only need the credentials, if any; other options # would be invalid, as they are intended for 'svn export'.
Like git which can have submodules, subversion can have externals. The default behaviour for subversion is to retrieve all the externals, unless told otherwise. For some repositories, the externals may be huge (e.g. a dataset or some assets) and may not be required for building the package. In such a case, retrieving the externals is both a waste of network bandwitdh and time, and a waste of disk storage. Like for git submodules and git lfs, add an option that packages can set to specify whether they want externals or not. Since we've so far been retrieving externals, we keep that the default, and packages can opt-out (rather than the opt-in for git submodules or git lfs). Signed-off-by: Yann E. MORIN <yann.morin@orange.com> --- docs/manual/adding-packages-generic.txt | 6 ++++++ package/pkg-download.mk | 1 + package/pkg-generic.mk | 9 +++++++++ support/download/svn | 6 +++++- 4 files changed, 21 insertions(+), 1 deletion(-)