[Buildroot] [PATCH 0/3 v2] support/download: enhance svn backend

[Buildroot] [PATCH 0/3 v2] support/download: enhance svn backend

[yann.morin]

Hello All!

This small series enhances the svn backend with two new features:
  - properly use credentials to retrieve changeset info
  - allow skipping svn-externals

Additionally, it also adds shellcheck fixes/exceptions, so that script
does not end up on the list of derelict scripts.

Changes v1 -> v2:
  - don't accidentally force submodules for git-downloaded packages


Regards,
Yann E. MORIN.


The following changes since commit 22e476d7886163484d233803b42a2a4c2b588a5b

  arch/Config.in.x86: add Intel and AMD GCC targets (2023-08-01 00:14:50 +0200)


are available as patches in this mail series,

for you to apply patches up to 2281fe5f1a1324621f726080c57b35374ab7bb55

  support/download: add support to exclude svn externals (2023-08-01 15:07:36 +0200)


----------------------------------------------------------------
Yann E. MORIN (3):
      support/download: fix shellcheck errors in svn backend
      support/download: use svn credentials to retrieve revision date
      support/download: add support to exclude svn externals

 .checkpackageignore                     |  1 -
 docs/manual/adding-packages-generic.txt |  6 ++++++
 package/pkg-download.mk                 |  1 +
 package/pkg-generic.mk                  | 10 ++++++++++
 support/download/svn                    | 28 +++++++++++++++++++++++++---
 5 files changed, 42 insertions(+), 4 deletions(-)

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/3 v2] support/download: fix shellcheck errors in svn backend

[yann.morin]

Bizarrely enough, the unquoted expansion of ${quiet} does not trigger
any warning from shellcheck, so we do not add any exception for it.

${SVN} can contain more than one item, but we don't care about splitting
on spaces when we just print it for debug, so we can just quote it
rather than add an exception.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
---
 .checkpackageignore  | 1 -
 support/download/svn | 4 +++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/.checkpackageignore b/.checkpackageignore
index 27c66d0bbe..427791d84b 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -1658,7 +1658,6 @@ support/download/go-post-process Shellcheck
 support/download/hg Shellcheck
 support/download/scp Shellcheck
 support/download/sftp Shellcheck
-support/download/svn Shellcheck
 support/download/wget Shellcheck
 support/gnuconfig/update Shellcheck
 support/libtool/buildroot-libtool-v1.5.patch ApplyOrder Sob Upstream
diff --git a/support/download/svn b/support/download/svn
index b23b7773d3..d9325b7478 100755
--- a/support/download/svn
+++ b/support/download/svn
@@ -20,6 +20,7 @@ set -e
 # Environment:
 #   SVN      : the svn command to call
 
+# shellcheck disable=SC1090 # Only provides mk_tar_gz()
 . "${0%/*}/helpers"
 
 quiet=
@@ -41,12 +42,13 @@ shift $((OPTIND-1)) # Get rid of our options
 # being expanded a second time (in case there are spaces in them)
 _svn() {
     if [ -z "${quiet}" ]; then
-        printf '%s ' ${SVN} "${@}"; printf '\n'
+        printf '%s ' "${SVN}" "${@}"; printf '\n'
     fi
     _plain_svn "$@"
 }
 # Note: please keep command below aligned with what is printed above
 _plain_svn() {
+    # shellcheck disable=SC2086 # We want word-splitting for SVN
     eval ${SVN} "${@}"
 }
 
-- 
2.34.1

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/3 v2] support/download: use svn credentials to retrieve revision date

[yann.morin]

When an svn repository requires credentials, and they are passed
in _DL_OPTS, they must be used also to retrieve the revision date.

