[Buildroot] [PATCH 00/11] support/download: fix scp and reintroduce source-check

[Buildroot] [PATCH 00/11] support/download: fix scp and reintroduce source-check

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Hello and happy 2019 to you all!

This series has two parts:
1. fix the broken scp download helper (seems no-one is using it on recent
   releases?) (patches 1 and 2)

2. reintroduce source-check (patches 3-11)

Details in the commit messages...

Best regards,
Thomas


Thomas De Schampheleire (11):
  support/download: fix scp downloads
  support/download: fix scp download with scheme prefix 'scp://'
  support/download: reintroduce 'source-check' target
  support/download: implement source-check in hg backend
  support/download: implement source-check in wget backend
  support/download: implement source-check in file backend
  support/download: implement source-check in scp backend
  support/download: implement source-check in git backend
  support/download: implement source-check in bzr backend
  support/download: implement source-check in cvs backend
  support/download: implement source-check in svn backend

 Config.in                   |  4 ++
 Makefile                    |  7 +++
 package/pkg-download.mk     | 20 ++++++++
 package/pkg-generic.mk      | 14 +++++-
 support/download/bzr        |  9 ++++
 support/download/cvs        | 11 +++++
 support/download/dl-wrapper | 99 ++++++++++++++++++++-----------------
 support/download/file       | 12 ++++-
 support/download/git        | 22 ++++++---
 support/download/hg         |  7 +++
 support/download/scp        | 22 ++++++++-
 support/download/svn        |  7 +++
 support/download/wget       |  7 +++
 13 files changed, 187 insertions(+), 54 deletions(-)

-- 
2.18.1

[Buildroot] [PATCH 01/11] support/download: fix scp downloads

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

scp download is broken, because scp is called without filename argument and
only the server is specified. The call is:
    scp <server> <outputfile>

but should be:
    scp <server>/<filename> <outputfile>

Instead of assuming '-u' lists a full URL including filename (which it is
not), align with the wget helper where -u is the server URL and -f gives the
filename.

With this commit, an scp download can work if FOO_SITE_METHOD is explicitly
set to 'scp' and the server does not have a scheme prefix 'scp://'.
The next commit will handle the case where a scheme prefix is present.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/scp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/support/download/scp b/support/download/scp
index 8ecf2f4b22..746c3c6ba0 100755
--- a/support/download/scp
+++ b/support/download/scp
@@ -8,7 +8,8 @@ set -e
 # Options:
 #   -q          Be quiet.
 #   -o FILE     Copy to local file FILE.
-#   -u FILE     Copy from remote file FILE.
+#   -f FILE     Copy from remote file FILE.
+#   -u URL      Download file at URL.
 #
 # Environment:
 #   SCP       : the scp command to call
@@ -18,6 +19,7 @@ while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
     q)  verbose=-q;;
     o)  output="${OPTARG}";;
+    f)  filename="${OPTARG}";;
     u)  uri="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
@@ -32,4 +34,4 @@ _scp() {
     eval ${SCP} "${@}"
 }
 
-_scp ${verbose} "${@}" "'${uri}'" "'${output}'"
+_scp ${verbose} "${@}" "'${uri}/${filename}'" "'${output}'"
-- 
2.18.1

[Buildroot] [PATCH 01/11] support/download: fix scp downloads

[Yann E. MORIN]

Thomas, All,

On 2019-01-03 21:40 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> scp download is broken, because scp is called without filename argument and
> only the server is specified. The call is:
>     scp <server> <outputfile>
> 
> but should be:
>     scp <server>/<filename> <outputfile>
> 
> Instead of assuming '-u' lists a full URL including filename (which it is
> not), align with the wget helper where -u is the server URL and -f gives the
> filename.
> 
> With this commit, an scp download can work if FOO_SITE_METHOD is explicitly
> set to 'scp' and the server does not have a scheme prefix 'scp://'.
> The next commit will handle the case where a scheme prefix is present.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  support/download/scp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/support/download/scp b/support/download/scp
> index 8ecf2f4b22..746c3c6ba0 100755
> --- a/support/download/scp
> +++ b/support/download/scp
> @@ -8,7 +8,8 @@ set -e
>  # Options:
>  #   -q          Be quiet.
>  #   -o FILE     Copy to local file FILE.
> -#   -u FILE     Copy from remote file FILE.
> +#   -f FILE     Copy from remote file FILE.
> +#   -u URL      Download file at URL.

This is not an URL, but an URI, and even then, this is only the
authority part [0] of the URI. ;-) But, Oh well... I'm bikeshedding
here, so:

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

[0] https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Generic_syntax

Regards,
Yann E. MORIN.

>  #
>  # Environment:
>  #   SCP       : the scp command to call
> @@ -18,6 +19,7 @@ while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
>      case "${OPT}" in
>      q)  verbose=-q;;
>      o)  output="${OPTARG}";;
> +    f)  filename="${OPTARG}";;
>      u)  uri="${OPTARG}";;
>      :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
>      \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
> @@ -32,4 +34,4 @@ _scp() {
>      eval ${SCP} "${@}"
>  }
>  
> -_scp ${verbose} "${@}" "'${uri}'" "'${output}'"
> +_scp ${verbose} "${@}" "'${uri}/${filename}'" "'${output}'"
> -- 
> 2.18.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 01/11] support/download: fix scp downloads

[Thomas De Schampheleire]

On Thu, Jan 3, 2019, 21:55 Yann E. MORIN <yann.morin.1998@free.fr wrote:

> Thomas, All,
>
> On 2019-01-03 21:40 +0100, Thomas De Schampheleire spake thusly:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > scp download is broken, because scp is called without filename argument
> and
> > only the server is specified. The call is:
> >     scp <server> <outputfile>
> >
> > but should be:
> >     scp <server>/<filename> <outputfile>
> >
> > Instead of assuming '-u' lists a full URL including filename (which it is
> > not), align with the wget helper where -u is the server URL and -f gives
> the
> > filename.
> >
> > With this commit, an scp download can work if FOO_SITE_METHOD is
> explicitly
> > set to 'scp' and the server does not have a scheme prefix 'scp://'.
> > The next commit will handle the case where a scheme prefix is present.
> >
> > Signed-off-by: Thomas De Schampheleire <
> thomas.de_schampheleire at nokia.com>
> > ---
> >  support/download/scp | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/support/download/scp b/support/download/scp
> > index 8ecf2f4b22..746c3c6ba0 100755
> > --- a/support/download/scp
> > +++ b/support/download/scp
> > @@ -8,7 +8,8 @@ set -e
> >  # Options:
> >  #   -q          Be quiet.
> >  #   -o FILE     Copy to local file FILE.
> > -#   -u FILE     Copy from remote file FILE.
> > +#   -f FILE     Copy from remote file FILE.
> > +#   -u URL      Download file at URL.
>
> This is not an URL, but an URI, and even then, this is only the
> authority part [0] of the URI. ;-) But, Oh well... I'm bikeshedding
> here, so:
>
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> [0]
> https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Generic_syntax


The differences between URL and URI have never been really clear to me.
So from the link, URI is the generic term and URL is a type of URI?
And because this -u argument does not refer to one resource but a server
hosting multiple resources, it cannot be a URL?

Thanks,
Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190103/2537e779/attachment.html>

[Buildroot] [PATCH 01/11] support/download: fix scp downloads

[Yann E. MORIN]

Thomas, All,

On 2019-01-03 22:03 +0100, Thomas De Schampheleire spake thusly:
> On Thu, Jan 3, 2019, 21:55 Yann E. MORIN < [1]yann.morin.1998@free.fr wrote:
>   This is not an URL, but an URI, and even then, this is only the
>   authority part [0] of the URI. ;-) But, Oh well... I'm bikeshedding
>   here, so:
> The differences between URL and URI have never been really clear to me.
> So from the link, URI is the generic term and URL is a type of URI??

Don't bother: in all practicality, they are useable interchangeably in
the contexts we are most dealing with here. And as I wrote: I was really
just bikeshedding (well, I should have rather said that I were being
pedantic, but this would be starting to be a bit meta now...).

Regards,
Yann E. MORIN.

> And because this -u argument does not refer to one resource but a server hosting multiple resources, it cannot be a URL?
> Thanks,
> Thomas
> 
> Links:
> 1. mailto:yann.morin.1998 at free.fr
> 2. mailto:thomas.de_schampheleire at nokia.com
> 3. mailto:thomas.de_schampheleire at nokia.com
> 4. mailto:yann.morin.1998 at free.fr
> 5. https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Generic_syntax

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 01/11] support/download: fix scp downloads

[Thomas Petazzoni]

Hello,

On Thu,  3 Jan 2019 21:40:16 +0100, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> scp download is broken, because scp is called without filename argument and
> only the server is specified. The call is:
>     scp <server> <outputfile>
> 
> but should be:
>     scp <server>/<filename> <outputfile>
> 
> Instead of assuming '-u' lists a full URL including filename (which it is
> not), align with the wget helper where -u is the server URL and -f gives the
> filename.
> 
> With this commit, an scp download can work if FOO_SITE_METHOD is explicitly
> set to 'scp' and the server does not have a scheme prefix 'scp://'.
> The next commit will handle the case where a scheme prefix is present.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  support/download/scp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Applied to master after s/URL/URI/ in the comment, as suggested by Yann.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 01/11] support/download: fix scp downloads

[Peter Korsgaard]

