[Buildroot] [PATCH 1/1] package: add support for <pkg>_SITE_ONLY

[Buildroot] [PATCH 1/1] package: add support for <pkg>_SITE_ONLY

[James Knight]

This will allow individual packages to hint that sources for the package
should only ever be downloaded from the specified `<pkg>_SITE` location.
Can be useful to avoid hitting configured cache sites when it is known
that a package would only ever be available from the package's site.

Signed-off-by: James Knight <git@jdknight.me>
---
 docs/manual/adding-packages-generic.adoc |  6 ++++++
 package/pkg-download.mk                  | 17 ++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/docs/manual/adding-packages-generic.adoc b/docs/manual/adding-packages-generic.adoc
index f0b0508f537d4a0329b48e2f8696ca43b098f1f2..35bd05e790520b92462e7acfb47472d44f5de792 100644
--- a/docs/manual/adding-packages-generic.adoc
+++ b/docs/manual/adding-packages-generic.adoc
@@ -336,6 +336,12 @@ not and can not work as people would expect it should:
      still patch the source code, use +LIBFOO_POST_RSYNC_HOOKS+, see
      xref:hooks-rsync[].
 
+* +LIBFOO_SITE_ONLY+ can be set to +YES+ to indicate that the package
+  should only be attempted to be downloaded from the configured site
+  defined by +LIBFOO_SITE+. Useful to avoid hitting configured cache
+  sites when it is known that the package would only ever be available
+  from the package's site.
+
 * +LIBFOO_GIT_SUBMODULES+ can be set to +YES+ to create an archive
   with the git submodules in the repository.  This is only available
   for packages downloaded with git (i.e. when
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index cf5959ea95549c896fd0837faf43f8e09f88b167..9b8ed92e9d3fb036aae6b84825cd2410337bf34d 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -59,6 +59,15 @@ gitlab = https://gitlab.com/$(1)/$(2)/-/archive/$(3)
 # Expressly do not check hashes for those files
 BR_NO_CHECK_HASH_FOR =
 
+################################################################################
+# DOWNLOAD_DEFAULT_URI - Default download site to get a package from.
+#
+# Argument 1 is the source location
+#
+################################################################################
+
+DOWNLOAD_DEFAULT_URI = $(patsubst %/,%,$(dir $(call qstrip,$(1))))
+
 ################################################################################
 # DOWNLOAD_URIS - List the candidates URIs where to get the package from:
 # 1) BR2_PRIMARY_SITE if enabled
@@ -77,8 +86,7 @@ DOWNLOAD_URIS += \
 endif
 
 ifeq ($(BR2_PRIMARY_SITE_ONLY),)
-DOWNLOAD_URIS += \
-	$(patsubst %/,%,$(dir $(call qstrip,$(1))))
+DOWNLOAD_URIS += $(DOWNLOAD_DEFAULT_URI)
 ifneq ($(call qstrip,$(BR2_BACKUP_SITE)),)
 DOWNLOAD_URIS += \
 	$(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)/$($(2)_DL_SUBDIR)),urlencode) \
@@ -129,7 +137,10 @@ define DOWNLOAD
 		$(if $(filter YES,$($(PKG)_SVN_EXTERNALS)),-r) \
 		$(if $($(PKG)_GIT_SUBMODULES),-r) \
 		$(if $($(PKG)_GIT_LFS),-l) \
-		$(foreach uri,$(call DOWNLOAD_URIS,$(1),$(PKG)),-u $(uri)) \
+		$(if $($(PKG)_SITE_ONLY), \
+			$(DOWNLOAD_DEFAULT_URI), \
+			$(foreach uri,$(call DOWNLOAD_URIS,$(1),$(PKG)),-u $(uri)) \
+		) \
 		$(2) \
 		$(QUIET) \
 		-- \
-- 
2.48.1.windows.1

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

Re: [Buildroot] [PATCH 1/1] package: add support for <pkg>_SITE_ONLY

[Arnout Vandecappelle via buildroot]

