diff mbox series

[v2,01/13] pkg-{download, generic}: remove source-check

Message ID 20171025201003.16105-2-ps.report@gmx.net
State Accepted
Headers show
Series New DL_DIR organisation; git cache feature | expand

Commit Message

Peter Seiderer Oct. 25, 2017, 8:09 p.m. UTC
From: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>

This feature is not used by anyone in the core developpers and makes a
drastic simplification of the pkg-download infrastructure harder.

The future patch will move much of what's in the current pkg-download.mk
file into the dl-wrapper which is a shell script.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
Changes v1 --> v2:
 - from https://github.com/maximeh/buildroot/commit/a4d0e83729a1ab534ff99d6f258058a1be1ed949.patch
 - remove more references to source-check (suggested by Thomas Petazzoni)
---
 Makefile                |  7 +------
 package/pkg-download.mk | 44 --------------------------------------------
 package/pkg-generic.mk  | 14 +-------------
 3 files changed, 2 insertions(+), 63 deletions(-)

Comments

Yann E. MORIN Jan. 8, 2018, 8:22 p.m. UTC | #1
Peter, Maxime, All,

On 2017-10-25 22:09 +0200, Peter Seiderer spake thusly:
> From: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> 
> This feature is not used by anyone in the core developpers and makes a
> drastic simplification of the pkg-download infrastructure harder.
> 
> The future patch will move much of what's in the current pkg-download.mk
> file into the dl-wrapper which is a shell script.
> 
> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>

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

Regards,
Yann E. MORIN.