>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

 > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
 > scp download is broken, because scp is called without filename argument and
 > only the server is specified. The call is:
 >     scp <server> <outputfile>

 > but should be:
 >     scp <server>/<filename> <outputfile>

 > Instead of assuming '-u' lists a full URL including filename (which it is
 > not), align with the wget helper where -u is the server URL and -f gives the
 > filename.

 > With this commit, an scp download can work if FOO_SITE_METHOD is explicitly
 > set to 'scp' and the server does not have a scheme prefix 'scp://'.
 > The next commit will handle the case where a scheme prefix is present.

 > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Committed to 2018.11.x, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 02/11] support/download: fix scp download with scheme prefix 'scp://'

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

The scp download helper is broken when the server URL starts with 'scp://'.
Such prefix is used in two situations:
1. to let FOO_SITE point to an scp location without explicitly having to set
   'FOO_SITE_METHOD = scp'

2. when BR2_PRIMARY_SITE or BR2_BACKUP_SITE points to an scp location. In
   this case, there is no equivalent of 'SITE_METHOD'.

Strip out the scheme prefix, similarly to how the 'file' download helper
does it. That helper has the same cases as above.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/scp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/support/download/scp b/support/download/scp
index 746c3c6ba0..55f588e157 100755
--- a/support/download/scp
+++ b/support/download/scp
@@ -34,4 +34,7 @@ _scp() {
     eval ${SCP} "${@}"
 }
 
+# Remove any scheme prefix
+uri="${uri##scp://}"
+
 _scp ${verbose} "${@}" "'${uri}/${filename}'" "'${output}'"
-- 
2.18.1

[Buildroot] [PATCH 02/11] support/download: fix scp download with scheme prefix 'scp://'

[Yann E. MORIN]

Thomas, All,

On 2019-01-03 21:40 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> The scp download helper is broken when the server URL starts with 'scp://'.
> Such prefix is used in two situations:
> 1. to let FOO_SITE point to an scp location without explicitly having to set
>    'FOO_SITE_METHOD = scp'
> 
> 2. when BR2_PRIMARY_SITE or BR2_BACKUP_SITE points to an scp location. In
>    this case, there is no equivalent of 'SITE_METHOD'.
> 
> Strip out the scheme prefix, similarly to how the 'file' download helper
> does it. That helper has the same cases as above.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  support/download/scp | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/support/download/scp b/support/download/scp
> index 746c3c6ba0..55f588e157 100755
> --- a/support/download/scp
> +++ b/support/download/scp
> @@ -34,4 +34,7 @@ _scp() {
>      eval ${SCP} "${@}"
>  }
>  
> +# Remove any scheme prefix
> +uri="${uri##scp://}"

I was pretty sure that we could pass sdcp:// to the scp tool. That's
funny how one can get very wrong about such simple things...

So it only proves that I don't use scp that often, indee.

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

>  _scp ${verbose} "${@}" "'${uri}/${filename}'" "'${output}'"
> -- 
> 2.18.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 03/11] support/download: reintroduce 'source-check' target

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Commit bf28a165d992681a85b43a886005e669b3cb579b removed the source-check
target to allow easier refactoring of the download infrastructure.
It was partly based on the fact that no-one really used this target.

However, it turns out that there _are_ good uses for it. In a work
environment where many people are submitting changes to a Buildroot
repository, one cannot always blindly trust every change. A typical case
of human error is when bumping the version of a package but forgetting to
upload the tarball to the internal BR2_PRIMARY_SITE or (in case of a
repository) pushing the new changesets to the repository.
If a user cannot directly push to the Buildroot repository but needs to
queue their change via an automatic validation system, that validation
system can use 'make source-check' on the relevant defconfigs to protect
against such errors. A full download would also be possible but takes
longer and unnecessarily uses internal network bandwidth.

With that use case in mind, this commit reintroduces the 'source-check'
target, but embedded in the current situation with a dl-wrapper.  The
changes to the wrapper are minimal (not considering additional indentation).
A new option '-C' means 'check only' and will be passed to the download
backends. If the backend supports the option, no download will happen. If it
does not, then the backend will actually perform a download as a means of
checking that the source exists (a form of graceful degradation). In neither
case, though, hash checking is performed (as the standard case is without
download thus without file to calculate hashes on).

Subsequent commits will actually implement -C in the backends.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 Makefile                    |  7 +++
 package/pkg-download.mk     | 19 +++++++
 package/pkg-generic.mk      | 14 +++++-
 support/download/dl-wrapper | 99 ++++++++++++++++++++-----------------
 4 files changed, 94 insertions(+), 45 deletions(-)

diff --git a/Makefile b/Makefile
index c5b78b3274..f3e22d213f 100644
--- a/Makefile
+++ b/Makefile
@@ -131,6 +131,7 @@ noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconf
 # (default target is to build), or when MAKECMDGOALS contains
 # something else than one of the nobuild_targets.
 nobuild_targets := source %-source \
+	source-check %-source-check %-all-source-check \
 	legal-info %-legal-info external-deps _external-deps \
 	clean distclean help show-targets graph-depends \
 	%-graph-depends %-show-depends %-show-version \
@@ -788,6 +789,9 @@ target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
 .PHONY: source
 source: $(foreach p,$(PACKAGES),$(p)-all-source)
 
+.PHONY: source-check
+source-check: $(foreach p,$(PACKAGES),$(p)-all-source-check)
+
 .PHONY: _external-deps external-deps
 _external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps)
 external-deps:
@@ -1070,6 +1074,8 @@ help:
 	@echo '  <pkg>-dirclean         - Remove <pkg> build directory'
 	@echo '  <pkg>-reconfigure      - Restart the build from the configure step'
 	@echo '  <pkg>-rebuild          - Restart the build from the build step'
+	@echo '  <pkg>-source-check     - Check package for valid download URLs'
+	@echo '  <pkg>-all-source-check - Check package and its dependencies for valid download URLs'
 	$(foreach p,$(HELP_PACKAGES), \
 		@echo $(sep) \
 		@echo '$($(p)_NAME):' $(sep) \
@@ -1089,6 +1095,7 @@ help:
 	@echo
 	@echo 'Miscellaneous:'
 	@echo '  source                 - download all sources needed for offline-build'
+	@echo '  source-check           - check selected packages for valid download URLs'
 	@echo '  external-deps          - list external packages used'
 	@echo '  legal-info             - generate info about license compliance'
 	@echo '  printvars              - dump all the internal variables'
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 7cd87c38ff..cc04e316e2 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -106,3 +106,22 @@ define DOWNLOAD
 		-- \
 		$($(PKG)_DL_OPTS)
 endef
+
+define SOURCE_CHECK
+	$(Q)mkdir -p $($(PKG)_DL_DIR)
+	$(Q)$(EXTRA_ENV) $(FLOCK) $(DL_WRAPPER) \
+		-C \
+		-c '$($(PKG)_DL_VERSION)' \
+		-d '$($(PKG)_DL_DIR)' \
+		-D '$(DL_DIR)' \
+		-f '$(notdir $(1))' \
+		-H '$($(PKG)_HASH_FILE)' \
+		-n '$($(PKG)_BASENAME_RAW)' \
+		-N '$($(PKG)_RAWNAME)' \
+		-o '$($(PKG)_DL_DIR)/$(notdir $(1))' \
+		$(if $($(PKG)_GIT_SUBMODULES),-r) \
+		$(DOWNLOAD_URIS) \
+		$(QUIET) \
+		-- \
+		$($(PKG)_DL_OPTS)
+endef
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f5cab2b9c2..707996e384 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -780,6 +780,10 @@ $(1)-legal-source:	$$($(2)_TARGET_ACTUAL_SOURCE)
 endif # actual sources != sources
 endif # actual sources != ""
 
+$(1)-source-check: PKG=$(2)
+$(1)-source-check:
+	$$(foreach p,$$($(2)_ALL_DOWNLOADS),$$(call SOURCE_CHECK,$$(p))$$(sep))
+
 $(1)-external-deps:
 	@for p in $$($(2)_SOURCE) $$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS) ; do \
 		echo `basename $$$$p` ; \
@@ -804,6 +808,9 @@ $(1)-rsync:		$$($(2)_TARGET_RSYNC)
 $(1)-source:
 $(1)-legal-source:
 
+$(1)-source-check:
+	test -d $$($(2)_OVERRIDE_SRCDIR)
+
 $(1)-external-deps:
 	@echo "file://$$($(2)_OVERRIDE_SRCDIR)"
 endif
@@ -838,6 +845,9 @@ $(1)-graph-rdepends: graph-depends-requirements
 $(1)-all-source:	$(1)-source
 $(1)-all-source:	$$(foreach p,$$($(2)_FINAL_ALL_DEPENDENCIES),$$(p)-all-source)
 
+$(1)-all-source-check:	$(1)-source-check
+$(1)-all-source-check:	$$(foreach p,$$($(2)_FINAL_ALL_DEPENDENCIES),$$(p)-all-source-check)
+
 $(1)-all-external-deps:	$(1)-external-deps
 $(1)-all-external-deps:	$$(foreach p,$$($(2)_FINAL_ALL_DEPENDENCIES),$$(p)-all-external-deps)
 
@@ -1036,6 +1046,7 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
 	$(1)-all-external-deps \
 	$(1)-all-legal-info \
 	$(1)-all-source \
+	$(1)-all-source-check \
 	$(1)-build \
 	$(1)-clean-for-rebuild \
 	$(1)-clean-for-reconfigure \
@@ -1060,7 +1071,8 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
 	$(1)-rsync \
 	$(1)-show-depends \
 	$(1)-show-version \
-	$(1)-source
+	$(1)-source \
+	$(1)-source-check
 
 ifneq ($$($(2)_SOURCE),)
 ifeq ($$($(2)_SITE),)
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 3315bd410e..1ed2e654de 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -17,7 +17,7 @@
 # We want to catch any unexpected failure, and exit immediately.
 set -e
 
-export BR_BACKEND_DL_GETOPTS=":hc:d:o:n:N:H:ru:qf:e"
+export BR_BACKEND_DL_GETOPTS=":hc:Cd:o:n:N:H:ru:qf:e"
 
 main() {
     local OPT OPTARG
@@ -25,9 +25,10 @@ main() {
     local -a uris
 
     # Parse our options; anything after '--' is for the backend
-    while getopts ":c:d:D:o:n:N:H:rf:u:q" OPT; do
+    while getopts ":c:Cd:D:o:n:N:H:rf:u:q" OPT; do
         case "${OPT}" in
         c)  cset="${OPTARG}";;
+        C)  checkonly=-C;;
         d)  dl_dir="${OPTARG}";;
         D)  old_dl_dir="${OPTARG}";;
         o)  output="${OPTARG}";;