One could argue that credentials should not be handled in _DL_OPTS, but
rather that they be fed through other means (e.g. by pre-authenticating
manually once in an interactive session, or by filling them in the usual
~/svn/auth/* mechanisms for a CI).

However, some public facing repositories are using authentication, even
though the credentials are public. This is the case for example for:
    http://software.rtcm-ntrip.org/

In such a case, it does make sense to pass credentials via _DL_OPTS,
because they are not really, even really not, secret.

Another use-case (e.g. for a CI) is to pass the credentials as
environment variables, with _DL_OPTS not hard-coded in the .mk file.

However, _DL_OPTS may contain options that are not valid for 'svn info',
as they are meant to be passed to 'svn export' in the first place. Since
the only options common to 'svn info' and 'svn export' are the
credentials, we just extract those and pass them to 'svn info'.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
---
 support/download/svn | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/support/download/svn b/support/download/svn
index d9325b7478..3e08a0ef36 100755
--- a/support/download/svn
+++ b/support/download/svn
@@ -54,12 +54,28 @@ _plain_svn() {
 
 _svn export --ignore-keywords ${quiet} "${@}" "'${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'.
+# We can also consume the positional parameters, as we'll no longer
+# be calling any other remote-reaching svn command.
+creds=
+while [ ${#} -gt 0 ]; do
+    case "${1}" in
+    --username=*)   creds+=" ${1}"; shift;;
+    --password=*)   creds+=" ${1}"; shift;;
+    --username)     creds+=" ${1} ${2}"; shift 2;;
+    --password)     creds+=" ${1} ${2}"; shift 2;;
+    *)              shift;;
+    esac
+done
+
 # Get the date of the revision, to generate reproducible archives.
 # The output format is YYYY-MM-DDTHH:MM:SS.mmmuuuZ (i.e. always in the
 # UTC timezone), which we can feed as-is to the --mtime option for tar.
 # In case there is a redirection (e.g. http -> https), just keep the
 # last line (svn outputs everything on stdout)
-date="$( _plain_svn info "'${uri}@${rev}'" \
+# shellcheck disable=SC2086 # creds may be empty
+date="$( _plain_svn info ${creds} "'${uri}@${rev}'" \
         |sed -r -e '/^Last Changed Date: /!d; s///'
        )"
 
-- 
2.34.1

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 3/3 v2] support/download: add support to exclude svn externals

[yann.morin]

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).

We must only set it when the package is actually hosted on svn, to avoid
passing -r when the package is not hosted by svn; otherwise, -r would
also be passed e.g. to a git-hosted package, triggering the download of
git submodules even when they are not requested. We need to do so,
because we have a default value, which we usually do not have in other
download options.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>

---
Changes v1 -> v2:
  - don't accidentally force submodules for git-downloaded packages
---
 docs/manual/adding-packages-generic.txt |  6 ++++++
 package/pkg-download.mk                 |  1 +
 package/pkg-generic.mk                  | 10 ++++++++++
 support/download/svn                    |  6 +++++-
 4 files changed, 22 insertions(+), 1 deletion(-)

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..86b9c1f9d3 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -643,6 +643,16 @@ 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
+  # Only set it when the package is actually hosted on svn
+  $(2)_SVN_EXTERNALS = $$(if $$(filter svn,$$($(2)_SITE_METHOD)),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'.
-- 
2.34.1

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 0/3 v2] support/download: enhance svn backend

[Thomas Petazzoni via buildroot]

Hello Yann,

On Tue, 1 Aug 2023 15:11:14 +0200
yann.morin@orange.com wrote:

> Yann E. MORIN (3):
>       support/download: fix shellcheck errors in svn backend
>       support/download: use svn credentials to retrieve revision date
>       support/download: add support to exclude svn externals

I have applied the series to next. As reported on IRC, the From: field
is not correct, as it is just From: yann.morin@orange.com, while it
should be From: Yann E. MORIN <yann.morin@orange.com> to match the
Signed-off-by value. I fixed that up when applying.

Another aspect I'd like to challenge though is your decision in PATCH
3/3 to preserve the current behavior of downloading externals as the
default. We have only one package in the tree that uses SVN as far as I
can see, so it would be easy to verify if this one uses external or
not, and do the right thing.

Yes, I know this change of the default behavior could break external
packages, but we were just discussing about switching the default CMake
backend from Makefile to Ninja once all packages in the tree have been
verified: if we following the same reasoning, we should also forever
preserve Makefile as the default CMake backend to not break external
packages.

I think we need to find the right balance between preserving backward
compatibility and moving forward with "better" solutions. I believe the
chances of having people having external packages *and* packages that
use external SVN repositories are fairly slim, so I would think that
defaulting to *not* downloading the SVN externals would be an
acceptable change. We can always mention it in the migration guidelines
at
https://buildroot.org/downloads/manual/manual.html#migrating-from-ol-versions
if we want to be extra nice.

What do you think?

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 0/3 v2] support/download: enhance svn backend

[yann.morin]

Thomas, All,

On 2023-08-06 16:40 +0200, Thomas Petazzoni spake thusly:
> On Tue, 1 Aug 2023 15:11:14 +0200
> yann.morin@orange.com wrote:
> > Yann E. MORIN (3):
> >       support/download: fix shellcheck errors in svn backend
> >       support/download: use svn credentials to retrieve revision date
> >       support/download: add support to exclude svn externals
> I have applied the series to next. As reported on IRC, the From: field
> is not correct, as it is just From: yann.morin@orange.com, while it
> should be From: Yann E. MORIN <yann.morin@orange.com> to match the
> Signed-off-by value. I fixed that up when applying.

Yeah, that's the mail server here that mangles the outgoing From:
fields... I'll try to coax git into always adding a From pseudo header
as a workaround... Sorry for the mess...

> Another aspect I'd like to challenge though is your decision in PATCH
> 3/3 to preserve the current behavior of downloading externals as the
> default. We have only one package in the tree that uses SVN as far as I
> can see, so it would be easy to verify if this one uses external or
> not, and do the right thing.

I was pondering changing the default, indeed, but that would mean
existing hashes would no longer match. So that would warrant a bump
to BR_FMT_VERSION_svn.

If that's OK, I can work on that.

> Yes, I know this change of the default behavior could break external
> packages,

If we bump BR_FMT_VERSION_svn, they'll more easily notice, and be able
to take action.

> I believe the
> chances of having people having external packages *and* packages that
> use external SVN repositories are fairly slim, so I would think that
> defaulting to *not* downloading the SVN externals would be an
> acceptable change.

I'm not sure about the pervasiveness of svn-externals, but the only
publicly svn-hosted package we needed to introduce in Buildroot does use
externals! ;-)

> We can always mention it in the migration guidelines
> at
> https://buildroot.org/downloads/manual/manual.html#migrating-from-ol-versions
> if we want to be extra nice.

Sure, I'll post a follow-up patch in a moment..

Regards,
Yann E. MORIN.

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot