Skip to contentSkip to conversation

rtai: remove option BR2_LINUX_KERNEL_EXT_RTAI_PATCH

PATCH New Thomas Petazzoni 3 participants 7 messages Apr 21, 2015 – Apr 22, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 rtai: remove option BR2_LINUX_KERNEL_EXT_RTAI_PATCH

This commit removes BR2_LINUX_KERNEL_EXT_RTAI_PATCH because this option never worked. It was added in commit 8797a9cd1fe6723db34b0c125d0d9d04e3483e8d, which added package/rtai/ and RTAI as a Linux extension.

The option prompt says "Path for RTAI patch file", so let's say you specify /home/foo/bar/myrtai.patch as the value for BR2_LINUX_KERNEL_EXT_RTAI_PATCH.

Then the code does:

RTAI_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_RTAI_PATCH))

and we have a package called 'rtai', so the normal logic of <pkg>_PATCH applies. Since the <pkg>_PATCH value does not contain ftp://, http:// or https://, the package infrastructure will try to download $(RTAI_SITE)/$(RTAI_PATCH), i.e:

https://www.rtai.org/userfiles/downloads/RTAI/home/foo/bar/myrtai.patch

Pretty clear that it has no chance of working.

Now, let's assume an URL is used as the value of BR2_LINUX_KERNEL_EXT_RTAI_PATCH, such as
http://foo.com/bar/myrtai.patch. In this case, it will be properly downloaded by the package infrastructure. But then, the following code kicks in:

define RTAI_PREPARE_KERNEL

       $(APPLY_PATCHES)                        \
               $(LINUX_DIR)                    \
               $(dir $(RTAI_PATCH))            \
               $(notdir $(RTAI_PATCH))

endef

The value of $(dir $(RTAI_PATCH)) will be http://foo.com/bar/. How can $(APPLY_PATCHES) make use of such a stupid patch location?

Bottom line is that this feature has never worked, so there is not even a point in adding Config.in.legacy support for it.

---
 linux/Config.ext.in     |  6 ------
 linux/linux-ext-rtai.mk | 11 -----------
 2 files changed, 17 deletions(-)
linux/Config.ext.in +0 -6
index c909254..ea79aa1 100644
@@ -44,12 +44,6 @@ config BR2_LINUX_KERNEL_EXT_RTAI
 	help
 	  RTAI Kernel part.
 
-config BR2_LINUX_KERNEL_EXT_RTAI_PATCH
-	depends on BR2_LINUX_KERNEL_EXT_RTAI
-	string "Path for RTAI patch file"
-	help
-	  Optionally, explicitly specify the RTAI patch to use.
-
 # fbtft
 config BR2_LINUX_KERNEL_EXT_FBTFT
 	bool "FB TFT drivers"
linux/linux-ext-rtai.mk +0 -11
index 07e7a19..0cc1232 100644
@@ -6,8 +6,6 @@
 
 LINUX_EXTENSIONS += rtai
 
-RTAI_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_RTAI_PATCH))
-
 ifeq ($(KERNEL_ARCH),i386)
 RTAI_ARCH = x86
 else ifeq ($(KERNEL_ARCH),x86_64)
@@ -19,7 +17,6 @@ RTAI_ARCH = $(KERNEL_ARCH)
 endif
 
 # Prepare kernel patch
-ifeq ($(RTAI_PATCH),)
 define RTAI_PREPARE_KERNEL
 	kver=`$(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) --no-print-directory -s kernelversion` ; \
 	if test -f $(RTAI_DIR)/base/arch/$(RTAI_ARCH)/patches/hal-linux-$${kver}-*patch ; then \
@@ -31,11 +28,3 @@ define RTAI_PREPARE_KERNEL
 		exit 1 ; \
 	fi
 endef
-else
-define RTAI_PREPARE_KERNEL
-	$(APPLY_PATCHES) 			\
-		$(LINUX_DIR)			\
-		$(dir $(RTAI_PATCH))		\
-		$(notdir $(RTAI_PATCH))
-endef
-endif
Signature · 1 line
2.1.0
#1 lore raw

PATCH 1/1 rtai: remove option BR2_LINUX_KERNEL_EXT_RTAI_PATCH

Quoted 4 lines · Thomas Petazzoni

This commit removes BR2_LINUX_KERNEL_EXT_RTAI_PATCH because this option never worked. It was added in commit 8797a9cd1fe6723db34b0c125d0d9d04e3483e8d, which added package/rtai/ and RTAI as a Linux extension.

I agree with this change, however...

[snip]

Quoted 5 lines
linux/Config.ext.in +0 -0
index c909254..ea79aa1 100644

linux/Config.ext.in config BR2_LINUX_KERNEL_EXT_RTAI · line 52

Quoted 9 lines
linux/Config.ext.in +0 -6
@@ -44,12 +44,6 @@ config BR2_LINUX_KERNEL_EXT_RTAI
 	help
 	  RTAI Kernel part.
 