@@ -46,38 +47,40 @@ main() {
     # Forget our options, and keep only those for the backend
     shift $((OPTIND-1))
 
-    if [ -z "${output}" ]; then
-        error "no output specified, use -o\n"
-    fi
+    if [ -z "${checkonly}" ]; then
+        if [ -z "${output}" ]; then
+            error "no output specified, use -o\n"
+        fi
 
-    # Legacy handling: check if the file already exists in the global
-    # download directory. If it does, hard-link it. If it turns out it
-    # was an incorrect download, we'd still check it below anyway.
-    # If we can neither link nor copy, fallback to doing a download.
-    # NOTE! This is not atomic, is subject to TOCTTOU, but the whole
-    # dl-wrapper runs under an flock, so we're safe.
-    if [ ! -e "${output}" -a -e "${old_dl_dir}/${filename}" ]; then
-        ln "${old_dl_dir}/${filename}" "${output}" || \
-        cp "${old_dl_dir}/${filename}" "${output}" || \
-        true
-    fi
+        # Legacy handling: check if the file already exists in the global
+        # download directory. If it does, hard-link it. If it turns out it
+        # was an incorrect download, we'd still check it below anyway.
+        # If we can neither link nor copy, fallback to doing a download.
+        # NOTE! This is not atomic, is subject to TOCTTOU, but the whole
+        # dl-wrapper runs under an flock, so we're safe.
+        if [ ! -e "${output}" -a -e "${old_dl_dir}/${filename}" ]; then
+            ln "${old_dl_dir}/${filename}" "${output}" || \
+            cp "${old_dl_dir}/${filename}" "${output}" || \
+            true
+        fi
 
-    # If the output file already exists and:
-    # - there's no .hash file: do not download it again and exit promptly
-    # - matches all its hashes: do not download it again and exit promptly
-    # - fails at least one of its hashes: force a re-download
-    # - there's no hash (but a .hash file): consider it a hard error
-    if [ -e "${output}" ]; then
-        if support/download/check-hash ${quiet} "${hfile}" "${output}" "${output##*/}"; then
-            exit 0
-        elif [ ${?} -ne 2 ]; then
-            # Do not remove the file, otherwise it might get re-downloaded
-            # from a later location (i.e. primary -> upstream -> mirror).
-            # Do not print a message, check-hash already did.
-            exit 1
+        # If the output file already exists and:
+        # - there's no .hash file: do not download it again and exit promptly
+        # - matches all its hashes: do not download it again and exit promptly
+        # - fails at least one of its hashes: force a re-download
+        # - there's no hash (but a .hash file): consider it a hard error
+        if [ -e "${output}" ]; then
+            if support/download/check-hash ${quiet} "${hfile}" "${output}" "${output##*/}"; then
+                exit 0
+            elif [ ${?} -ne 2 ]; then
+                # Do not remove the file, otherwise it might get re-downloaded
+                # from a later location (i.e. primary -> upstream -> mirror).
+                # Do not print a message, check-hash already did.
+                exit 1
+            fi
+            rm -f "${output}"
+            warn "Re-downloading '%s'...\n" "${output##*/}"
         fi
-        rm -f "${output}"
-        warn "Re-downloading '%s'...\n" "${output##*/}"
     fi
 
     # Look through all the uris that we were given to download the package
@@ -127,7 +130,7 @@ main() {
                 -f "${filename}" \
                 -u "${uri}" \
                 -o "${tmpf}" \
-                ${quiet} ${recurse} -- "${@}"
+                ${quiet} ${recurse} ${checkonly} -- "${@}"
         then
             # cd back to keep path coherence
             cd "${OLDPWD}"
@@ -138,19 +141,21 @@ main() {
         # cd back to free the temp-dir, so we can remove it later
         cd "${OLDPWD}"
 
-        # Check if the downloaded file is sane, and matches the stored hashes
-        # for that file
-        if support/download/check-hash ${quiet} "${hfile}" "${tmpf}" "${output##*/}"; then
-            rc=0
-        else
-            if [ ${?} -ne 3 ]; then
-                rm -rf "${tmpd}"
-                continue
+        if [ -z "${checkonly}" ]; then
+            # Check if the downloaded file is sane, and matches the stored hashes
+            # for that file
+            if support/download/check-hash ${quiet} "${hfile}" "${tmpf}" "${output##*/}"; then
+                rc=0
+            else
+                if [ ${?} -ne 3 ]; then
+                    rm -rf "${tmpd}"
+                    continue
+                fi
+
+                # the hash file exists and there was no hash to check the file
+                # against
+                rc=1
             fi
-
-            # the hash file exists and there was no hash to check the file
-            # against
-            rc=1
         fi
         download_and_check=1
         break
@@ -163,6 +168,12 @@ main() {
         exit 1
     fi
 
+    # If we only need to check the presence of sources, stop here.
+    # No need to handle output files.
+    if [ -n "${checkonly}" ]; then
+        exit 0
+    fi
+
     # tmp_output is in the same directory as the final output, so we can
     # later move it atomically.
     tmp_output="$(mktemp "${output}.XXXXXX")"
-- 
2.18.1

[Buildroot] [PATCH 03/11] support/download: reintroduce 'source-check' target

[Yann E. MORIN]

Thomas, All,

On 2019-01-03 21:40 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> Commit bf28a165d992681a85b43a886005e669b3cb579b removed the source-check
> target to allow easier refactoring of the download infrastructure.
> It was partly based on the fact that no-one really used this target.
> 
> However, it turns out that there _are_ good uses for it. In a work
> environment where many people are submitting changes to a Buildroot
> repository, one cannot always blindly trust every change. A typical case
> of human error is when bumping the version of a package but forgetting to
> upload the tarball to the internal BR2_PRIMARY_SITE or (in case of a
> repository) pushing the new changesets to the repository.
> If a user cannot directly push to the Buildroot repository but needs to
> queue their change via an automatic validation system, that validation
> system can use 'make source-check' on the relevant defconfigs to protect
> against such errors. A full download would also be possible but takes
> longer and unnecessarily uses internal network bandwidth.

But still, at some point, you will want your CI to actually test the
change, so you will need to have the stuff downloaded... So, why can't
you simply use 'make source && make' ? It would (mostly) have the actual
result you are looking for: do the check that everything is available,
and if it is, then the build can proceed. If something was missing, that
would have bailed out early.

The only thing that differs with source-check, is that the network will
actually be used (boohoo!).

However, since we added local cache for git, you would not need to fetch
much. Also, tarballs (from wget et al) were already cached locally. Of
course, that means you'd have to have BR2_DL_DIR in your envioronment,
pointing to a dl location that is persistent...

What is missing (guess it) is a local cache for Hg. I started working on
it a while ago (rght after the git cache was merged), but dropped it as
I had no way to throughly test it.

So, if you are really concerned about not exhausting your internal
network that much (I know some companies have slow links between remote
sites, so I understand [0]), what about you provide an Hg caching like
we have for git instead? ;-)

So, I am definitely not convinced by the need for source-check...

[0] But then, does it make sense that the CI and the source code are
hosted at different sites? That'd be weird...

Regards,
Yann E. MORIN.

> With that use case in mind, this commit reintroduces the 'source-check'
> target, but embedded in the current situation with a dl-wrapper.  The
> changes to the wrapper are minimal (not considering additional indentation).
> A new option '-C' means 'check only' and will be passed to the download
> backends. If the backend supports the option, no download will happen. If it
> does not, then the backend will actually perform a download as a means of
> checking that the source exists (a form of graceful degradation). In neither
> case, though, hash checking is performed (as the standard case is without
> download thus without file to calculate hashes on).
> 
> Subsequent commits will actually implement -C in the backends.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  Makefile                    |  7 +++
>  package/pkg-download.mk     | 19 +++++++
>  package/pkg-generic.mk      | 14 +++++-
>  support/download/dl-wrapper | 99 ++++++++++++++++++++-----------------
>  4 files changed, 94 insertions(+), 45 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c5b78b3274..f3e22d213f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -131,6 +131,7 @@ noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconf
>  # (default target is to build), or when MAKECMDGOALS contains
>  # something else than one of the nobuild_targets.
>  nobuild_targets := source %-source \
> +	source-check %-source-check %-all-source-check \
>  	legal-info %-legal-info external-deps _external-deps \
>  	clean distclean help show-targets graph-depends \
>  	%-graph-depends %-show-depends %-show-version \
> @@ -788,6 +789,9 @@ target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
>  .PHONY: source
>  source: $(foreach p,$(PACKAGES),$(p)-all-source)
>  
> +.PHONY: source-check
> +source-check: $(foreach p,$(PACKAGES),$(p)-all-source-check)
> +
>  .PHONY: _external-deps external-deps
>  _external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps)
>  external-deps:
> @@ -1070,6 +1074,8 @@ help:
>  	@echo '  <pkg>-dirclean         - Remove <pkg> build directory'
>  	@echo '  <pkg>-reconfigure      - Restart the build from the configure step'
>  	@echo '  <pkg>-rebuild          - Restart the build from the build step'
> +	@echo '  <pkg>-source-check     - Check package for valid download URLs'
> +	@echo '  <pkg>-all-source-check - Check package and its dependencies for valid download URLs'
>  	$(foreach p,$(HELP_PACKAGES), \
>  		@echo $(sep) \
>  		@echo '$($(p)_NAME):' $(sep) \
> @@ -1089,6 +1095,7 @@ help:
>  	@echo
>  	@echo 'Miscellaneous:'
>  	@echo '  source                 - download all sources needed for offline-build'
> +	@echo '  source-check           - check selected packages for valid download URLs'
>  	@echo '  external-deps          - list external packages used'
>  	@echo '  legal-info             - generate info about license compliance'
>  	@echo '  printvars              - dump all the internal variables'
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 7cd87c38ff..cc04e316e2 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -106,3 +106,22 @@ define DOWNLOAD
>  		-- \
>  		$($(PKG)_DL_OPTS)
>  endef
> +
> +define SOURCE_CHECK
> +	$(Q)mkdir -p $($(PKG)_DL_DIR)
> +	$(Q)$(EXTRA_ENV) $(FLOCK) $(DL_WRAPPER) \
> +		-C \
> +		-c '$($(PKG)_DL_VERSION)' \
> +		-d '$($(PKG)_DL_DIR)' \
> +		-D '$(DL_DIR)' \
> +		-f '$(notdir $(1))' \
> +		-H '$($(PKG)_HASH_FILE)' \
> +		-n '$($(PKG)_BASENAME_RAW)' \
> +		-N '$($(PKG)_RAWNAME)' \
> +		-o '$($(PKG)_DL_DIR)/$(notdir $(1))' \
> +		$(if $($(PKG)_GIT_SUBMODULES),-r) \
> +		$(DOWNLOAD_URIS) \
> +		$(QUIET) \
> +		-- \
> +		$($(PKG)_DL_OPTS)
> +endef
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f5cab2b9c2..707996e384 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -780,6 +780,10 @@ $(1)-legal-source:	$$($(2)_TARGET_ACTUAL_SOURCE)
>  endif # actual sources != sources
>  endif # actual sources != ""
>  
> +$(1)-source-check: PKG=$(2)
> +$(1)-source-check:
> +	$$(foreach p,$$($(2)_ALL_DOWNLOADS),$$(call SOURCE_CHECK,$$(p))$$(sep))
> +
>  $(1)-external-deps:
>  	@for p in $$($(2)_SOURCE) $$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS) ; do \
>  		echo `basename $$$$p` ; \
> @@ -804,6 +808,9 @@ $(1)-rsync:		$$($(2)_TARGET_RSYNC)
>  $(1)-source:
>  $(1)-legal-source:
>  
> +$(1)-source-check:
> +	test -d $$($(2)_OVERRIDE_SRCDIR)
> +
>  $(1)-external-deps:
>  	@echo "file://$$($(2)_OVERRIDE_SRCDIR)"
>  endif
> @@ -838,6 +845,9 @@ $(1)-graph-rdepends: graph-depends-requirements
>  $(1)-all-source:	$(1)-source
>  $(1)-all-source:	$$(foreach p,$$($(2)_FINAL_ALL_DEPENDENCIES),$$(p)-all-source)
>  
> +$(1)-all-source-check:	$(1)-source-check
> +$(1)-all-source-check:	$$(foreach p,$$($(2)_FINAL_ALL_DEPENDENCIES),$$(p)-all-source-check)
> +
>  $(1)-all-external-deps:	$(1)-external-deps
>  $(1)-all-external-deps:	$$(foreach p,$$($(2)_FINAL_ALL_DEPENDENCIES),$$(p)-all-external-deps)
>  
> @@ -1036,6 +1046,7 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>  	$(1)-all-external-deps \
>  	$(1)-all-legal-info \
>  	$(1)-all-source \
> +	$(1)-all-source-check \
>  	$(1)-build \
>  	$(1)-clean-for-rebuild \
>  	$(1)-clean-for-reconfigure \
> @@ -1060,7 +1071,8 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>  	$(1)-rsync \
>  	$(1)-show-depends \
>  	$(1)-show-version \
> -	$(1)-source
> +	$(1)-source \
> +	$(1)-source-check
>  
>  ifneq ($$($(2)_SOURCE),)
>  ifeq ($$($(2)_SITE),)
> diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
> index 3315bd410e..1ed2e654de 100755
> --- a/support/download/dl-wrapper
> +++ b/support/download/dl-wrapper
> @@ -17,7 +17,7 @@
>  # We want to catch any unexpected failure, and exit immediately.
>  set -e
>  
> -export BR_BACKEND_DL_GETOPTS=":hc:d:o:n:N:H:ru:qf:e"
> +export BR_BACKEND_DL_GETOPTS=":hc:Cd:o:n:N:H:ru:qf:e"
>  
>  main() {
>      local OPT OPTARG
> @@ -25,9 +25,10 @@ main() {
>      local -a uris
>  
>      # Parse our options; anything after '--' is for the backend
> -    while getopts ":c:d:D:o:n:N:H:rf:u:q" OPT; do
> +    while getopts ":c:Cd:D:o:n:N:H:rf:u:q" OPT; do
>          case "${OPT}" in
>          c)  cset="${OPTARG}";;
> +        C)  checkonly=-C;;
>          d)  dl_dir="${OPTARG}";;
>          D)  old_dl_dir="${OPTARG}";;
>          o)  output="${OPTARG}";;
> @@ -46,38 +47,40 @@ main() {
>      # Forget our options, and keep only those for the backend
>      shift $((OPTIND-1))
>  
> -    if [ -z "${output}" ]; then
> -        error "no output specified, use -o\n"
> -    fi
> +    if [ -z "${checkonly}" ]; then
> +        if [ -z "${output}" ]; then
> +            error "no output specified, use -o\n"
> +        fi
>  
> -    # Legacy handling: check if the file already exists in the global
> -    # download directory. If it does, hard-link it. If it turns out it
> -    # was an incorrect download, we'd still check it below anyway.
> -    # If we can neither link nor copy, fallback to doing a download.
> -    # NOTE! This is not atomic, is subject to TOCTTOU, but the whole
> -    # dl-wrapper runs under an flock, so we're safe.
> -    if [ ! -e "${output}" -a -e "${old_dl_dir}/${filename}" ]; then
> -        ln "${old_dl_dir}/${filename}" "${output}" || \
> -        cp "${old_dl_dir}/${filename}" "${output}" || \
> -        true
> -    fi
> +        # Legacy handling: check if the file already exists in the global
> +        # download directory. If it does, hard-link it. If it turns out it
> +        # was an incorrect download, we'd still check it below anyway.
> +        # If we can neither link nor copy, fallback to doing a download.
> +        # NOTE! This is not atomic, is subject to TOCTTOU, but the whole
> +        # dl-wrapper runs under an flock, so we're safe.
> +        if [ ! -e "${output}" -a -e "${old_dl_dir}/${filename}" ]; then
> +            ln "${old_dl_dir}/${filename}" "${output}" || \
> +            cp "${old_dl_dir}/${filename}" "${output}" || \
> +            true
> +        fi
>  
> -    # If the output file already exists and:
> -    # - there's no .hash file: do not download it again and exit promptly
> -    # - matches all its hashes: do not download it again and exit promptly
> -    # - fails at least one of its hashes: force a re-download
> -    # - there's no hash (but a .hash file): consider it a hard error
> -    if [ -e "${output}" ]; then
> -        if support/download/check-hash ${quiet} "${hfile}" "${output}" "${output##*/}"; then
> -            exit 0
> -        elif [ ${?} -ne 2 ]; then
> -            # Do not remove the file, otherwise it might get re-downloaded
> -            # from a later location (i.e. primary -> upstream -> mirror).
> -            # Do not print a message, check-hash already did.
> -            exit 1
> +        # If the output file already exists and:
> +        # - there's no .hash file: do not download it again and exit promptly
> +        # - matches all its hashes: do not download it again and exit promptly
> +        # - fails at least one of its hashes: force a re-download
> +        # - there's no hash (but a .hash file): consider it a hard error
> +        if [ -e "${output}" ]; then
> +            if support/download/check-hash ${quiet} "${hfile}" "${output}" "${output##*/}"; then
> +                exit 0
> +            elif [ ${?} -ne 2 ]; then
> +                # Do not remove the file, otherwise it might get re-downloaded
> +                # from a later location (i.e. primary -> upstream -> mirror).
> +                # Do not print a message, check-hash already did.
> +                exit 1
> +            fi
> +            rm -f "${output}"
> +            warn "Re-downloading '%s'...\n" "${output##*/}"
>          fi
> -        rm -f "${output}"
> -        warn "Re-downloading '%s'...\n" "${output##*/}"
>      fi
>  
>      # Look through all the uris that we were given to download the package
> @@ -127,7 +130,7 @@ main() {
>                  -f "${filename}" \
>                  -u "${uri}" \
>                  -o "${tmpf}" \
> -                ${quiet} ${recurse} -- "${@}"
> +                ${quiet} ${recurse} ${checkonly} -- "${@}"
>          then
>              # cd back to keep path coherence
>              cd "${OLDPWD}"
> @@ -138,19 +141,21 @@ main() {
>          # cd back to free the temp-dir, so we can remove it later
>          cd "${OLDPWD}"
>  
> -        # Check if the downloaded file is sane, and matches the stored hashes
> -        # for that file
> -        if support/download/check-hash ${quiet} "${hfile}" "${tmpf}" "${output##*/}"; then
> -            rc=0
> -        else
> -            if [ ${?} -ne 3 ]; then
> -                rm -rf "${tmpd}"
> -                continue
> +        if [ -z "${checkonly}" ]; then
> +            # Check if the downloaded file is sane, and matches the stored hashes
> +            # for that file
> +            if support/download/check-hash ${quiet} "${hfile}" "${tmpf}" "${output##*/}"; then
> +                rc=0
> +            else
> +                if [ ${?} -ne 3 ]; then
> +                    rm -rf "${tmpd}"
> +                    continue
> +                fi
> +
> +                # the hash file exists and there was no hash to check the file
> +                # against
> +                rc=1
>              fi
> -
> -            # the hash file exists and there was no hash to check the file
> -            # against
> -            rc=1
>          fi
>          download_and_check=1
>          break
> @@ -163,6 +168,12 @@ main() {
>          exit 1
>      fi
>  
> +    # If we only need to check the presence of sources, stop here.
> +    # No need to handle output files.
> +    if [ -n "${checkonly}" ]; then
> +        exit 0
> +    fi
> +
>      # tmp_output is in the same directory as the final output, so we can
>      # later move it atomically.
>      tmp_output="$(mktemp "${output}.XXXXXX")"
> -- 
> 2.18.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 03/11] support/download: reintroduce 'source-check' target

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 > But still, at some point, you will want your CI to actually test the
 > change, so you will need to have the stuff downloaded... So, why can't
 > you simply use 'make source && make' ? It would (mostly) have the actual
 > result you are looking for: do the check that everything is available,
 > and if it is, then the build can proceed. If something was missing, that
 > would have bailed out early.

 > The only thing that differs with source-check, is that the network will
 > actually be used (boohoo!).

 > However, since we added local cache for git, you would not need to fetch
 > much. Also, tarballs (from wget et al) were already cached locally. Of
 > course, that means you'd have to have BR2_DL_DIR in your envioronment,
 > pointing to a dl location that is persistent...

 > What is missing (guess it) is a local cache for Hg. I started working on
 > it a while ago (rght after the git cache was merged), but dropped it as
 > I had no way to throughly test it.

 > So, if you are really concerned about not exhausting your internal
 > network that much (I know some companies have slow links between remote
 > sites, so I understand [0]), what about you provide an Hg caching like
 > we have for git instead? ;-)

 > So, I am definitely not convinced by the need for source-check...

Agreed. Thomas, can you explain in more detail why you think
source-check is needed?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 03/11] support/download: reintroduce 'source-check' target

[Thomas De Schampheleire]

On Thu, Jan 3, 2019, 22:41 Peter Korsgaard <peter@korsgaard.com wrote:

> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>
> Hi,
>
>  > But still, at some point, you will want your CI to actually test the
>  > change, so you will need to have the stuff downloaded... So, why can't
>  > you simply use 'make source && make' ? It would (mostly) have the actual
>  > result you are looking for: do the check that everything is available,
>  > and if it is, then the build can proceed. If something was missing, that
>  > would have bailed out early.
>
>  > The only thing that differs with source-check, is that the network will
>  > actually be used (boohoo!).
>
>  > However, since we added local cache for git, you would not need to fetch
>  > much. Also, tarballs (from wget et al) were already cached locally. Of
>  > course, that means you'd have to have BR2_DL_DIR in your envioronment,
>  > pointing to a dl location that is persistent...
>
>  > What is missing (guess it) is a local cache for Hg. I started working on
>  > it a while ago (rght after the git cache was merged), but dropped it as
>  > I had no way to throughly test it.
>
>  > So, if you are really concerned about not exhausting your internal
>  > network that much (I know some companies have slow links between remote
>  > sites, so I understand [0]), what about you provide an Hg caching like
>  > we have for git instead? ;-)
>
>  > So, I am definitely not convinced by the need for source-check...
>
> Agreed. Thomas, can you explain in more detail why you think
> source-check is needed?
>

