[Buildroot] [PATCH v2 1/1] linux: xenomai: allow URL for the ADEOS/IPIPE patch

[Buildroot] [PATCH v2 1/1] linux: xenomai: allow URL for the ADEOS/IPIPE patch

[Sébastien Szymanski]

With an URL Buildroot will be able to download the patch automatically.

Signed-off-by: S?bastien Szymanski <sebastien.szymanski@armadeus.com>
---

Changes for v2:
 - don't rename BR2_LINUX_KERNEL_EXT_XENOMAI_ADEOS_PATCH to BR2_LINUX_KERNEL_EXT_XENOMAI_ADEOS_PATCH_URL
 - use $(notdir ...) instead of $(shell basename)
 - be backward compatbile with paths

 linux/Config.ext.in        | 13 ++++++++-----
 linux/linux-ext-xenomai.mk | 12 +++++++++++-
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/linux/Config.ext.in b/linux/Config.ext.in
index 32dacbdf06..14140d352c 100644
--- a/linux/Config.ext.in
+++ b/linux/Config.ext.in
@@ -22,18 +22,21 @@ config BR2_LINUX_KERNEL_EXT_XENOMAI
 
 	  However, it is recommended to use the latest version of the
 	  Adeos/Ipipe patch available at
-	  http://download.gna.org/adeos/patches
+	  https://xenomai.org/downloads/ipipe/
 
 	  Xenomai is know to support Blackfin, SH4, x86, ARM, NIOS2
 	  and PowerPC architectures.
 
 config BR2_LINUX_KERNEL_EXT_XENOMAI_ADEOS_PATCH
-	string "Path for Adeos patch file"
+	string "Path/URL for Adeos patch file"
 	depends on BR2_LINUX_KERNEL_EXT_XENOMAI
 	help
-	  Optionally, explicitly specify the Adeos patch to use.
-	  Download it at http://download.gna.org/adeos/patches
-	  and verify that your kernel version in buildroot matches.
+	  Optionally, explicitly specify where to find the Adeos
+	  patch to use.
+	  Examples:
+	  https://xenomai.org/downloads/ipipe/v4.x/arm/ipipe-core-4.19.33-arm-2.patch
+	  or /home/foo/ipipe-core-4.19.33-arm-2.patch
+	  Please verify that your kernel version in Buildroot matches.
 
 comment "xenomai needs a uClibc or glibc toolchain w/ threads"
 	depends on BR2_PACKAGE_XENOMAI_COBALT_ARCH_SUPPORTS
diff --git a/linux/linux-ext-xenomai.mk b/linux/linux-ext-xenomai.mk
index d066bb32ac..ed29943e6c 100644
--- a/linux/linux-ext-xenomai.mk
+++ b/linux/linux-ext-xenomai.mk
@@ -8,10 +8,20 @@ LINUX_EXTENSIONS += xenomai
 
 # Adeos patch version
 XENOMAI_ADEOS_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_XENOMAI_ADEOS_PATCH))