-config BR2_LINUX_KERNEL_EXT_RTAI_PATCH
-	depends on BR2_LINUX_KERNEL_EXT_RTAI
-	string "Path for RTAI patch file"
-	help
-	  Optionally, explicitly specify the RTAI patch to use.
-

This requires legacy handling. With in the help text something like

The BR2_LINUX_KERNEL_EXT_RTAI_PATCH option never worked, so it is probably not useful. If you do have a valid use case and it used to work for you, please let us know on buildroot at buildroot.net.

Regards,
Arnout

[snip]

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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#2 lore raw

PATCH 1/1 rtai: remove option BR2_LINUX_KERNEL_EXT_RTAI_PATCH

Arnout,

Quoted 5 lines · Arnout Vandecappelle

This requires legacy handling. With in the help text something like

The BR2_LINUX_KERNEL_EXT_RTAI_PATCH option never worked, so it is probably not useful. If you do have a valid use case and it used to work for you, please let us know on buildroot at buildroot.net.

Well, I talked about Config.in.legacy handling in the commit message:

"""
Bottom line is that this feature has never worked, so there is not even a point in adding Config.in.legacy support for it. """

Should I understand that you don't agree with this reasoning, and still want Config.in.legacy handling?

Thanks,

Thomas

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

PATCH 1/1 rtai: remove option BR2_LINUX_KERNEL_EXT_RTAI_PATCH

Quoted 11 lines · Thomas Petazzoni

Arnout,

Quoted 5 lines · Arnout Vandecappelle

This requires legacy handling. With in the help text something like

The BR2_LINUX_KERNEL_EXT_RTAI_PATCH option never worked, so it is probably not useful. If you do have a valid use case and it used to work for you, please let us know on buildroot at buildroot.net.

Well, I talked about Config.in.legacy handling in the commit message:

TL;DR :-)

Quoted 8 lines

"""
Bottom line is that this feature has never worked, so there is not even a point in adding Config.in.legacy support for it. """

Should I understand that you don't agree with this reasoning, and still want Config.in.legacy handling?

Yep, because if someone _does_ have it set, it probably works for them and we
should know about it. If nobody has it set, then it doesn't hurt to have it in legacy.

Regards,
Arnout

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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#4 lore raw

PATCH 1/1 rtai: remove option BR2_LINUX_KERNEL_EXT_RTAI_PATCH

Arnout, All,

On 2015-04-21 23:59 +0200, Arnout Vandecappelle spake thusly:

Quoted 21 lines · Thomas Petazzoni
Quoted 11 lines

Arnout,

Quoted 5 lines · Arnout Vandecappelle

This requires legacy handling. With in the help text something like

The BR2_LINUX_KERNEL_EXT_RTAI_PATCH option never worked, so it is probably not useful. If you do have a valid use case and it used to work for you, please let us know on buildroot at buildroot.net.

Well, I talked about Config.in.legacy handling in the commit message:

TL;DR :-)

Quoted 6 lines

"""
Bottom line is that this feature has never worked, so there is not even a point in adding Config.in.legacy support for it. """

Quoted 6 lines
Quoted 2 lines

Should I understand that you don't agree with this reasoning, and still want Config.in.legacy handling?

Yep, because if someone _does_ have it set, it probably works for them and we
should know about it. If nobody has it set, then it doesn't hurt to have it in legacy.

But there is absolutely *no way* this can work *at all*, as demonstrated by Thomas.

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#5 lore raw

PATCH 1/1 rtai: remove option BR2_LINUX_KERNEL_EXT_RTAI_PATCH

In reply to Thomas Petazzoni

Thomas, All,

On 2015-04-21 23:36 +0200, Thomas Petazzoni spake thusly:

Quoted 35 lines

This commit removes BR2_LINUX_KERNEL_EXT_RTAI_PATCH because this option never worked. It was added in commit 8797a9cd1fe6723db34b0c125d0d9d04e3483e8d, which added package/rtai/ and RTAI as a Linux extension.

The option prompt says "Path for RTAI patch file", so let's say you specify /home/foo/bar/myrtai.patch as the value for BR2_LINUX_KERNEL_EXT_RTAI_PATCH.

Then the code does:

RTAI_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_RTAI_PATCH))

and we have a package called 'rtai', so the normal logic of <pkg>_PATCH applies. Since the <pkg>_PATCH value does not contain ftp://, http:// or https://, the package infrastructure will try to download $(RTAI_SITE)/$(RTAI_PATCH), i.e:

https://www.rtai.org/userfiles/downloads/RTAI/home/foo/bar/myrtai.patch

Pretty clear that it has no chance of working.

Now, let's assume an URL is used as the value of BR2_LINUX_KERNEL_EXT_RTAI_PATCH, such as
http://foo.com/bar/myrtai.patch. In this case, it will be properly downloaded by the package infrastructure. But then, the following code kicks in:

define RTAI_PREPARE_KERNEL

       $(APPLY_PATCHES)                        \
               $(LINUX_DIR)                    \
               $(dir $(RTAI_PATCH))            \
               $(notdir $(RTAI_PATCH))

endef

Quoted 7 lines

The value of $(dir $(RTAI_PATCH)) will be http://foo.com/bar/. How can $(APPLY_PATCHES) make use of such a stupid patch location?

Bottom line is that this feature has never worked, so there is not even a point in adding Config.in.legacy support for it.

Regards,
Yann E. MORIN.

Quoted 62 lines
---
 linux/Config.ext.in     |  6 ------
 linux/linux-ext-rtai.mk | 11 -----------
 2 files changed, 17 deletions(-)
linux/Config.ext.in +0 -6
index c909254..ea79aa1 100644
@@ -44,12 +44,6 @@ config BR2_LINUX_KERNEL_EXT_RTAI
 	help
 	  RTAI Kernel part.
 
-config BR2_LINUX_KERNEL_EXT_RTAI_PATCH
-	depends on BR2_LINUX_KERNEL_EXT_RTAI
-	string "Path for RTAI patch file"
-	help
-	  Optionally, explicitly specify the RTAI patch to use.
-
 # fbtft
 config BR2_LINUX_KERNEL_EXT_FBTFT
 	bool "FB TFT drivers"
linux/linux-ext-rtai.mk +0 -11
index 07e7a19..0cc1232 100644
@@ -6,8 +6,6 @@
 
 LINUX_EXTENSIONS += rtai
 
-RTAI_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_RTAI_PATCH))
-
 ifeq ($(KERNEL_ARCH),i386)
 RTAI_ARCH = x86
 else ifeq ($(KERNEL_ARCH),x86_64)
@@ -19,7 +17,6 @@ RTAI_ARCH = $(KERNEL_ARCH)
 endif
 
 # Prepare kernel patch
-ifeq ($(RTAI_PATCH),)
 define RTAI_PREPARE_KERNEL
 	kver=`$(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) --no-print-directory -s kernelversion` ; \
 	if test -f $(RTAI_DIR)/base/arch/$(RTAI_ARCH)/patches/hal-linux-$${kver}-*patch ; then \
@@ -31,11 +28,3 @@ define RTAI_PREPARE_KERNEL
 		exit 1 ; \
 	fi
 endef
-else
-define RTAI_PREPARE_KERNEL
-	$(APPLY_PATCHES) 			\
-		$(LINUX_DIR)			\
-		$(dir $(RTAI_PATCH))		\
-		$(notdir $(RTAI_PATCH))
-endef
-endif
Signature · 6 lines
2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#6 lore raw

PATCH 1/1 rtai: remove option BR2_LINUX_KERNEL_EXT_RTAI_PATCH

In reply to Thomas Petazzoni

Hello,

Quoted 38 lines · Thomas Petazzoni

This commit removes BR2_LINUX_KERNEL_EXT_RTAI_PATCH because this option never worked. It was added in commit 8797a9cd1fe6723db34b0c125d0d9d04e3483e8d, which added package/rtai/ and RTAI as a Linux extension.

The option prompt says "Path for RTAI patch file", so let's say you specify /home/foo/bar/myrtai.patch as the value for BR2_LINUX_KERNEL_EXT_RTAI_PATCH.

Then the code does:

RTAI_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_RTAI_PATCH))

and we have a package called 'rtai', so the normal logic of <pkg>_PATCH applies. Since the <pkg>_PATCH value does not contain ftp://, http:// or https://, the package infrastructure will try to download $(RTAI_SITE)/$(RTAI_PATCH), i.e:

https://www.rtai.org/userfiles/downloads/RTAI/home/foo/bar/myrtai.patch

Pretty clear that it has no chance of working.

Now, let's assume an URL is used as the value of BR2_LINUX_KERNEL_EXT_RTAI_PATCH, such as
http://foo.com/bar/myrtai.patch. In this case, it will be properly downloaded by the package infrastructure. But then, the following code kicks in:

define RTAI_PREPARE_KERNEL

       $(APPLY_PATCHES)                        \
               $(LINUX_DIR)                    \
               $(dir $(RTAI_PATCH))            \
               $(notdir $(RTAI_PATCH))

endef

The value of $(dir $(RTAI_PATCH)) will be http://foo.com/bar/. How can $(APPLY_PATCHES) make use of such a stupid patch location?

Quoted 8 lines

Bottom line is that this feature has never worked, so there is not even a point in adding Config.in.legacy support for it.

---
 linux/Config.ext.in     |  6 ------
 linux/linux-ext-rtai.mk | 11 -----------
 2 files changed, 17 deletions(-)

I've applied, after adding Config.in.legacy handling as suggested by Arnout.

Thomas

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