Okay, give me some time, I will gather some numbers and a well-thought-out
response :-)

Best regards,
Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190104/1f8bbbf1/attachment.html>

[Buildroot] [PATCH 03/11] support/download: reintroduce 'source-check' target

[Thomas De Schampheleire]

Hello,

El vie., 4 ene. 2019 a las 10:07, Thomas De Schampheleire
(<patrickdepinguin@gmail.com>) escribi?:
> On Thu, Jan 3, 2019, 22:41 Peter Korsgaard <peter@korsgaard.com wrote:
>>
>> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>>
>> Hi,
>>
>>  > But still, at some point, you will want your CI to actually test the
>>  > change, so you will need to have the stuff downloaded... So, why can't
>>  > you simply use 'make source && make' ? It would (mostly) have the actual
>>  > result you are looking for: do the check that everything is available,
>>  > and if it is, then the build can proceed. If something was missing, that
>>  > would have bailed out early.
>>
>>  > The only thing that differs with source-check, is that the network will
>>  > actually be used (boohoo!).
>>
>>  > However, since we added local cache for git, you would not need to fetch
>>  > much. Also, tarballs (from wget et al) were already cached locally. Of
>>  > course, that means you'd have to have BR2_DL_DIR in your envioronment,
>>  > pointing to a dl location that is persistent...
>>
>>  > What is missing (guess it) is a local cache for Hg. I started working on
>>  > it a while ago (rght after the git cache was merged), but dropped it as
>>  > I had no way to throughly test it.
>>
>>  > So, if you are really concerned about not exhausting your internal
>>  > network that much (I know some companies have slow links between remote
>>  > sites, so I understand [0]), what about you provide an Hg caching like
>>  > we have for git instead? ;-)
>>
>>  > So, I am definitely not convinced by the need for source-check...
>>
>> Agreed. Thomas, can you explain in more detail why you think
>> source-check is needed?
>
>
> Okay, give me some time, I will gather some numbers and a well-thought-out response :-)
>

source-check could be used with two different goals in mind:
1. to verify that sources needed for a defconfig are available, either locally
   (in BR2_DL_DIR) or on a remote location (i.e. BR2_PRIMARY_SITE, upstream, or
   BR2_BACKUP_SITE)
2. to verify that sources needed for a defconfig are available in the remote
   location (i.e. BR2_PRIMARY_SITE, upstream, or BR2_BACKUP_SITE).


If you are only interested in goal 1, one could argue that 'source-check' and
'source' are very similar except for the actual download of missing files.
Once you have a (partially) populated download directory, the impact of 'make
source' may be reasonable.

At this point there are two 'buts':
- if you start from an empty download directory, then the 'source'
target is much more
  heavyweight than 'source-check'. This could be e.g. in a CI server with
  multiple executor nodes and the job running 'make source/source-check' can be
  executed on any one of these nodes.

- if the sources of certain packages change a lot, e.g. linux kernel sources,
  then even if you have a populated download directory, there will still be
  large downloads. If these sources reside in a git repository, then repo
  caching will help a lot. For mercurial repositories caching is not currently
  implemented but could be added. For actual tarballs this problem is still
  important.


If you are (also) interested in the second goal (verify the remote presence of
all sources), then things are a bit different. In this case, you don't want to
rely on local presence of files, but exhaustively check that the sources you
need are still downloadable.
A specific variant of this use case is when you want to verify that your
BR2_PRIMARY_SITE is complete, so that you are sure you will be able to make your
builds in 10 years time even if upstream locations disappear. In this case, you
can force-set BR2_BACKUP_SITE to empty, and set an invalid proxy.
In this case, to remove reliance on local files, you would empty your existing
download directory or force a scratch location with BR2_DL_DIR. Here, 'make
source' will download all tarballs, each time you run the test.


Some data from our use case:
- we have 30 defconfigs that need to be tested
- a source-check for all defconfigs, distributed over 8 processes, takes about 3
  to 5 minutes on my machine when connected to the corporate network and forcing
  download from the primary site (which is in the same network).
- a 'make source' in the same environment as above, takes 13-14 minutes. Total
  download size when using separate dl directories per defconfig is 11 GB.
- improving the 'make source' case by sharing one dl directory for all
  defconfigs, brings down the total download size to 3 GB.
  With one process, it takes 24 minutes to handle all 30 defconfigs.
  With 8 processes, it takes 12 minutes.
  With 12 processes, it still takes about 11-12 minutes.
