diff mbox series

wireguard: needs 3.10+ kernel

Message ID 20171003080448.3809-1-peter@korsgaard.com
State Accepted
Commit 1afda25ed281b0e842833cf85c3f3aa7cca343df
Headers show
Series wireguard: needs 3.10+ kernel | expand

Commit Message

Peter Korsgaard Oct. 3, 2017, 8:04 a.m. UTC
The dependency is actually only for the kernel module (and thus on the
runtime kernel version rather than kernel headers), but as we don't know the
runtime version in kconfig this will have to do.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
 package/wireguard/Config.in | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Arnout Vandecappelle Oct. 5, 2017, 6:44 p.m. UTC | #1
Hi Peter,

 Looks like I don't manage to apply any of your patches today :-)

On 03-10-17 10:04, Peter Korsgaard wrote:
> The dependency is actually only for the kernel module (and thus on the
> runtime kernel version rather than kernel headers), but as we don't know the
> runtime version in kconfig this will have to do.
> 
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> ---
>  package/wireguard/Config.in | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/wireguard/Config.in b/package/wireguard/Config.in
> index 0321755db3..acce6663ba 100644
> --- a/package/wireguard/Config.in
> +++ b/package/wireguard/Config.in
> @@ -1,5 +1,6 @@
>  config BR2_PACKAGE_WIREGUARD
>  	bool "wireguard"
> +	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10

 The kernel module is only built if BR2_LINUX_KERNEL=y, so shouldn't this be

	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10 || !BR2_LINUX_KERNEL

? And of course in the comment as well.

 In addition, I think you should add the (abbreviated) contents of the commit
message as a comment here, otherwise it's not at all clear.

 Regards,
 Arnout

>  	select BR2_PACKAGE_LIBMNL
>  	help
>  	  WireGuard is an extremely simple yet fast and modern VPN
> @@ -16,3 +17,6 @@ config BR2_PACKAGE_WIREGUARD
>  	  VPN solution in the industry.
>  
>  	  https://www.wireguard.com
> +
> +comment "wireguard needs a toolchain w/ headers > 3.10"
> +	depends on !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10
>
Peter Korsgaard Oct. 5, 2017, 7:46 p.m. UTC | #2
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 >  Hi Peter,
 >  Looks like I don't manage to apply any of your patches today :-)

 > On 03-10-17 10:04, Peter Korsgaard wrote:
 >> The dependency is actually only for the kernel module (and thus on the
 >> runtime kernel version rather than kernel headers), but as we don't know the
 >> runtime version in kconfig this will have to do.
 >> 
 >> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
 >> ---
 >> package/wireguard/Config.in | 4 ++++
 >> 1 file changed, 4 insertions(+)
 >> 
 >> diff --git a/package/wireguard/Config.in b/package/wireguard/Config.in
 >> index 0321755db3..acce6663ba 100644
 >> --- a/package/wireguard/Config.in
 >> +++ b/package/wireguard/Config.in
 >> @@ -1,5 +1,6 @@
 >> config BR2_PACKAGE_WIREGUARD
 >> bool "wireguard"
 >> +	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10

 >  The kernel module is only built if BR2_LINUX_KERNEL=y, so shouldn't this be

 > 	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10 || !BR2_LINUX_KERNEL

That was my first thought as well, but it doesn't make much sense to
build the user space part if the kernel is too old, and I couldn't come
up with a good and clear wording for the comment - So I opted to keep it
simple.

But we can change it if you feel strongly about it. What do you suggest
for the comment text?

 > ? And of course in the comment as well.

 >  In addition, I think you should add the (abbreviated) contents of the commit
 > message as a comment here, otherwise it's not at all clear.

Ok, I can do that when applying.
Arnout Vandecappelle Oct. 5, 2017, 7:51 p.m. UTC | #3
On 05-10-17 21:46, Peter Korsgaard wrote:
>>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
> 
>  >  Hi Peter,
>  >  Looks like I don't manage to apply any of your patches today :-)
> 
>  > On 03-10-17 10:04, Peter Korsgaard wrote:
>  >> The dependency is actually only for the kernel module (and thus on the
>  >> runtime kernel version rather than kernel headers), but as we don't know the
>  >> runtime version in kconfig this will have to do.
>  >> 
>  >> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
>  >> ---
>  >> package/wireguard/Config.in | 4 ++++
>  >> 1 file changed, 4 insertions(+)
>  >> 
>  >> diff --git a/package/wireguard/Config.in b/package/wireguard/Config.in
>  >> index 0321755db3..acce6663ba 100644
>  >> --- a/package/wireguard/Config.in
>  >> +++ b/package/wireguard/Config.in
>  >> @@ -1,5 +1,6 @@
>  >> config BR2_PACKAGE_WIREGUARD
>  >> bool "wireguard"
>  >> +	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10
> 
>  >  The kernel module is only built if BR2_LINUX_KERNEL=y, so shouldn't this be
> 
>  > 	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10 || !BR2_LINUX_KERNEL
> 
> That was my first thought as well, but it doesn't make much sense to
> build the user space part if the kernel is too old,

 That's true indeed, could you add that reasoning to the commit message? Then
there's no need to complicate the dependency.

> and I couldn't come
> up with a good and clear wording for the comment - So I opted to keep it
> simple.
> 
> But we can change it if you feel strongly about it. What do you suggest
> for the comment text?

	# kernel module requires 3.10+, userspace makes no sense without it


 Regards,
 Arnout

> 
>  > ? And of course in the comment as well.
> 
>  >  In addition, I think you should add the (abbreviated) contents of the commit
>  > message as a comment here, otherwise it's not at all clear.
> 
> Ok, I can do that when applying.
>
Peter Korsgaard Oct. 5, 2017, 8:53 p.m. UTC | #4
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> That was my first thought as well, but it doesn't make much sense to
 >> build the user space part if the kernel is too old,

 >  That's true indeed, could you add that reasoning to the commit message? Then
 > there's no need to complicate the dependency.

 >> and I couldn't come
 >> up with a good and clear wording for the comment - So I opted to keep it
 >> simple.
 >> 
 >> But we can change it if you feel strongly about it. What do you suggest
 >> for the comment text?

 > 	# kernel module requires 3.10+, userspace makes no sense without it

Committed with these fixes, thanks.
diff mbox series

Patch

diff --git a/package/wireguard/Config.in b/package/wireguard/Config.in
index 0321755db3..acce6663ba 100644
--- a/package/wireguard/Config.in
+++ b/package/wireguard/Config.in
@@ -1,5 +1,6 @@ 
 config BR2_PACKAGE_WIREGUARD
 	bool "wireguard"
+	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10
 	select BR2_PACKAGE_LIBMNL
 	help
 	  WireGuard is an extremely simple yet fast and modern VPN
@@ -16,3 +17,6 @@  config BR2_PACKAGE_WIREGUARD
 	  VPN solution in the industry.
 
 	  https://www.wireguard.com
+
+comment "wireguard needs a toolchain w/ headers > 3.10"
+	depends on !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10