On 19/03/2025 05:39, James Knight wrote:
> This will allow individual packages to hint that sources for the package
> should only ever be downloaded from the specified `<pkg>_SITE` location.
> Can be useful to avoid hitting configured cache sites when it is known
> that a package would only ever be available from the package's site.

  Can you be a bit more specific about the use case? Is it intended for 
non-upstream packages for which you're sure they won't be in BR2_BACKUP_SITE? So 
it is just a small optimisation that only applies if you happen to have defined 
the wrong version or wrong hash (because otherwise BR2_BACKUP_SITE isn't even 
tried, so who cares)? Or are there other reasons?


  In addition to the comments I have below, I have a few more concerns.

- Since there are no in-tree users of this feature, there really should be a 
runtime test. It should also cover corner cases like BR2_PRIMARY_SITE_ONLY and 
_EXTRA_DOWNLOADS. And just to be sure, also a non-http download method.

- For all YES/NO options, we set a default in pkg-generic.mk. That way you see 
all values explicitly with `make printvars VARS=LIBFOO_%`.

- I guess we'll want the automatic host-from-target inheritance in 
pkg-generic.mk. Cfr. _AUTORECONF in pkg-autotools.mk.

-
> 
> Signed-off-by: James Knight <git@jdknight.me>
> ---
>   docs/manual/adding-packages-generic.adoc |  6 ++++++
>   package/pkg-download.mk                  | 17 ++++++++++++++---
>   2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/manual/adding-packages-generic.adoc b/docs/manual/adding-packages-generic.adoc
> index f0b0508f537d4a0329b48e2f8696ca43b098f1f2..35bd05e790520b92462e7acfb47472d44f5de792 100644
> --- a/docs/manual/adding-packages-generic.adoc
> +++ b/docs/manual/adding-packages-generic.adoc
> @@ -336,6 +336,12 @@ not and can not work as people would expect it should:
>        still patch the source code, use +LIBFOO_POST_RSYNC_HOOKS+, see
>        xref:hooks-rsync[].
>   
> +* +LIBFOO_SITE_ONLY+ can be set to +YES+ to indicate that the package
> +  should only be attempted to be downloaded from the configured site
> +  defined by +LIBFOO_SITE+. Useful to avoid hitting configured cache
> +  sites when it is known that the package would only ever be available
> +  from the package's site.

  This could be a bit more explicit, something like:

* +LIBFOO_SITE_ONLY+ can be set to +YES+ to indicate that the package
   should always be downloaded from the +LIBFOO_SITE+ and never use
   +BR2_PRIMARY_SITE+ or +BR2_BACKUP_SITE+. Even if +BR2_PRIMARY_SITE_ONLY+
   is enabled, +BR2_PRIMARY_SITE+ will not be used for this package. This
   option applies to all of the package's downloads, including those
   specified in +LIBFOO_PATCH+ and +LIBFOO_EXTRA_DOWNLOADS+.

