[Buildroot] [PATCH 1/1] linux: Setting a repo_version is mandatory for custom repositories

[Buildroot] [PATCH 1/1] linux: Setting a repo_version is mandatory for custom repositories

[Christian Kellermann]

If not set the system will use an empty string which will result in
download errors for 'linux-.tar.gz' packages.

This patch makes it obvious to the user that the variable needs to be
set.

Signed-off-by: Christian Kellermann <christian.kellermann@solectrix.de>
---
 linux/linux.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/linux/linux.mk b/linux/linux.mk
index 7e826cc..f8a2cd9 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -35,6 +35,14 @@ LINUX_SOURCE = linux-$(LINUX_VERSION).tar.xz
 ifeq ($(BR2_LINUX_KERNEL_CUSTOM_VERSION),y)
 BR_NO_CHECK_HASH_FOR += $(LINUX_SOURCE)
 endif
+
+# When a custom repository has been set, check for the repository version
+ifeq ($(BR2_LINUX_KERNEL_CUSTOM_SVN)$(BR2_LINUX_KERNEL_CUSTOM_GIT)$(BR2_LINUX_KERNEL_CUSTOM_HG),y)
+ifeq ($(BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION),)
+$(error No custom repository version set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION setting)
+endif
+endif
+
 # In X.Y.Z, get X and Y. We replace dots and dashes by spaces in order
 # to use the $(word) function. We support versions such as 4.0, 3.1,
 # 2.6.32, 2.6.32-rc1, 3.0-rc6, etc.
-- 
2.1.4

[Buildroot] [PATCH 1/1] linux: Setting a repo_version is mandatory for custom repositories

[Arnout Vandecappelle]

On 06-12-16 09:13, Christian Kellermann wrote:
> If not set the system will use an empty string which will result in
> download errors for 'linux-.tar.gz' packages.
> 
> This patch makes it obvious to the user that the variable needs to be
> set.

 Good idea! However...

> 
> Signed-off-by: Christian Kellermann <christian.kellermann@solectrix.de>
> ---
>  linux/linux.mk | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 7e826cc..f8a2cd9 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -35,6 +35,14 @@ LINUX_SOURCE = linux-$(LINUX_VERSION).tar.xz
>  ifeq ($(BR2_LINUX_KERNEL_CUSTOM_VERSION),y)
>  BR_NO_CHECK_HASH_FOR += $(LINUX_SOURCE)
>  endif
> +
> +# When a custom repository has been set, check for the repository version
> +ifeq ($(BR2_LINUX_KERNEL_CUSTOM_SVN)$(BR2_LINUX_KERNEL_CUSTOM_GIT)$(BR2_LINUX_KERNEL_CUSTOM_HG),y)
> +ifeq ($(BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION),)

 Unless I'm very mistaken, this will never trigger, because the option will be
"" instead of empty when it has not been set by the user. So it should be
qstrip'ped.

 I think it is more convenient to check $(LINUX_VERSION).

 You can also drop the outer condition, so that it also checked for
BR2_LINUX_KERNEL_CUSTOM_VERSION. But then of course it should still be in an
ifeq ($(BR2_LINUX_KERNEL),y) condition. And you should check if the CUSTOM_LOCAL
and OVERRIDE_SRCDIR options still work correctly.

 And while you're at it, perhaps you can do the same for LINUX_SITE.


 Regards,
 Arnout

> +$(error No custom repository version set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION setting)
> +endif
> +endif
> +
>  # In X.Y.Z, get X and Y. We replace dots and dashes by spaces in order
>  # to use the $(word) function. We support versions such as 4.0, 3.1,
>  # 2.6.32, 2.6.32-rc1, 3.0-rc6, etc.
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/1] linux: Setting a repo_version is mandatory for custom repositories

[Christian Kellermann]

>   Unless I'm very mistaken, this will never trigger, because the
>  option will be "" instead of empty when it has not been set by the
>  user. So it should be qstrip'ped.

I have added the qstrip and also added the warning for the linux-header package.

>   I think it is more convenient to check $(LINUX_VERSION).

I don't understand what you mean by this.

>   You can also drop the outer condition, so that it also checked for
>  BR2_LINUX_KERNEL_CUSTOM_VERSION. But then of course it should still
>  be in an ifeq ($(BR2_LINUX_KERNEL),y) condition. And you should
>  check if the CUSTOM_LOCAL and OVERRIDE_SRCDIR options still work
>  correctly.

I don't think this will be easier to read though. But if you think
it's an improvement I will change the patches accordingly.

CUSTOM_LOCAL works for me, how do I test OVERRIDE_SRCDIR?

>   And while you're at it, perhaps you can do the same for LINUX_SITE.

Did this.