- our 'primary site' is accessed via scp.
- 'hg' downloads are intercepted and changed to download the archive for the
  requested version directly from the scp server, i.e. there is no actual
  clone/pull happening, the server creates the archive.


The 12 minutes do not seem very long, but this is the best-case scenario. When
the verification is done from a network not physically next to the primary site,
the download time will increase significantly.
For example, when working from home, I see an average download speed in this
test of about 500 KB/s, meaning the 3 GB download will take more than 2 hours,
while all I am interested in is the simple fact that everything is there (in
this scenario taking 15 minutes).

Other factors to consider:
- 'make source' will actually touch files on the disk. For SSD this means
  unnecessary write cycles wearing down flash sectors. For spinning hard disk
  this means impact on other hard disk I/O happening at the same time. This
  could be mitigated by using a 'tmpfs' if you have the right to create such
  mounts (which a regular user normally does not have).

- This check may happen on other nodes or in other workspaces than where the
  actual build will happen. So, a real download is not necessarily needed.
  (this is in response to a comment from Yann).

From my point of view, adding source-check does not bring many disadvantages.
It's just an additional thing people could use if it fits their needs, without
hogging network and I/O buses.

From your side, I would like to understand what exactly is causing your
reservations. Are you concerned about additional complexity, lack of testing
(which I could cover in support/testing), ... ?

Thanks,
Thomas

[Buildroot] [PATCH 03/11] support/download: reintroduce 'source-check' target

[Thomas De Schampheleire]

El mar., 8 ene. 2019 a las 13:11, Thomas De Schampheleire
(<patrickdepinguin@gmail.com>) escribi?:
>
> Hello,
>
> El vie., 4 ene. 2019 a las 10:07, Thomas De Schampheleire
> (<patrickdepinguin@gmail.com>) escribi?:
> > On Thu, Jan 3, 2019, 22:41 Peter Korsgaard <peter@korsgaard.com wrote:
> >>
> >> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> >>
> >> Hi,
> >>
> >>  > But still, at some point, you will want your CI to actually test the
> >>  > change, so you will need to have the stuff downloaded... So, why can't
> >>  > you simply use 'make source && make' ? It would (mostly) have the actual
> >>  > result you are looking for: do the check that everything is available,
> >>  > and if it is, then the build can proceed. If something was missing, that
> >>  > would have bailed out early.
> >>
> >>  > The only thing that differs with source-check, is that the network will
> >>  > actually be used (boohoo!).
> >>
> >>  > However, since we added local cache for git, you would not need to fetch
> >>  > much. Also, tarballs (from wget et al) were already cached locally. Of
> >>  > course, that means you'd have to have BR2_DL_DIR in your envioronment,
> >>  > pointing to a dl location that is persistent...
> >>
> >>  > What is missing (guess it) is a local cache for Hg. I started working on
> >>  > it a while ago (rght after the git cache was merged), but dropped it as
> >>  > I had no way to throughly test it.
> >>
> >>  > So, if you are really concerned about not exhausting your internal
> >>  > network that much (I know some companies have slow links between remote
> >>  > sites, so I understand [0]), what about you provide an Hg caching like
> >>  > we have for git instead? ;-)
> >>
> >>  > So, I am definitely not convinced by the need for source-check...
> >>
> >> Agreed. Thomas, can you explain in more detail why you think
> >> source-check is needed?
> >
> >
> > Okay, give me some time, I will gather some numbers and a well-thought-out response :-)
> >
>
> source-check could be used with two different goals in mind:
> 1. to verify that sources needed for a defconfig are available, either locally
>    (in BR2_DL_DIR) or on a remote location (i.e. BR2_PRIMARY_SITE, upstream, or
>    BR2_BACKUP_SITE)
> 2. to verify that sources needed for a defconfig are available in the remote
>    location (i.e. BR2_PRIMARY_SITE, upstream, or BR2_BACKUP_SITE).
>
>
> If you are only interested in goal 1, one could argue that 'source-check' and
> 'source' are very similar except for the actual download of missing files.
> Once you have a (partially) populated download directory, the impact of 'make
> source' may be reasonable.
>
> At this point there are two 'buts':
> - if you start from an empty download directory, then the 'source'
> target is much more
>   heavyweight than 'source-check'. This could be e.g. in a CI server with
>   multiple executor nodes and the job running 'make source/source-check' can be
>   executed on any one of these nodes.
>
> - if the sources of certain packages change a lot, e.g. linux kernel sources,
>   then even if you have a populated download directory, there will still be
>   large downloads. If these sources reside in a git repository, then repo
>   caching will help a lot. For mercurial repositories caching is not currently
>   implemented but could be added. For actual tarballs this problem is still
>   important.
>
>
> If you are (also) interested in the second goal (verify the remote presence of
> all sources), then things are a bit different. In this case, you don't want to
> rely on local presence of files, but exhaustively check that the sources you
> need are still downloadable.
> A specific variant of this use case is when you want to verify that your
> BR2_PRIMARY_SITE is complete, so that you are sure you will be able to make your
> builds in 10 years time even if upstream locations disappear. In this case, you
> can force-set BR2_BACKUP_SITE to empty, and set an invalid proxy.
> In this case, to remove reliance on local files, you would empty your existing
> download directory or force a scratch location with BR2_DL_DIR. Here, 'make
> source' will download all tarballs, each time you run the test.
>
>
> Some data from our use case:
> - we have 30 defconfigs that need to be tested
> - a source-check for all defconfigs, distributed over 8 processes, takes about 3
>   to 5 minutes on my machine when connected to the corporate network and forcing
>   download from the primary site (which is in the same network).
> - a 'make source' in the same environment as above, takes 13-14 minutes. Total
>   download size when using separate dl directories per defconfig is 11 GB.
> - improving the 'make source' case by sharing one dl directory for all
>   defconfigs, brings down the total download size to 3 GB.
>   With one process, it takes 24 minutes to handle all 30 defconfigs.
>   With 8 processes, it takes 12 minutes.
>   With 12 processes, it still takes about 11-12 minutes.
> - our 'primary site' is accessed via scp.
> - 'hg' downloads are intercepted and changed to download the archive for the
>   requested version directly from the scp server, i.e. there is no actual
>   clone/pull happening, the server creates the archive.
>
>
> The 12 minutes do not seem very long, but this is the best-case scenario. When
> the verification is done from a network not physically next to the primary site,
> the download time will increase significantly.
> For example, when working from home, I see an average download speed in this
> test of about 500 KB/s, meaning the 3 GB download will take more than 2 hours,
> while all I am interested in is the simple fact that everything is there (in
> this scenario taking 15 minutes).
>
> Other factors to consider:
> - 'make source' will actually touch files on the disk. For SSD this means
>   unnecessary write cycles wearing down flash sectors. For spinning hard disk
>   this means impact on other hard disk I/O happening at the same time. This
>   could be mitigated by using a 'tmpfs' if you have the right to create such
>   mounts (which a regular user normally does not have).
>
> - This check may happen on other nodes or in other workspaces than where the
>   actual build will happen. So, a real download is not necessarily needed.
>   (this is in response to a comment from Yann).
>
> From my point of view, adding source-check does not bring many disadvantages.
> It's just an additional thing people could use if it fits their needs, without
> hogging network and I/O buses.
>
> From your side, I would like to understand what exactly is causing your
> reservations. Are you concerned about additional complexity, lack of testing
> (which I could cover in support/testing), ... ?
>

Any feedback? Thanks.

[Buildroot] [PATCH 04/11] support/download: implement source-check in hg backend

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Note: the implementation is different (better) than what used to be in
Buildroot before source-check was removed.

The original implementation:
    hg incoming --force -l1 <URL>
would only verify that the repository exists, not that the requested
revision is present.

An already better implementation is:
    hg incoming --force -l1 -r <revision> <URL>
but compared to the next solution it has a large resource consumption on the
local machine. In the background, the full repository is first downloaded.

The implemented solution is:
    hg identify -r <revision> <URL>
which operates directly on the remote repository.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/hg | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/support/download/hg b/support/download/hg
index efb515fca5..7c698ee7e3 100755
--- a/support/download/hg
+++ b/support/download/hg
@@ -7,6 +7,7 @@ set -e
 #
 # Options:
 #   -q          Be quiet.
+#   -C          Only check that the changeset exists in the remote repository
 #   -o FILE     Generate archive in FILE.
 #   -u URI      Clone from repository at URI.
 #   -c CSET     Use changeset (or revision) CSET.
@@ -19,6 +20,7 @@ verbose=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
     q)  verbose=-q;;
+    C)  checkonly=1;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG}";;
     c)  cset="${OPTARG}";;
@@ -36,6 +38,11 @@ _hg() {
     eval ${HG} "${@}"
 }
 
+if [ -n "${checkonly}" ]; then
+    _hg identify ${verbose} "${@}" --rev "'${cset}'" "'${uri}'" > /dev/null
+    exit ${?}
+fi
+
 _hg clone ${verbose} "${@}" --noupdate "'${uri}'" "'${basename}'"
 
 _hg archive ${verbose} --repository "'${basename}'" --type tgz \
-- 
2.18.1

[Buildroot] [PATCH 05/11] support/download: implement source-check in wget backend

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/wget | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/support/download/wget b/support/download/wget
index c69e6071aa..7f626e0071 100755
--- a/support/download/wget
+++ b/support/download/wget
@@ -7,6 +7,7 @@ set -e
 #
 # Options:
 #   -q          Be quiet.