+
+ifneq ($(filter ftp://% http://% https://%, $(XENOMAI_ADEOS_PATCH)),)
+XENOMAI_ADEOS_PATCH_NAME = $(notdir $(XENOMAI_ADEOS_PATCH))
+XENOMAI_ADEOS_PATCH_PATH = $(LINUX_DL_DIR)/$(XENOMAI_ADEOS_PATCH_NAME)
+LINUX_EXTRA_DOWNLOADS += $(XENOMAI_ADEOS_PATCH)
+BR_NO_CHECK_HASH_FOR += $(XENOMAI_ADEOS_PATCH_NAME)
+else
+XENOMAI_ADEOS_PATCH_PATH = $(subst file://,,$(XENOMAI_ADEOS_PATCH))
+endif
+
 ifeq ($(XENOMAI_ADEOS_PATCH),)
 XENOMAI_ADEOS_OPTS = --default
 else
-XENOMAI_ADEOS_OPTS = --adeos=$(XENOMAI_ADEOS_PATCH)
+XENOMAI_ADEOS_OPTS = --adeos=$(XENOMAI_ADEOS_PATCH_PATH)
 endif
 
 # Prepare kernel patch
-- 
2.19.2

[Buildroot] [PATCH v2 1/1] linux: xenomai: allow URL for the ADEOS/IPIPE patch

[Thomas Petazzoni]

Hello,

On Fri, 17 May 2019 14:50:49 +0200
S?bastien Szymanski <sebastien.szymanski@armadeus.com> wrote:


> diff --git a/linux/linux-ext-xenomai.mk b/linux/linux-ext-xenomai.mk
> index d066bb32ac..ed29943e6c 100644
> --- a/linux/linux-ext-xenomai.mk
> +++ b/linux/linux-ext-xenomai.mk
> @@ -8,10 +8,20 @@ LINUX_EXTENSIONS += xenomai
>  
>  # Adeos patch version
>  XENOMAI_ADEOS_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_XENOMAI_ADEOS_PATCH))
> +
> +ifneq ($(filter ftp://% http://% https://%, $(XENOMAI_ADEOS_PATCH)),)
> +XENOMAI_ADEOS_PATCH_NAME = $(notdir $(XENOMAI_ADEOS_PATCH))
> +XENOMAI_ADEOS_PATCH_PATH = $(LINUX_DL_DIR)/$(XENOMAI_ADEOS_PATCH_NAME)
> +LINUX_EXTRA_DOWNLOADS += $(XENOMAI_ADEOS_PATCH)
> +BR_NO_CHECK_HASH_FOR += $(XENOMAI_ADEOS_PATCH_NAME)
> +else
> +XENOMAI_ADEOS_PATCH_PATH = $(subst file://,,$(XENOMAI_ADEOS_PATCH))

I don't feel super strongly about this, but everywhere else in
Buildroot when paths are specified through configuration options, we do
not support having a file:// URI. For example the
BR2_LINUX_KERNEL_PATCH option will support file paths only without
file://.

So for consistency, I would do the same here, and therefore just do:

XENOMAI_ADEOS_PATCH_PATH = $(XENOMAI_ADEOS_PATCH)

Yann, what do you think ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 1/1] linux: xenomai: allow URL for the ADEOS/IPIPE patch

[Yann E. MORIN]

Thomas, S?bastien, All,

On 2019-05-18 22:06 +0200, Thomas Petazzoni spake thusly:
> On Fri, 17 May 2019 14:50:49 +0200
> S?bastien Szymanski <sebastien.szymanski@armadeus.com> wrote:
> > diff --git a/linux/linux-ext-xenomai.mk b/linux/linux-ext-xenomai.mk
> > index d066bb32ac..ed29943e6c 100644
> > --- a/linux/linux-ext-xenomai.mk
> > +++ b/linux/linux-ext-xenomai.mk
> > @@ -8,10 +8,20 @@ LINUX_EXTENSIONS += xenomai
> >  
> >  # Adeos patch version
> >  XENOMAI_ADEOS_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_XENOMAI_ADEOS_PATCH))
> > +
> > +ifneq ($(filter ftp://% http://% https://%, $(XENOMAI_ADEOS_PATCH)),)
> > +XENOMAI_ADEOS_PATCH_NAME = $(notdir $(XENOMAI_ADEOS_PATCH))
> > +XENOMAI_ADEOS_PATCH_PATH = $(LINUX_DL_DIR)/$(XENOMAI_ADEOS_PATCH_NAME)
> > +LINUX_EXTRA_DOWNLOADS += $(XENOMAI_ADEOS_PATCH)
> > +BR_NO_CHECK_HASH_FOR += $(XENOMAI_ADEOS_PATCH_NAME)
> > +else
> > +XENOMAI_ADEOS_PATCH_PATH = $(subst file://,,$(XENOMAI_ADEOS_PATCH))
> 
> I don't feel super strongly about this, but everywhere else in
> Buildroot when paths are specified through configuration options, we do
> not support having a file:// URI. For example the
> BR2_LINUX_KERNEL_PATCH option will support file paths only without
> file://.
> 
> So for consistency, I would do the same here, and therefore just do:
> 
> XENOMAI_ADEOS_PATCH_PATH = $(XENOMAI_ADEOS_PATCH)
> 
> Yann, what do you think ?

Indeed, I don't see the point in the file:// scheme. I don't even see
people really using it routinely.

But as far as I can see, the above, although wrong, does not require
that the file:// prefix be used. If it is there, it is scrubbed;
otherwise the filename is used as-is.

And it is wrong, because it should only remove it at the beginning.
$(subst ...) would remove it anywhere. So if we were to use it (and I
think we should not), we'd have to use $(patsubst ...) instead;
    XENOMAI_ADEOS_PATCH_PATH = $(patsubst file://%,%,$(XENOMAI_ADEOS_PATCH))

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 1/1] linux: xenomai: allow URL for the ADEOS/IPIPE patch

[Sébastien Szymanski]

Hello,

On 5/19/19 11:17 AM, Yann E. MORIN wrote:
> Thomas, S?bastien, All,
> 
> On 2019-05-18 22:06 +0200, Thomas Petazzoni spake thusly:
>> On Fri, 17 May 2019 14:50:49 +0200
>> S?bastien Szymanski <sebastien.szymanski@armadeus.com> wrote:
>>> diff --git a/linux/linux-ext-xenomai.mk b/linux/linux-ext-xenomai.mk
>>> index d066bb32ac..ed29943e6c 100644
>>> --- a/linux/linux-ext-xenomai.mk
>>> +++ b/linux/linux-ext-xenomai.mk
>>> @@ -8,10 +8,20 @@ LINUX_EXTENSIONS += xenomai
>>>  
>>>  # Adeos patch version
>>>  XENOMAI_ADEOS_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_XENOMAI_ADEOS_PATCH))
>>> +
>>> +ifneq ($(filter ftp://% http://% https://%, $(XENOMAI_ADEOS_PATCH)),)
>>> +XENOMAI_ADEOS_PATCH_NAME = $(notdir $(XENOMAI_ADEOS_PATCH))
>>> +XENOMAI_ADEOS_PATCH_PATH = $(LINUX_DL_DIR)/$(XENOMAI_ADEOS_PATCH_NAME)
>>> +LINUX_EXTRA_DOWNLOADS += $(XENOMAI_ADEOS_PATCH)
>>> +BR_NO_CHECK_HASH_FOR += $(XENOMAI_ADEOS_PATCH_NAME)
>>> +else
>>> +XENOMAI_ADEOS_PATCH_PATH = $(subst file://,,$(XENOMAI_ADEOS_PATCH))
>>
>> I don't feel super strongly about this, but everywhere else in
>> Buildroot when paths are specified through configuration options, we do
>> not support having a file:// URI. For example the
>> BR2_LINUX_KERNEL_PATCH option will support file paths only without
>> file://.
>>
>> So for consistency, I would do the same here, and therefore just do:
>>
>> XENOMAI_ADEOS_PATCH_PATH = $(XENOMAI_ADEOS_PATCH)
>>
>> Yann, what do you think ?
> 
> Indeed, I don't see the point in the file:// scheme. I don't even see
> people really using it routinely.
> 
> But as far as I can see, the above, although wrong, does not require
> that the file:// prefix be used. If it is there, it is scrubbed;
> otherwise the filename is used as-is.

I don't understand what you mean with "it is scrubbed", sorry.
With the file:// scheme, Buildroot will try to download the patch with
wget and fail, that's why I filter the file:// scheme out.

But I agree that the file:// scheme is not really useful.

> 
> And it is wrong, because it should only remove it at the beginning.
> $(subst ...) would remove it anywhere. So if we were to use it (and I
> think we should not), we'd have to use $(patsubst ...) instead;
>     XENOMAI_ADEOS_PATCH_PATH = $(patsubst file://%,%,$(XENOMAI_ADEOS_PATCH))

Indeed... Thanks

Regards,

> 
> Regards,
> Yann E. MORIN.
> 


-- 
S?bastien Szymanski
Software engineer, Armadeus Systems
Tel: +33 (0)9 72 29 41 44
Fax: +33 (0)9 72 28 79 26

[Buildroot] [PATCH v2 1/1] linux: xenomai: allow URL for the ADEOS/IPIPE patch

[Thomas Petazzoni]

Hello,

On Mon, 20 May 2019 09:29:55 +0200
S?bastien Szymanski <sebastien.szymanski@armadeus.com> wrote:

> I don't understand what you mean with "it is scrubbed", sorry.
> With the file:// scheme, Buildroot will try to download the patch with
> wget and fail, that's why I filter the file:// scheme out.

Not with your latest version. With your latest version, you add the
patch URL to EXTRA_DOWNLOADS only if it contains http://, https:// or
ftp://. So if it's a plain path, like /home/foo/bar, it certainly won't
try to download it.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 1/1] linux: xenomai: allow URL for the ADEOS/IPIPE patch

[Sébastien Szymanski]

Hello,

On 5/20/19 9:31 AM, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 20 May 2019 09:29:55 +0200
> S?bastien Szymanski <sebastien.szymanski@armadeus.com> wrote:
> 
>> I don't understand what you mean with "it is scrubbed", sorry.
>> With the file:// scheme, Buildroot will try to download the patch with
>> wget and fail, that's why I filter the file:// scheme out.
> 
> Not with your latest version. With your latest version, you add the
> patch URL to EXTRA_DOWNLOADS only if it contains http://, https:// or
> ftp://. So if it's a plain path, like /home/foo/bar, it certainly won't
> try to download it.

Yes... I need more coffee ;)

Regards,

> 
> Thomas
> 


-- 
S?bastien Szymanski
Software engineer, Armadeus Systems
Tel: +33 (0)9 72 29 41 44
Fax: +33 (0)9 72 28 79 26

[Buildroot] [PATCH v2 1/1] linux: xenomai: allow URL for the ADEOS/IPIPE patch

[Thomas Petazzoni]

Hello,

On Fri, 17 May 2019 14:50:49 +0200
S?bastien Szymanski <sebastien.szymanski@armadeus.com> wrote:

> With an URL Buildroot will be able to download the patch automatically.
> 
> Signed-off-by: S?bastien Szymanski <sebastien.szymanski@armadeus.com>
> ---

Applied to next after dropping the file:// substitution.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com