> ---
> Changes v1 --> v2:
>  - from https://github.com/maximeh/buildroot/commit/a4d0e83729a1ab534ff99d6f258058a1be1ed949.patch
>  - remove more references to source-check (suggested by Thomas Petazzoni)
> ---
>  Makefile                |  7 +------
>  package/pkg-download.mk | 44 --------------------------------------------
>  package/pkg-generic.mk  | 14 +-------------
>  3 files changed, 2 insertions(+), 63 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 79db7fe48a..45047f7e9a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -141,7 +141,7 @@ noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconf
>  # We're building in two situations: when MAKECMDGOALS is empty
>  # (default target is to build), or when MAKECMDGOALS contains
>  # something else than one of the nobuild_targets.
> -nobuild_targets := source %-source source-check \
> +nobuild_targets := source %-source \
>  	legal-info %-legal-info external-deps _external-deps \
>  	clean distclean help show-targets graph-depends \
>  	%-graph-depends %-show-depends %-show-version \
> @@ -754,10 +754,6 @@ _external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps)
>  external-deps:
>  	@$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u
>  
> -# check if download URLs are outdated
> -.PHONY: source-check
> -source-check: $(foreach p,$(PACKAGES),$(p)-all-source-check)
> -
>  .PHONY: legal-info-clean
>  legal-info-clean:
>  	@rm -fr $(LEGAL_INFO_DIR)
> @@ -1044,7 +1040,6 @@ 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 dc4ff1c8c7..2b845e77a2 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -68,9 +68,6 @@ export BR_NO_CHECK_HASH_FOR =
>  # ssh authentication. DOWNLOAD_WGET is the normal wget-based download
>  # mechanism.
>  #
> -# The SOURCE_CHECK_* helpers are in charge of simply checking that the source
> -# is available for download. This can be used to make sure one will be able
> -# to get all the sources needed for one's build configuration.
>  ################################################################################
>  
>  define DOWNLOAD_GIT
> @@ -86,12 +83,6 @@ define DOWNLOAD_GIT
>  		$($(PKG)_DL_OPTS)
>  endef
>  
> -# TODO: improve to check that the given PKG_DL_VERSION exists on the remote
> -# repository
> -define SOURCE_CHECK_GIT
> -	$(GIT) ls-remote --heads $($(PKG)_SITE) > /dev/null
> -endef
> -
>  define DOWNLOAD_BZR
>  	$(EXTRA_ENV) $(DL_WRAPPER) -b bzr \
>  		-o $(DL_DIR)/$($(PKG)_SOURCE) \
> @@ -103,10 +94,6 @@ define DOWNLOAD_BZR
>  		$($(PKG)_DL_OPTS)
>  endef
>  
> -define SOURCE_CHECK_BZR
> -	$(BZR) ls --quiet $($(PKG)_SITE) > /dev/null
> -endef
> -
>  define DOWNLOAD_CVS
>  	$(EXTRA_ENV) $(DL_WRAPPER) -b cvs \
>  		-o $(DL_DIR)/$($(PKG)_SOURCE) \
> @@ -119,11 +106,6 @@ define DOWNLOAD_CVS
>  		$($(PKG)_DL_OPTS)
>  endef
>  
> -# Not all CVS servers support ls/rls, use login to see if we can connect
> -define SOURCE_CHECK_CVS
> -	$(CVS) -d:pserver:anonymous:@$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) login
> -endef
> -
>  define DOWNLOAD_SVN
>  	$(EXTRA_ENV) $(DL_WRAPPER) -b svn \
>  		-o $(DL_DIR)/$($(PKG)_SOURCE) \
> @@ -135,10 +117,6 @@ define DOWNLOAD_SVN
>  		$($(PKG)_DL_OPTS)
>  endef
>  
> -define SOURCE_CHECK_SVN
> -	$(SVN) ls $($(PKG)_SITE)@$($(PKG)_DL_VERSION) > /dev/null
> -endef
> -
>  # SCP URIs should be of the form scp://[user@]host:filepath
>  # Note that filepath is relative to the user's home directory, so you may want
>  # to prepend the path with a slash: scp://[user@]host:/absolutepath
> @@ -152,10 +130,6 @@ define DOWNLOAD_SCP
>  		$($(PKG)_DL_OPTS)
>  endef
>  
> -define SOURCE_CHECK_SCP
> -	$(SSH) $(call domain,$(1),:) ls '$(call notdomain,$(1),:)' > /dev/null
> -endef
> -
>  define DOWNLOAD_HG
>  	$(EXTRA_ENV) $(DL_WRAPPER) -b hg \
>  		-o $(DL_DIR)/$($(PKG)_SOURCE) \
> @@ -167,12 +141,6 @@ define DOWNLOAD_HG
>  		$($(PKG)_DL_OPTS)
>  endef
>  
> -# TODO: improve to check that the given PKG_DL_VERSION exists on the remote
> -# repository
> -define SOURCE_CHECK_HG
> -	$(HG) incoming --force -l1 $($(PKG)_SITE) > /dev/null
> -endef
> -
>  define DOWNLOAD_WGET
>  	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
>  		-o $(DL_DIR)/$(2) \
> @@ -183,10 +151,6 @@ define DOWNLOAD_WGET
>  		$($(PKG)_DL_OPTS)
>  endef
>  
> -define SOURCE_CHECK_WGET
> -	$(WGET) --spider '$(call qstrip,$(1))'
> -endef
> -
>  define DOWNLOAD_LOCALFILES
>  	$(EXTRA_ENV) $(DL_WRAPPER) -b cp \
>  		-o $(DL_DIR)/$(2) \
> @@ -197,10 +161,6 @@ define DOWNLOAD_LOCALFILES
>  		$($(PKG)_DL_OPTS)
>  endef
>  
> -define SOURCE_CHECK_LOCALFILES
> -	test -e $(call stripurischeme,$(call qstrip,$(1)))
> -endef
> -
>  ################################################################################
>  # DOWNLOAD -- Download helper. Will try to download source from:
>  # 1) BR2_PRIMARY_SITE if enabled
> @@ -221,10 +181,6 @@ define DOWNLOAD
>  	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),DOWNLOAD)
>  endef
>  
> -define SOURCE_CHECK
> -	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),SOURCE_CHECK)
> -endef
> -
>  define DOWNLOAD_INNER
>  	$(Q)$(if $(filter bzr cvs hg svn,$($(PKG)_SITE_METHOD)),export BR_NO_CHECK_HASH_FOR=$(2);) \
>  	if test -n "$(call qstrip,$(BR2_PRIMARY_SITE))" ; then \
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index cca94ba338..5989417902 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -706,10 +706,6 @@ $(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` ; \
> @@ -734,9 +730,6 @@ $(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
> @@ -762,9 +755,6 @@ $(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)
>  
> @@ -964,7 +954,6 @@ 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 \
> @@ -989,8 +978,7 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>  	$(1)-rsync \
>  	$(1)-show-depends \
>  	$(1)-show-version \
> -	$(1)-source \
> -	$(1)-source-check
> +	$(1)-source
>  
>  ifneq ($$($(2)_SOURCE),)
>  ifeq ($$($(2)_SITE),)
> -- 
> 2.14.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni Jan. 8, 2018, 8:28 p.m. UTC | #2
Hello,

On Wed, 25 Oct 2017 22:09:51 +0200, Peter Seiderer wrote:
> From: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> 
> This feature is not used by anyone in the core developpers and makes a
> drastic simplification of the pkg-download infrastructure harder.
> 
> The future patch will move much of what's in the current pkg-download.mk
> file into the dl-wrapper which is a shell script.
> 
> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> ---
> Changes v1 --> v2:
>  - from https://github.com/maximeh/buildroot/commit/a4d0e83729a1ab534ff99d6f258058a1be1ed949.patch
>  - remove more references to source-check (suggested by Thomas Petazzoni)
> ---
>  Makefile                |  7 +------
>  package/pkg-download.mk | 44 --------------------------------------------
>  package/pkg-generic.mk  | 14 +-------------
>  3 files changed, 2 insertions(+), 63 deletions(-)

Applied to master, thanks.

Thomas
Thomas De Schampheleire Jan. 10, 2018, 12:04 p.m. UTC | #3
Hi all,


2018-01-08 21:28 GMT+01:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Hello,
>
> On Wed, 25 Oct 2017 22:09:51 +0200, Peter Seiderer wrote:
>> From: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
>>
>> This feature is not used by anyone in the core developpers and makes a
>> drastic simplification of the pkg-download infrastructure harder.
>>
>> The future patch will move much of what's in the current pkg-download.mk
>> file into the dl-wrapper which is a shell script.
>>
>> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
>> ---
>> Changes v1 --> v2:
>>  - from https://github.com/maximeh/buildroot/commit/a4d0e83729a1ab534ff99d6f258058a1be1ed949.patch
>>  - remove more references to source-check (suggested by Thomas Petazzoni)
>> ---
>>  Makefile                |  7 +------
>>  package/pkg-download.mk | 44 --------------------------------------------
>>  package/pkg-generic.mk  | 14 +-------------
>>  3 files changed, 2 insertions(+), 63 deletions(-)
>
> Applied to master, thanks.
>

I saw this on the agenda of the FOSDEM developer days, so thought it
would be discussed there before going forward with applying this. I
should have expressed my concerns on the patch immediately, sorry.

In fact, just recently I did start using 'make source-check' in our
environment. There are two use cases:
1. Our Buildroot tree contains a few packages with in-house source
code. They are typically Mercurial repositories. A developer changing
code in such a repository is expected to push to that repository and
then update the revision hash in the Buildroot .mk file (or if it
concerns a Linux/Uboot tree in the defconfig file).
Now, it sometimes happens that people update the hash but forget to
push the corresponding changes to the package's repository. This is of
course human error. In our delivery system (where people don't push
directly to the main repo but rather to some staging area where
changes are first automatically validated), it would be caught by
making a build, but this takes quite some time, order of 1 hour (we
have many defconfigs). Hence, we added an initial check to our
delivery system that just performs 'make source-check' for all
defconfigs. This only takes a few minutes. Deliveries that fail the
check are thus rejected very quickly and no computing resources are
lost.

2. Besides the in-house repositories, there is a use case even for
normal open-source packages. We have an internal BR2_PRIMARY_SITE set
up, that avoids package downloads over the internet. As we need to
guarantee reproducibility over a long time span, we cannot rely on
external sources and we need to mirror all packages used to that
primary site. Developers that enable new packages (or bump one) are
expected to upload the new sources to the primary site, but again may
forget to do so.
Again, this would be caught by the build step that takes ~1 hour,
while the source-check target would detect it in minutes.

Doing a 'make source' rather than 'make source-check' is an
alternative, but it takes longer and consumes much more network
bandwidth.

I would therefore like to find a way to reintroduce 'make
source-check', but of course taking into account the reasons of
removing it in the first place. Perhaps another implementation can be
found using the dl-wrapper.

Thoughts?

Thanks,
Thomas
Thomas Petazzoni Jan. 10, 2018, 12:41 p.m. UTC | #4
Hello,

On Wed, 10 Jan 2018 13:04:03 +0100, Thomas De Schampheleire wrote:

> I saw this on the agenda of the FOSDEM developer days, so thought it
> would be discussed there before going forward with applying this. I
> should have expressed my concerns on the patch immediately, sorry.

No problem. It's never too late to revert something, or re-introduce it
in a different form.

However, my plan is in fact to get the Git download caching stuff
merged for 2018.02, hence before the meeting at FOSDEM. I've recently
worked on a few projects were the kernel is hosted in Git and regularly
changed, and the full git clone every time you update the kernel is
simply horrible, and makes Buildroot pretty much unusable.

> In fact, just recently I did start using 'make source-check' in our
> environment. There are two use cases:
> 1. Our Buildroot tree contains a few packages with in-house source
> code. They are typically Mercurial repositories. A developer changing
> code in such a repository is expected to push to that repository and
> then update the revision hash in the Buildroot .mk file (or if it
> concerns a Linux/Uboot tree in the defconfig file).
> Now, it sometimes happens that people update the hash but forget to
> push the corresponding changes to the package's repository. This is of
> course human error. In our delivery system (where people don't push
> directly to the main repo but rather to some staging area where
> changes are first automatically validated), it would be caught by
> making a build, but this takes quite some time, order of 1 hour (we
> have many defconfigs). Hence, we added an initial check to our
> delivery system that just performs 'make source-check' for all
> defconfigs. This only takes a few minutes. Deliveries that fail the
> check are thus rejected very quickly and no computing resources are
> lost.
> 
> 2. Besides the in-house repositories, there is a use case even for
> normal open-source packages. We have an internal BR2_PRIMARY_SITE set
> up, that avoids package downloads over the internet. As we need to
> guarantee reproducibility over a long time span, we cannot rely on
> external sources and we need to mirror all packages used to that
> primary site. Developers that enable new packages (or bump one) are
> expected to upload the new sources to the primary site, but again may
> forget to do so.
> Again, this would be caught by the build step that takes ~1 hour,
> while the source-check target would detect it in minutes.
> 
> Doing a 'make source' rather than 'make source-check' is an
> alternative, but it takes longer and consumes much more network
> bandwidth.
> 
> I would therefore like to find a way to reintroduce 'make
> source-check', but of course taking into account the reasons of
> removing it in the first place. Perhaps another implementation can be
> found using the dl-wrapper.

We can definitely re-introduce a "make source-check" implementation. It
was mainly dropped because we had no identification anyone was really
using it.

Perhaps we could for now have "make source-check" implemented by doing
"make source", and later on re-optimize it?

As I said, I'd really to have this Git caching stuff merged in 2018.02.
The source-check removal has been on the mailing list since July 2017,
and nobody complained, until today. So while I do acknowledge that
there are some use cases for source-check, I'm not really willing to
delay merging the Git caching stuff just because source-check will be
missing. But maybe re-adding source-check is not that hard, and can be
done before 2018.02 as well.

Best regards,

Thomas Petazzoni
Thomas De Schampheleire Jan. 10, 2018, 12:51 p.m. UTC | #5
Hi Thomas,

Thanks for your feedback...

2018-01-10 13:41 GMT+01:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Hello,
>
> On Wed, 10 Jan 2018 13:04:03 +0100, Thomas De Schampheleire wrote:
>
>> I saw this on the agenda of the FOSDEM developer days, so thought it
>> would be discussed there before going forward with applying this. I
>> should have expressed my concerns on the patch immediately, sorry.
>
> No problem. It's never too late to revert something, or re-introduce it
> in a different form.
>
> However, my plan is in fact to get the Git download caching stuff
> merged for 2018.02, hence before the meeting at FOSDEM. I've recently
> worked on a few projects were the kernel is hosted in Git and regularly
> changed, and the full git clone every time you update the kernel is
> simply horrible, and makes Buildroot pretty much unusable.
>
>> In fact, just recently I did start using 'make source-check' in our
>> environment. There are two use cases:
>> 1. Our Buildroot tree contains a few packages with in-house source
>> code. They are typically Mercurial repositories. A developer changing
>> code in such a repository is expected to push to that repository and
>> then update the revision hash in the Buildroot .mk file (or if it
>> concerns a Linux/Uboot tree in the defconfig file).
>> Now, it sometimes happens that people update the hash but forget to
>> push the corresponding changes to the package's repository. This is of
>> course human error. In our delivery system (where people don't push
>> directly to the main repo but rather to some staging area where
>> changes are first automatically validated), it would be caught by
>> making a build, but this takes quite some time, order of 1 hour (we
>> have many defconfigs). Hence, we added an initial check to our
>> delivery system that just performs 'make source-check' for all
>> defconfigs. This only takes a few minutes. Deliveries that fail the
>> check are thus rejected very quickly and no computing resources are
>> lost.
>>
>> 2. Besides the in-house repositories, there is a use case even for
>> normal open-source packages. We have an internal BR2_PRIMARY_SITE set
>> up, that avoids package downloads over the internet. As we need to
>> guarantee reproducibility over a long time span, we cannot rely on
>> external sources and we need to mirror all packages used to that
>> primary site. Developers that enable new packages (or bump one) are
>> expected to upload the new sources to the primary site, but again may
>> forget to do so.
>> Again, this would be caught by the build step that takes ~1 hour,
>> while the source-check target would detect it in minutes.
>>
>> Doing a 'make source' rather than 'make source-check' is an
>> alternative, but it takes longer and consumes much more network
>> bandwidth.
>>
>> I would therefore like to find a way to reintroduce 'make
>> source-check', but of course taking into account the reasons of
>> removing it in the first place. Perhaps another implementation can be
>> found using the dl-wrapper.
>
> We can definitely re-introduce a "make source-check" implementation. It
> was mainly dropped because we had no identification anyone was really
> using it.
>
> Perhaps we could for now have "make source-check" implemented by doing
> "make source", and later on re-optimize it?
>
> As I said, I'd really to have this Git caching stuff merged in 2018.02.
> The source-check removal has been on the mailing list since July 2017,
> and nobody complained, until today. So while I do acknowledge that
> there are some use cases for source-check, I'm not really willing to
> delay merging the Git caching stuff just because source-check will be
> missing. But maybe re-adding source-check is not that hard, and can be
> done before 2018.02 as well.

By no means do I want to delay other patches like git caching because of this.

I can definitely live with a temporary local patch on our tree, but of
course would like to find a method that can be upstreamed.

/Thomas
Yann E. MORIN Jan. 10, 2018, 6:18 p.m. UTC | #6
ThomasĀ², All,

On 2018-01-10 13:04 +0100, Thomas De Schampheleire spake thusly:
> 2018-01-08 21:28 GMT+01:00 Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com>:
> >> ---
> >>  Makefile                |  7 +------
> >>  package/pkg-download.mk | 44 --------------------------------------------
> >>  package/pkg-generic.mk  | 14 +-------------
> >>  3 files changed, 2 insertions(+), 63 deletions(-)
> >
> > Applied to master, thanks.
> 
> I saw this on the agenda of the FOSDEM developer days, so thought it
> would be discussed there before going forward with applying this. I
> should have expressed my concerns on the patch immediately, sorry.
> 
> In fact, just recently I did start using 'make source-check' in our
> environment. There are two use cases:
> 1. Our Buildroot tree contains a few packages with in-house source
> code. They are typically Mercurial repositories. A developer changing
> code in such a repository is expected to push to that repository and
> then update the revision hash in the Buildroot .mk file (or if it
> concerns a Linux/Uboot tree in the defconfig file).
> Now, it sometimes happens that people update the hash but forget to
> push the corresponding changes to the package's repository. This is of
> course human error. In our delivery system (where people don't push
> directly to the main repo but rather to some staging area where
> changes are first automatically validated), it would be caught by
> making a build, but this takes quite some time, order of 1 hour (we
> have many defconfigs). Hence, we added an initial check to our
> delivery system that just performs 'make source-check' for all
> defconfigs. This only takes a few minutes. Deliveries that fail the
> check are thus rejected very quickly and no computing resources are
> lost.
> 
> 2. Besides the in-house repositories, there is a use case even for
> normal open-source packages. We have an internal BR2_PRIMARY_SITE set
> up, that avoids package downloads over the internet. As we need to
> guarantee reproducibility over a long time span, we cannot rely on
> external sources and we need to mirror all packages used to that
> primary site. Developers that enable new packages (or bump one) are
> expected to upload the new sources to the primary site, but again may
> forget to do so.
> Again, this would be caught by the build step that takes ~1 hour,
> while the source-check target would detect it in minutes.
> 
> Doing a 'make source' rather than 'make source-check' is an
> alternative, but it takes longer and consumes much more network
> bandwidth.
> 
> I would therefore like to find a way to reintroduce 'make
> source-check', but of course taking into account the reasons of
> removing it in the first place. Perhaps another implementation can be
> found using the dl-wrapper.
> 
> Thoughts?

The way I haved solved this issue (and is something I forgot to talk
about during my talk at ELCE) without source-check is that the automatic
build system (jenkins and gitlab-ci) have a job that does basically:

    for t in ${targets}; do
        make ${t}_defconfig
        sed -r -e 's:^BR2_WGET="(.+)"$:BR2_WGET="/path/to/dl-wrapper \1":'
        make source
    done

(ditto for svn, git, hg, scp...)

Then the /path/to/dl-wrapper basically does:

    #!/bin/bash
    "${@}"  # Do the actual download
    if [[ ${url} =~ ^http://internal-server/mirror/ ]]; then
        : ; # Already on server
    elif [[ ${url} =~ ^http://internal-server/repositories/ ]]; then
        # New revision, push to filer
        scp "${url##*/}" "user@internal-server:path/to/mirror/"
    elif [[ ${url} =~ ^http://backup-server/mirror/ ]]; then
        # From backup, ARG...
        send-email "ARG: mot on mirror, nor upstream, from backup ${pkg}"
        scp "${url##*/}" "user@internal-server:path/to/mirror/"
    else
        # From upstream
        send-email "WARN: new package from upstream: ${pkg}"
        scp "${url##*/}" "user@internal-server:path/to/mirror/"
    fi

Of course, this is extermely simplified, but you get the idea...

Then the real build jobs only have access to the internal mirror, and
never to upstream, whether it is internal or exgternal. So we can also
fill a directory for delivery and archiving when needed.

Even though we still have a Buildroot with source-check, we are not
using it because it is more powerfull to just wrap around the real
download.

Regards,
Yann E. MORIN.

> Thanks,
> Thomas
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 79db7fe48a..45047f7e9a 100644
--- a/Makefile
+++ b/Makefile
@@ -141,7 +141,7 @@  noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconf
 # We're building in two situations: when MAKECMDGOALS is empty
 # (default target is to build), or when MAKECMDGOALS contains
 # something else than one of the nobuild_targets.
-nobuild_targets := source %-source source-check \
+nobuild_targets := source %-source \
 	legal-info %-legal-info external-deps _external-deps \
 	clean distclean help show-targets graph-depends \
 	%-graph-depends %-show-depends %-show-version \
@@ -754,10 +754,6 @@  _external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps)
 external-deps:
 	@$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u
 
-# check if download URLs are outdated
-.PHONY: source-check
-source-check: $(foreach p,$(PACKAGES),$(p)-all-source-check)
-
 .PHONY: legal-info-clean
 legal-info-clean:
 	@rm -fr $(LEGAL_INFO_DIR)
@@ -1044,7 +1040,6 @@  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 dc4ff1c8c7..2b845e77a2 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -68,9 +68,6 @@  export BR_NO_CHECK_HASH_FOR =
 # ssh authentication. DOWNLOAD_WGET is the normal wget-based download
 # mechanism.
 #
-# The SOURCE_CHECK_* helpers are in charge of simply checking that the source
-# is available for download. This can be used to make sure one will be able
-# to get all the sources needed for one's build configuration.
 ################################################################################
 
 define DOWNLOAD_GIT
@@ -86,12 +83,6 @@  define DOWNLOAD_GIT
 		$($(PKG)_DL_OPTS)
 endef
 
-# TODO: improve to check that the given PKG_DL_VERSION exists on the remote
-# repository
-define SOURCE_CHECK_GIT
-	$(GIT) ls-remote --heads $($(PKG)_SITE) > /dev/null
-endef
-
 define DOWNLOAD_BZR
 	$(EXTRA_ENV) $(DL_WRAPPER) -b bzr \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
@@ -103,10 +94,6 @@  define DOWNLOAD_BZR
 		$($(PKG)_DL_OPTS)
 endef
 
-define SOURCE_CHECK_BZR
-	$(BZR) ls --quiet $($(PKG)_SITE) > /dev/null
-endef
-
 define DOWNLOAD_CVS
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cvs \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
@@ -119,11 +106,6 @@  define DOWNLOAD_CVS
 		$($(PKG)_DL_OPTS)
 endef
 
-# Not all CVS servers support ls/rls, use login to see if we can connect
-define SOURCE_CHECK_CVS
-	$(CVS) -d:pserver:anonymous:@$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) login
-endef
-
 define DOWNLOAD_SVN
 	$(EXTRA_ENV) $(DL_WRAPPER) -b svn \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
@@ -135,10 +117,6 @@  define DOWNLOAD_SVN
 		$($(PKG)_DL_OPTS)
 endef
 
-define SOURCE_CHECK_SVN
-	$(SVN) ls $($(PKG)_SITE)@$($(PKG)_DL_VERSION) > /dev/null
-endef
-
 # SCP URIs should be of the form scp://[user@]host:filepath
 # Note that filepath is relative to the user's home directory, so you may want
 # to prepend the path with a slash: scp://[user@]host:/absolutepath
@@ -152,10 +130,6 @@  define DOWNLOAD_SCP
 		$($(PKG)_DL_OPTS)
 endef
 
-define SOURCE_CHECK_SCP
-	$(SSH) $(call domain,$(1),:) ls '$(call notdomain,$(1),:)' > /dev/null
-endef
-
 define DOWNLOAD_HG
 	$(EXTRA_ENV) $(DL_WRAPPER) -b hg \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
@@ -167,12 +141,6 @@  define DOWNLOAD_HG
 		$($(PKG)_DL_OPTS)
 endef
 
-# TODO: improve to check that the given PKG_DL_VERSION exists on the remote
-# repository
-define SOURCE_CHECK_HG
-	$(HG) incoming --force -l1 $($(PKG)_SITE) > /dev/null
-endef
-
 define DOWNLOAD_WGET
 	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
 		-o $(DL_DIR)/$(2) \
@@ -183,10 +151,6 @@  define DOWNLOAD_WGET
 		$($(PKG)_DL_OPTS)
 endef
 
-define SOURCE_CHECK_WGET
-	$(WGET) --spider '$(call qstrip,$(1))'
-endef
-
 define DOWNLOAD_LOCALFILES
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cp \
 		-o $(DL_DIR)/$(2) \
@@ -197,10 +161,6 @@  define DOWNLOAD_LOCALFILES
 		$($(PKG)_DL_OPTS)
 endef
 
-define SOURCE_CHECK_LOCALFILES
-	test -e $(call stripurischeme,$(call qstrip,$(1)))
-endef
-
 ################################################################################
 # DOWNLOAD -- Download helper. Will try to download source from:
 # 1) BR2_PRIMARY_SITE if enabled
@@ -221,10 +181,6 @@  define DOWNLOAD
 	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),DOWNLOAD)
 endef
 
-define SOURCE_CHECK
-	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),SOURCE_CHECK)
-endef
-
 define DOWNLOAD_INNER
 	$(Q)$(if $(filter bzr cvs hg svn,$($(PKG)_SITE_METHOD)),export BR_NO_CHECK_HASH_FOR=$(2);) \
 	if test -n "$(call qstrip,$(BR2_PRIMARY_SITE))" ; then \
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index cca94ba338..5989417902 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -706,10 +706,6 @@  $(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` ; \
@@ -734,9 +730,6 @@  $(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
@@ -762,9 +755,6 @@  $(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)
 
@@ -964,7 +954,6 @@  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 \
@@ -989,8 +978,7 @@  DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
 	$(1)-rsync \
 	$(1)-show-depends \
 	$(1)-show-version \
-	$(1)-source \
-	$(1)-source-check
+	$(1)-source
 
 ifneq ($$($(2)_SOURCE),)
 ifeq ($$($(2)_SITE),)