+#   -C          Only check that the file exists remotely
 #   -o FILE     Save into file FILE.
 #   -f FILENAME The filename of the tarball to get at URL
 #   -u URL      Download file at URL.
@@ -19,6 +20,7 @@ verbose=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
     q)  verbose=-q;;
+    C)  checkonly=1;;
     o)  output="${OPTARG}";;
     f)  filename="${OPTARG}";;
     u)  url="${OPTARG}";;
@@ -40,4 +42,9 @@ _wget() {
 # mirror
 [ -n "${encode}" ] && filename=${filename//\?/%3F}
 
+if [ -n "${checkonly}" ]; then
+    _wget --spider ${verbose} "${@}" "'${url}/${filename}'"
+    exit ${?}
+fi
+
 _wget ${verbose} "${@}" -O "'${output}'" "'${url}/${filename}'"
-- 
2.18.1

[Buildroot] [PATCH 06/11] support/download: implement source-check in file backend

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/file | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/support/download/file b/support/download/file
index e52fcf2c8c..3a92fa9f67 100755
--- a/support/download/file
+++ b/support/download/file
@@ -7,6 +7,7 @@ set -e
 #
 # Options:
 #   -q          Be quiet.
+#   -C          Only check that the source file exists
 #   -o FILE     Copy to file FILE.
 #   -f FILE     Copy from basename file FILE.
 #   -u DIR      Copy from FILE in DIR.
@@ -23,6 +24,7 @@ verbose=-v
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
     q)  verbose=;;
+    C)  checkonly=1;;
     o)  output="${OPTARG}";;
     f)  file="${OPTARG}";;
     u)  dir="${OPTARG}";;
@@ -39,4 +41,12 @@ _localfiles() {
     eval ${LOCALFILES} "${@}"
 }
 
-_localfiles ${verbose} "'${dir##file://}/${file}'" "'${output}'"
+# Remove any scheme prefix
+dir="${dir##file://}"
+
+if [ -n "${checkonly}" ]; then
+    test -e "'${dir}/${file}'"
+    exit ${?}
+fi
+
+_localfiles ${verbose} "'${dir}/${file}'" "'${output}'"
-- 
2.18.1

[Buildroot] [PATCH 07/11] support/download: implement source-check in scp backend

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Note that this reintroduces BR2_SSH removed with
db9473bf6cd7bd12aa1f9faad0a917c973c33827 ("core/download: drop the SSH
command").

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 Config.in               |  4 ++++
 package/pkg-download.mk |  1 +
 support/download/scp    | 13 +++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/Config.in b/Config.in
index f965e9d6d8..03e4eb3928 100644
--- a/Config.in
+++ b/Config.in
@@ -136,6 +136,10 @@ config BR2_SCP
 	string "Secure copy (scp) command"
 	default "scp"
 
+config BR2_SSH
+	string "Secure shell (ssh) command"
+	default "ssh"
+
 config BR2_HG
 	string "Mercurial (hg) command"
 	default "hg"
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index cc04e316e2..e9506ae2a5 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -15,6 +15,7 @@ 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 SSH := $(call qstrip,$(BR2_SSH))
 export LOCALFILES := $(call qstrip,$(BR2_LOCALFILES))
 
 DL_WRAPPER = support/download/dl-wrapper
diff --git a/support/download/scp b/support/download/scp
index 55f588e157..c8fa58f952 100755
--- a/support/download/scp
+++ b/support/download/scp
@@ -7,17 +7,20 @@ set -e
 #
 # Options:
 #   -q          Be quiet.
+#   -C          Only check that the file exists remotely
 #   -o FILE     Copy to local file FILE.
 #   -f FILE     Copy from remote file FILE.
 #   -u URL      Download file at URL.
 #
 # Environment:
 #   SCP       : the scp command to call
+#   SSH       : the ssh command to use for checkonly
 
 verbose=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
     q)  verbose=-q;;
+    C)  checkonly=1;;
     o)  output="${OPTARG}";;
     f)  filename="${OPTARG}";;
     u)  uri="${OPTARG}";;
@@ -33,6 +36,16 @@ shift $((OPTIND-1)) # Get rid of our options
 _scp() {
     eval ${SCP} "${@}"
 }
+_ssh() {
+    eval ${SSH} "${@}"
+}
+
+if [ -n "${checkonly}" ]; then
+    domain="${uri%%:*}"
+    path="${uri#*:}"
+    _ssh ${verbose} "${@}" "'${domain}'" ls "'${path}'" > /dev/null
+    exit ${?}
+fi
 
 # Remove any scheme prefix
 uri="${uri##scp://}"
-- 
2.18.1

[Buildroot] [PATCH 08/11] support/download: implement source-check in git backend

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

The implementation is the same as originally was present.
It suffers from the disadvantage that an invalid revision on a valid URL
will not be detected.

However, git does not seem to allow a good way to remotely check the
validity of a revision, without cloning the repository.

For source-check, we don't want to do such download which can be large.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/git | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/support/download/git b/support/download/git
index 17ca04eb98..54f3494c61 100755
--- a/support/download/git
+++ b/support/download/git
@@ -7,6 +7,7 @@ set -E
 #
 # Options:
 #   -q          Be quiet.
+#   -C          Only check that the changeset exists in the remote repository
 #   -r          Clone and archive sub-modules.
 #   -o FILE     Generate archive in FILE.
 #   -u URI      Clone from repository at URI.
@@ -48,6 +49,7 @@ recurse=0
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
     q)  verbose=-q; exec >/dev/null;;
+    C)  checkonly=1;;
     r)  recurse=1;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG}";;
@@ -61,6 +63,20 @@ 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)
+_git() {
+    eval GIT_DIR="${git_cache}/.git" ${GIT} "${@}"
+}
+
+if [ -n "${checkonly}" ]; then
+    # TODO this check only checks that the remote repository exists, not that
+    # it actually contains the requested revision. And for checkonly, we want
+    # to avoid creating a clone first.
+    _git ls-remote --heads "${@}" "'${uri}'" > /dev/null
+    exit ${?}
+fi
+
 # Create and cd into the directory that will contain the local git cache
 git_cache="${dl_dir}/git"
 mkdir -p "${git_cache}"
@@ -69,12 +85,6 @@ pushd "${git_cache}" >/dev/null
 # Any error now should try to recover
 trap _on_error ERR
 
-# Caller needs to single-quote its arguments to prevent them from
-# being expanded a second time (in case there are spaces in them)
-_git() {
-    eval GIT_DIR="${git_cache}/.git" ${GIT} "${@}"
-}
-
 # Create a warning file, that the user should not use the git cache.
 # It's ours. Our precious.
 cat <<-_EOF_ >"${dl_dir}/git.readme"
-- 
2.18.1

[Buildroot] [PATCH 08/11] support/download: implement source-check in git backend

[Thomas Petazzoni]

Hello Thomas,

On Thu,  3 Jan 2019 21:40:23 +0100, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> The implementation is the same as originally was present.
> It suffers from the disadvantage that an invalid revision on a valid URL
> will not be detected.
> 
> However, git does not seem to allow a good way to remotely check the
> validity of a revision, without cloning the repository.
> 
> For source-check, we don't want to do such download which can be large.

While I understand the limitation, I don't really agree with the
conclusion: we should go ahead and download the full thing. Indeed the
selling argument for source-check in your cover letter is precisely to
verify that the version of the package that has been committed by
someone is *really* available. If there's no version check in the git,
bzr and cvs source-check implementation, it makes the selling point of
the cover letter a bit moot, no?

Of course, I realize that your primary interest is in hg, and hg has
this capability. But still we should ensure git/bzr/cvs provide the same
guarantees, by falling back to the slower but working method of
downloading everything.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 08/11] support/download: implement source-check in git backend

[Yann E. MORIN]

Thomas, DS, Thomas P, All,

On 2019-01-03 21:59 +0100, Thomas Petazzoni spake thusly:
> On Thu,  3 Jan 2019 21:40:23 +0100, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > 
> > The implementation is the same as originally was present.
> > It suffers from the disadvantage that an invalid revision on a valid URL
> > will not be detected.
> > 
> > However, git does not seem to allow a good way to remotely check the
> > validity of a revision, without cloning the repository.
> > 
> > For source-check, we don't want to do such download which can be large.
> 
> While I understand the limitation, I don't really agree with the
> conclusion: we should go ahead and download the full thing. Indeed the
> selling argument for source-check in your cover letter is precisely to
> verify that the version of the package that has been committed by
> someone is *really* available. If there's no version check in the git,
> bzr and cvs source-check implementation, it makes the selling point of
> the cover letter a bit moot, no?

Agreed here. *If* we were to re-add support for source-check, it should
really behave as it says on the can: check that the source really
exists. If a backend can't do a cheap check, then it has to download
everything.

> Of course, I realize that your primary interest is in hg, and hg has
> this capability. But still we should ensure git/bzr/cvs provide the same
> guarantees, by falling back to the slower but working method of
> downloading everything.

Yup.

But this is not an endorsement of re-adding source-check, mind you1 ;-)
I'm still unconvinced we need it.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 08/11] support/download: implement source-check in git backend

[Thomas De Schampheleire]