Thanks,

Christian

[Buildroot] [PATCH] linux: Setting a repo_version is mandatory for custom repositories

[Christian Kellermann]

If not set the system will use an empty string which will result in
download errors for 'linux-.tar.gz' packages.

This patch makes it obvious to the user that the variable needs to be
set.

Signed-off-by: Christian Kellermann <christian.kellermann@solectrix.de>
---
 linux/linux.mk                         | 12 ++++++++++++
 package/linux-headers/linux-headers.mk | 11 +++++++++++
 2 files changed, 23 insertions(+)

diff --git a/linux/linux.mk b/linux/linux.mk
index 7e826cc..2460ebd 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -35,6 +35,18 @@ LINUX_SOURCE = linux-$(LINUX_VERSION).tar.xz
 ifeq ($(BR2_LINUX_KERNEL_CUSTOM_VERSION),y)
 BR_NO_CHECK_HASH_FOR += $(LINUX_SOURCE)
 endif
+
+# When a custom repository has been set, check for the repository version
+ifeq ($(BR2_LINUX_KERNEL_CUSTOM_SVN)$(BR2_LINUX_KERNEL_CUSTOM_GIT)$(BR2_LINUX_KERNEL_CUSTOM_HG),y)
+ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION)),)
+$(error No custom repository version set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION setting)
+endif
+ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL)),)
+$(error No custom repo URL set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_URL setting)
+endif
+
+endif
+
 # In X.Y.Z, get X and Y. We replace dots and dashes by spaces in order
 # to use the $(word) function. We support versions such as 4.0, 3.1,
 # 2.6.32, 2.6.32-rc1, 3.0-rc6, etc.
diff --git a/package/linux-headers/linux-headers.mk b/package/linux-headers/linux-headers.mk
index 0900778..b79ae29 100644
--- a/package/linux-headers/linux-headers.mk
+++ b/package/linux-headers/linux-headers.mk
@@ -51,6 +51,17 @@ LINUX_HEADERS_SITE := $(LINUX_HEADERS_SITE)/testing
 endif # -rc
 endif
 
+# When a custom repository has been set, check for the repository version
+ifeq ($(BR2_LINUX_KERNEL_CUSTOM_SVN)$(BR2_LINUX_KERNEL_CUSTOM_GIT)$(BR2_LINUX_KERNEL_CUSTOM_HG),y)
+ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION)),)
+$(error No custom repository version set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION setting)
+endif
+ifeq ($(call qstrip,$(LINUX_HEADERS_SITE)),)
+$(error No header site URL set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_URL setting)
+endif
+endif
+
+
 LINUX_HEADERS_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
 
 # We rely on the generic package infrastructure to download and apply
-- 
2.1.4

[Buildroot] [PATCH] linux: Setting a repo_version is mandatory for custom repositories

[Thomas Petazzoni]

Hello,

On Mon, 12 Dec 2016 12:27:37 +0100, Christian Kellermann wrote:

> diff --git a/linux/linux.mk b/linux/linux.mk
> index 7e826cc..2460ebd 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -35,6 +35,18 @@ LINUX_SOURCE = linux-$(LINUX_VERSION).tar.xz
>  ifeq ($(BR2_LINUX_KERNEL_CUSTOM_VERSION),y)
>  BR_NO_CHECK_HASH_FOR += $(LINUX_SOURCE)
>  endif
> +
> +# When a custom repository has been set, check for the repository version
> +ifeq ($(BR2_LINUX_KERNEL_CUSTOM_SVN)$(BR2_LINUX_KERNEL_CUSTOM_GIT)$(BR2_LINUX_KERNEL_CUSTOM_HG),y)
> +ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION)),)
> +$(error No custom repository version set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION setting)
> +endif
> +ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL)),)
> +$(error No custom repo URL set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_URL setting)
> +endif
> +
> +endif

I moved this chunk closer to other checks done by linux.mk, towards the
bottom of the file.

> +# When a custom repository has been set, check for the repository version
> +ifeq ($(BR2_LINUX_KERNEL_CUSTOM_SVN)$(BR2_LINUX_KERNEL_CUSTOM_GIT)$(BR2_LINUX_KERNEL_CUSTOM_HG),y)
> +ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION)),)
> +$(error No custom repository version set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION setting)
> +endif
> +ifeq ($(call qstrip,$(LINUX_HEADERS_SITE)),)
> +$(error No header site URL set. Check your BR2_LINUX_KERNEL_CUSTOM_REPO_URL setting)
> +endif
> +endif

These checks were not needed, as they basically replicate the same
checks as the previous chunk, except for LINUX_HEADERS_SITE being
empty, but that's already checked by the common package infrastructure.

Applied with those modifications. Thanks!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com