| Message ID | 1481012002-30425-1-git-send-email-christian.kellermann@solectrix.de |
|---|---|
| State | Changes Requested |
| Headers | show |
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. >
> 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
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.
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(+)