El jue., 3 ene. 2019 a las 22:00, Thomas Petazzoni
(<thomas.petazzoni@bootlin.com>) escribi?:
>
> Hello Thomas,
>
> On Thu,  3 Jan 2019 21:40:23 +0100, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > The implementation is the same as originally was present.
> > It suffers from the disadvantage that an invalid revision on a valid URL
> > will not be detected.
> >
> > However, git does not seem to allow a good way to remotely check the
> > validity of a revision, without cloning the repository.
> >
> > For source-check, we don't want to do such download which can be large.
>
> While I understand the limitation, I don't really agree with the
> conclusion: we should go ahead and download the full thing. Indeed the
> selling argument for source-check in your cover letter is precisely to
> verify that the version of the package that has been committed by
> someone is *really* available. If there's no version check in the git,
> bzr and cvs source-check implementation, it makes the selling point of
> the cover letter a bit moot, no?
>
> Of course, I realize that your primary interest is in hg, and hg has
> this capability. But still we should ensure git/bzr/cvs provide the same
> guarantees, by falling back to the slower but working method of
> downloading everything.

Yes, I can agree with this. If you accept adding source-check in the
first place, then I'll improve these backends.

/Thomas

[Buildroot] [PATCH 09/11] support/download: implement source-check in bzr backend

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

The implementation is the same as originally was present.
It suffers from the disadvantage that an invalid revision on a valid URL
will not be detected.
If someone is proficient in bzr-speak, please improve.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/bzr | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/support/download/bzr b/support/download/bzr
index 5289a421cd..2f13a607c9 100755
--- a/support/download/bzr
+++ b/support/download/bzr
@@ -7,6 +7,7 @@ set -e
 #
 # Options:
 #   -q          Be quiet
+#   -C          Only check that the changeset exists in the remote repository
 #   -o FILE     Generate archive in FILE.
 #   -u URI      Clone from repository at URI.
 #   -c CSET     Use changeset (or revision) CSET.
@@ -20,6 +21,7 @@ verbose=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
     q)  verbose=-q;;
+    C)  checkonly=1;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG}";;
     c)  cset="${OPTARG}";;
@@ -37,6 +39,13 @@ _bzr() {
     eval ${BZR} "${@}"
 }
 
+if [ -n "${checkonly}" ]; then
+    # TODO this check only checks that the remote repository exists, not that
+    # it actually contains the requested revision.
+    _bzr ls --quiet "${@}" "'${uri}'" > /dev/null
+    exit ${?}
+fi
+
 # --per-file-timestamps comes with bzr-2.2 (released August 2010),
 # so only pass it if bzr is recent enough. We compute versions as:
 # major*1000 + minor
-- 
2.18.1

[Buildroot] [PATCH 10/11] support/download: implement source-check in cvs backend

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

The implementation is the same as originally was present.
It suffers from the disadvantage that an invalid revision on a valid URL
will not be detected.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/cvs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/support/download/cvs b/support/download/cvs
index 9d0dc3cb3a..0ae7aa30d2 100755
--- a/support/download/cvs
+++ b/support/download/cvs
@@ -7,6 +7,7 @@ set -e
 #
 # Options:
 #   -q          Be quiet
+#   -C          Only check that the revision exists in the remote repository
 #   -o FILE     Generate archive in FILE.
 #   -u URI      Checkout from repository at URI.
 #   -c REV      Use revision REV.
@@ -20,6 +21,7 @@ verbose=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
     q)  verbose=-Q;;
+    C)  checkonly=1;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG#*://}";;
     c)  rev="${OPTARG}";;
@@ -57,6 +59,15 @@ if [[ ! "${uri}" =~ ^: ]]; then
 fi
 
 export TZ=UTC
+
+if [ -n "${checkonly}" ]; then
+    # Not all CVS servers support ls/rls, use login to see if we can connect.
+    # TODO this check only checks that the remote repository exists, not that
+    # it actually contains the requested revision.
+    _cvs ${verbose} -d"'${uri}'" login
+    exit ${?}
+fi
+
 _cvs ${verbose} -z3 -d"'${uri}'" \
      co "${@}" -d "'${basename}'" ${select} "'${rev}'" -P "'${rawname}'"
 
-- 
2.18.1

[Buildroot] [PATCH 11/11] support/download: implement source-check in svn backend

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/svn | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/support/download/svn b/support/download/svn
index 542b25c0a2..e6f41f24fc 100755
--- a/support/download/svn
+++ b/support/download/svn
@@ -7,6 +7,7 @@ set -e
 #
 # Options:
 #   -q          Be quiet.
+#   -C          Only check that the revision exists in the remote repository
 #   -o FILE     Generate archive in FILE.
 #   -u URI      Checkout from repository at URI.
 #   -c REV      Use revision REV.
@@ -19,6 +20,7 @@ verbose=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
     q)  verbose=-q;;
+    C)  checkonly=1;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG}";;
     c)  rev="${OPTARG}";;
@@ -36,6 +38,11 @@ _svn() {
     eval ${SVN} "${@}"
 }
 
+if [ -n "${checkonly}" ]; then
+    _svn ls ${verbose} "${@}" "'${uri}@${rev}'" > /dev/null
+    exit ${?}
+fi
+
 _svn export ${verbose} "${@}" "'${uri}@${rev}'" "'${basename}'"
 
 tar czf "${output}" "${basename}"
-- 
2.18.1

[Buildroot] [PATCH 00/11] support/download: fix scp and reintroduce source-check

[Thomas Petazzoni]

Hello,

On Thu,  3 Jan 2019 21:40:15 +0100, Thomas De Schampheleire wrote:

> Hello and happy 2019 to you all!
> 
> This series has two parts:
> 1. fix the broken scp download helper (seems no-one is using it on recent
>    releases?) (patches 1 and 2)

Indeed, I believe the scp download method is rarely used and tested. The
only solution to not break it again is to have proper tests for it in
support/testing/. Should it be part of your patch series ? :-)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 00/11] support/download: fix scp and reintroduce source-check

[Thomas De Schampheleire]

On Thu, Jan 3, 2019, 21:46 Thomas Petazzoni <thomas.petazzoni@bootlin.com
wrote:

> Hello,
>
> On Thu,  3 Jan 2019 21:40:15 +0100, Thomas De Schampheleire wrote:
>
> > Hello and happy 2019 to you all!
> >
> > This series has two parts:
> > 1. fix the broken scp download helper (seems no-one is using it on recent
> >    releases?) (patches 1 and 2)
>
> Indeed, I believe the scp download method is rarely used and tested. The
> only solution to not break it again is to have proper tests for it in
> support/testing/. Should it be part of your patch series ? :-)
>

But how to generically test it? It requires a configured ssh server, which
is not even guaranteed on localhost.

/Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190103/c9e4942c/attachment.html>

[Buildroot] [PATCH 00/11] support/download: fix scp and reintroduce source-check

[Thomas Petazzoni]

Hello,

+Ricardo in Cc.

On Thu, 3 Jan 2019 21:54:19 +0100, Thomas De Schampheleire wrote:

> > Indeed, I believe the scp download method is rarely used and tested. The
> > only solution to not break it again is to have proper tests for it in
> > support/testing/. Should it be part of your patch series ? :-)
> 
> But how to generically test it? It requires a configured ssh server, which
> is not even guaranteed on localhost.

I am not sure. A dummy SSH server ? Or do the build with a fake scp
command in the PATH, which verifies it has the expected arguments, and
emulates what scp would do ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 00/11] support/download: fix scp and reintroduce source-check

[Ricardo Martincoski]

Hello,

Sorry the long delay.

On Thu, Jan 03, 2019 at 06:57 PM, Thomas Petazzoni wrote:

> +Ricardo in Cc.
> 
> On Thu, 3 Jan 2019 21:54:19 +0100, Thomas De Schampheleire wrote:
> 
>> > Indeed, I believe the scp download method is rarely used and tested. The
>> > only solution to not break it again is to have proper tests for it in
>> > support/testing/. Should it be part of your patch series ? :-)
>> 
>> But how to generically test it? It requires a configured ssh server, which
>> is not even guaranteed on localhost.

But this can be guaranteed on the docker image.
This is the main use case for the runtime tests.
Of course I also like to be able to run the tests locally without the docker
image, but what we need to ensure is running inside the docker image. And we
could have a configured ssh server in the docker image.

> 
> I am not sure. A dummy SSH server ? Or do the build with a fake scp
> command in the PATH, which verifies it has the expected arguments, and
> emulates what scp would do ?

So more a unit test than a runtime test. I like the idea, but I am don't know
how difficult would be to implement this.
For me any test case that can catch regressions is welcome.
If we follow this way, perhaps this patch [1] can be an example of using the
same runtime infra for tests that are not runtime tests (do not run on the
target, I mean).
[1] http://patchwork.ozlabs.org/patch/992700/

One way to have a test case that runs in both local machine (without the docker
image) and in the docker image (either in the local machine or in the Gitlab CI)
is to have a rootfs image generated by Buildroot with a working server and use
that image as a downloaded artifact for the test case, and use a br2-external to
hold packages that use that server. See these commits [2][3] that I did not send
as patches yet, adding a image with a cvs server and using it to test the
download of packages with the cvs backend.

[2] https://gitlab.com/RicardoMartincoski/buildroot/commit/22e548dc4f49c225b658cb6e1119373bfae6b02b
[3] https://gitlab.com/RicardoMartincoski/buildroot/commit/064bd4ba344a107c5f56471800891af8fbb9ae2b


Regards,
Ricardo