Skip to contentSkip to conversation

linux: Setting a repo_version is mandatory for custom repositories

PATCH New Christian Kellermann 3 participants 5 messages Dec 6, 2016 – Dec 17, 2016

lore
Share & tools
lore

Christian Kellermann

author
#0 lore raw

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

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 <[email protected]>
---
 linux/linux.mk | 8 ++++++++
 1 file changed, 8 insertions(+)
linux/linux.mk +8 -0
index 7e826cc..f8a2cd9 100644
@@ -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.
Signature · 1 line
2.1.4
#1 lore raw

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

Quoted 5 lines · 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.

Good idea! However...

Quoted 11 lines
Signed-off-by Christian Kellermann <[email protected]>
---
 linux/linux.mk | 8 ++++++++
 1 file changed, 8 insertions(+)
linux/linux.mk +0 -0
index 7e826cc..f8a2cd9 100644

linux/linux.mk LINUX_SOURCE = linux-$(LINUX_VERSION).tar.xz · line 41

Quoted 7 lines
linux/linux.mk +4 -0
@@ -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

Quoted 8 lines
linux/linux.mk +4 -0
+$(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.
Signature · 6 lines
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
author
#2 lore raw

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

Quoted 5 lines

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.

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

I don't understand what you mean by this.

Quoted 5 lines

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?

Quoted 1 line

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

Did this.

Thanks,

Christian

author
#3 lore raw

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

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 <[email protected]>
---
 linux/linux.mk                         | 12 ++++++++++++
 package/linux-headers/linux-headers.mk | 11 +++++++++++
 2 files changed, 23 insertions(+)
linux/linux.mk +12 -0
index 7e826cc..2460ebd 100644
@@ -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.
package/linux-headers/linux-headers.mk +11 -0
index 0900778..b79ae29 100644
@@ -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
Signature · 1 line
2.1.4
#4 lore raw

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

Hello,

Quoted 9 lines · Christian Kellermann
linux/linux.mk +1 -0
index 7e826cc..2460ebd 100644
@@ -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
+

linux/linux.mk LINUX_SOURCE = linux-$(LINUX_VERSION).tar.xz · line 48

Quoted 10 lines
linux/linux.mk +10 -0
+# 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.

linux/linux.mk LINUX_SOURCE = linux-$(LINUX_VERSION).tar.xz

Quoted 9 lines
linux/linux.mk +9 -0
+# 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

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