> +
>   * +LIBFOO_GIT_SUBMODULES+ can be set to +YES+ to create an archive
>     with the git submodules in the repository.  This is only available
>     for packages downloaded with git (i.e. when
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index cf5959ea95549c896fd0837faf43f8e09f88b167..9b8ed92e9d3fb036aae6b84825cd2410337bf34d 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -59,6 +59,15 @@ gitlab = https://gitlab.com/$(1)/$(2)/-/archive/$(3)
>   # Expressly do not check hashes for those files
>   BR_NO_CHECK_HASH_FOR =
>   
> +################################################################################
> +# DOWNLOAD_DEFAULT_URI - Default download site to get a package from.
> +#
> +# Argument 1 is the source location
> +#
> +################################################################################
> +
> +DOWNLOAD_DEFAULT_URI = $(patsubst %/,%,$(dir $(call qstrip,$(1))))
> +
>   ################################################################################
>   # DOWNLOAD_URIS - List the candidates URIs where to get the package from:
>   # 1) BR2_PRIMARY_SITE if enabled
> @@ -77,8 +86,7 @@ DOWNLOAD_URIS += \
>   endif
>   
>   ifeq ($(BR2_PRIMARY_SITE_ONLY),)
> -DOWNLOAD_URIS += \
> -	$(patsubst %/,%,$(dir $(call qstrip,$(1))))
> +DOWNLOAD_URIS += $(DOWNLOAD_DEFAULT_URI)
>   ifneq ($(call qstrip,$(BR2_BACKUP_SITE)),)
>   DOWNLOAD_URIS += \
>   	$(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)/$($(2)_DL_SUBDIR)),urlencode) \
> @@ -129,7 +137,10 @@ define DOWNLOAD
>   		$(if $(filter YES,$($(PKG)_SVN_EXTERNALS)),-r) \
>   		$(if $($(PKG)_GIT_SUBMODULES),-r) \
>   		$(if $($(PKG)_GIT_LFS),-l) \
> -		$(foreach uri,$(call DOWNLOAD_URIS,$(1),$(PKG)),-u $(uri)) \
> +		$(if $($(PKG)_SITE_ONLY), \
> +			$(DOWNLOAD_DEFAULT_URI), \

  Does this work? The -u option seems to be missing...


  I marked this patch as Changes Requested.

  Regards,
  Arnout


> +			$(foreach uri,$(call DOWNLOAD_URIS,$(1),$(PKG)),-u $(uri)) \
> +		) \
>   		$(2) \
>   		$(QUIET) \
>   		-- \

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

Re: [Buildroot] [PATCH 1/1] package: add support for <pkg>_SITE_ONLY

[Arnout Vandecappelle via buildroot]

On 25/03/2025 21:15, Arnout Vandecappelle wrote:
>  I marked this patch as Changes Requested. 


  Actually, it was already marked as Deferred... Did you do that?


  Regards,
  Arnout


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

Re: [Buildroot] [PATCH 1/1] package: add support for <pkg>_SITE_ONLY

[James Knight]

Arnout,

> On Tue, Mar 25, 2025 at 4:19 PM Arnout Vandecappelle wrote:
> [It] was already marked as Deferred... Did you do that?

First off, sorry for the late reply to all this.

Yes, I flagged it as deferred as the more testing performed at the
time revealed issues with the implementation. I am still planning to
revisit this again in the future when time permits (taking into
consideration the points you have previously made).

> On Tue, Mar 25, 2025 at 4:15 PM Arnout Vandecappelle wrote:
> Can you be a bit more specific about the use case? Is it intended for
non-upstream packages for which you're sure they won't be in BR2_BACKUP_SITE?

Yes.

For the Buildroot configurations I use, I typically set up the
`BR2_PRIMARY_SITE` configuration to point to a local cache node of
artifacts to improve fetch duration (and as a bonus to avoid wasting
bandwidth). However, if the cache does not have a copy of an artifact
(e.g. an updated Buildroot revision), the preference is to fall back
onto the package-defined site and then fallback onto `BR2_BACKUP_SITE`
(which is current left configured as the stock Buildroot site mirror).
At the end of a build, any newly downloaded artifacts are stored on
the local cache.

This process works great for most packages. However, there are some
cases where there can be packages defined under a br2-external path
that will never be stored on the local cache (if it's already being
stored on another local instance) and will most likely never be hosted
on Buildroot's cache mirror. So, it would be nice if there was a way
for a package to define a means to say "only ever download from the
configured site since it will never be on a primary override (cache)
or external mirrors".

> I have a few more concerns...

I agree with all the comments you made. If I recall correctly, the use
of the `_EXTRA_DOWNLOADS` feature was something I overlooked that was
not handled. If I can submit another patch by EOY, I will make
appropriate changes.

However,

> [We'll] want the automatic host-from-target inheritance in pkg-generic.mk. Cfr. _AUTORECONF in pkg-autotools.mk.

I am not sure I understand this point, but maybe I'll figure it out
the more I look into specifics of the code.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot