[Buildroot] [PATCH v2,1/2] package/knock: bump to version 0.8

[Buildroot] [PATCH v2,1/2] package/knock: bump to version 0.8

[Fabrice Fontaine]

https://github.com/jvinet/knock/blob/v0.8/ChangeLog

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
Changes v1 -> v2 (after review of Yann E. Morin):
 - Don't use official tarball as it is a complete git working copy

 package/knock/knock.hash | 2 +-
 package/knock/knock.mk   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/package/knock/knock.hash b/package/knock/knock.hash
index 0b9a0918fc..fb8d9aef78 100644
--- a/package/knock/knock.hash
+++ b/package/knock/knock.hash
@@ -1,3 +1,3 @@
 # locally computed hash
-sha256  940a8de61b4b3530560805c3b9310a077c28c99173c2ccb07ff96720b0e93c58  knock-258a27e5a47809f97c2b9f2751a88c2f94aae891.tar.gz
+sha256  7a82c276ca1540faa7f5bb25010f4a59d45323a31c44a30fbe4a6e484dd18b1a  knock-0.8.tar.gz
 sha256  d747e19206e41702e40822dd91d37cbf40edd86f364ea416d667a0e3013f7189  COPYING
diff --git a/package/knock/knock.mk b/package/knock/knock.mk
index 55f5bfdf36..2cfa738074 100644
--- a/package/knock/knock.mk
+++ b/package/knock/knock.mk
@@ -4,8 +4,8 @@
 #
 ################################################################################
 
-KNOCK_VERSION = 258a27e5a47809f97c2b9f2751a88c2f94aae891
-KNOCK_SITE = $(call github,jvinet,knock,$(KNOCK_VERSION))
+KNOCK_VERSION = 0.8
+KNOCK_SITE = $(call github,jvinet,knock,v$(KNOCK_VERSION))
 KNOCK_AUTORECONF = YES
 KNOCK_LICENSE = GPL-2.0+
 KNOCK_LICENSE_FILES = COPYING
-- 
2.33.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2,2/2] package/knock: add knockd option

[Fabrice Fontaine]

Add knockd option to make libpcap and MMU optional and enable it by
default for backward compatibility

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/knock/Config.in | 14 ++++++++++++--
 package/knock/knock.mk  |  9 +++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/package/knock/Config.in b/package/knock/Config.in
index 0060ad07ce..5a4c00d9e8 100644
--- a/package/knock/Config.in
+++ b/package/knock/Config.in
@@ -1,7 +1,5 @@
 config BR2_PACKAGE_KNOCK
 	bool "knock"
-	depends on BR2_USE_MMU # fork()
-	select BR2_PACKAGE_LIBPCAP
 	help
 	  A port knocking implementation.
 	  Provides a daemon and a user application. Port knocking can be
@@ -11,3 +9,15 @@ config BR2_PACKAGE_KNOCK
 	  firewall.
 
 	  http://www.zeroflux.org/projects/knock
+
+if BR2_PACKAGE_KNOCK
+
+config BR2_PACKAGE_KNOCK_KNOCKD
+	bool "knockd"
+	default y
+	depends on BR2_USE_MMU # fork()
+	select BR2_PACKAGE_LIBPCAP
+	help
+	  Enable knockd
+
+endif
diff --git a/package/knock/knock.mk b/package/knock/knock.mk
index 2cfa738074..82a9ba9b53 100644
--- a/package/knock/knock.mk
+++ b/package/knock/knock.mk
@@ -9,10 +9,15 @@ KNOCK_SITE = $(call github,jvinet,knock,v$(KNOCK_VERSION))
 KNOCK_AUTORECONF = YES
 KNOCK_LICENSE = GPL-2.0+
 KNOCK_LICENSE_FILES = COPYING
-KNOCK_DEPENDENCIES = libpcap
 
+ifeq ($(BR2_PACKAGE_KNOCK_KNOCKD),y)
+KNOCK_DEPENDENCIES = libpcap
+KNOCK_CONF_OPTS += --enable-knockd
 ifeq ($(BR2_STATIC_LIBS),y)
-KNOCK_CONF_OPTS = LIBS="`$(STAGING_DIR)/usr/bin/pcap-config --static --additional-libs`"
+KNOCK_CONF_OPTS += LIBS="`$(STAGING_DIR)/usr/bin/pcap-config --static --additional-libs`"
+endif
+else
+KNOCK_CONF_OPTS += --disable-knockd
 endif
 
 $(eval $(autotools-package))
-- 
2.33.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2,1/2] package/knock: bump to version 0.8

[Thomas Petazzoni]

On Sat, 16 Oct 2021 11:32:33 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> https://github.com/jvinet/knock/blob/v0.8/ChangeLog
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
> Changes v1 -> v2 (after review of Yann E. Morin):
>  - Don't use official tarball as it is a complete git working copy

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2,2/2] package/knock: add knockd option

[Thomas Petazzoni]

On Sat, 16 Oct 2021 11:32:34 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> +ifeq ($(BR2_PACKAGE_KNOCK_KNOCKD),y)
> +KNOCK_DEPENDENCIES = libpcap

From commit 97f3ad7af3dd54a15a10aa35786e7fa08cf5e7b1:

    Repeat after me: "Forcing the value of <pkg>_DEPENDENCIES inside a
    conditional is the root of all evil."
    
    Repeat after me: "Forcing the value of <pkg>_DEPENDENCIES inside a
    conditional is the root of all evil."
    
    Repeat after me: "Forcing the value of <pkg>_DEPENDENCIES inside a
    conditional is the root of all evil."
    
    Repeat after me: "Forcing the value of <pkg>_DEPENDENCIES inside a
    conditional is the root of all evil."

Care to repeat after me ? :-)

Anyway, with this typo fixed, applied!

Thanks,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2,2/2] package/knock: add knockd option

[Yann E. MORIN]

Thomas, All,

On 2021-10-18 22:04 +0200, Thomas Petazzoni spake thusly:
> On Sat, 16 Oct 2021 11:32:34 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> > +ifeq ($(BR2_PACKAGE_KNOCK_KNOCKD),y)
> > +KNOCK_DEPENDENCIES = libpcap
> From commit 97f3ad7af3dd54a15a10aa35786e7fa08cf5e7b1:
>     Repeat after me: "Forcing the value of <pkg>_DEPENDENCIES inside a
>     conditional is the root of all evil."
> Care to repeat after me ? :-)

OK, I missed that in my review of v1...
/me hides in shame...

But seriously though: why is that not caught by check-package?

Because it is not actually an override: the variable is not yet set
there, so it is valid to unconditionally set it.

check-package should probably recognise that the variable is
<FOO>_DEPENDENCIES and consider that conditional assignment should
still be forbidden in that case.

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 mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] conditional assigment check in check-package [was: Re: [PATCH v2, 2/2] package/knock: add knockd option]

[Arnout Vandecappelle]

On 18/10/2021 22:18, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2021-10-18 22:04 +0200, Thomas Petazzoni spake thusly:
>> On Sat, 16 Oct 2021 11:32:34 +0200
>> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>>> +ifeq ($(BR2_PACKAGE_KNOCK_KNOCKD),y)
>>> +KNOCK_DEPENDENCIES = libpcap
>>  From commit 97f3ad7af3dd54a15a10aa35786e7fa08cf5e7b1:
>>      Repeat after me: "Forcing the value of <pkg>_DEPENDENCIES inside a
>>      conditional is the root of all evil."
>> Care to repeat after me ? :-)
> 
> OK, I missed that in my review of v1...
> /me hides in shame...
> 
> But seriously though: why is that not caught by check-package?
> 
> Because it is not actually an override: the variable is not yet set
> there, so it is valid to unconditionally set it.
> 
> check-package should probably recognise that the variable is
> <FOO>_DEPENDENCIES and consider that conditional assignment should
> still be forbidden in that case.

  I had a look at this, and it is not entirely trivial to test for in check-package.

  Basically, this is allowed:

ifeq(...)
FOO_VAR = ...
endif
... $(FOO_VAR) ... # Empty if condition not true

  And this as well:

ifeq(...)
FOO_VAR = ...
else
FOO_VAR = ...
endif

  But this is not:

ifeq(...)
FOO_VAR = ...
endif

ifeq(...)
FOO_VAR = ...
endif


  So basically:
- after a variable is assigned, it can't be assigned to any more in the same 
conditional branch;
- it can be assigned to again in the else branch (including else ifeq constructs);
- it can not be assigned to any more after the endif.

  In addition, conditions can be nested...

  The only way I can think of implementing this is by keeping a stack of 
conditionally assigned variables. In the else branch, the top of the stack is 
saved en empties. At the endif, the top of the stack is popped, merged with the 
saved one, and merged into the new top of the stack.

  Wow, by writing it down, I almost implemented it :-) Let's see if I can still 
find the time to actually do it tonight...


  Regards,
  Arnout

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] conditional assigment check in check-package [was: Re: [PATCH v2, 2/2] package/knock: add knockd option]

[Yann E. MORIN]

Arnout, All,

On 2021-10-21 20:43 +0200, Arnout Vandecappelle spake thusly:
> On 18/10/2021 22:18, Yann E. MORIN wrote:
> >Thomas, All,
> >On 2021-10-18 22:04 +0200, Thomas Petazzoni spake thusly:
> >>On Sat, 16 Oct 2021 11:32:34 +0200
> >>Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> >>>+ifeq ($(BR2_PACKAGE_KNOCK_KNOCKD),y)
> >>>+KNOCK_DEPENDENCIES = libpcap
> >> From commit 97f3ad7af3dd54a15a10aa35786e7fa08cf5e7b1:
> >>     Repeat after me: "Forcing the value of <pkg>_DEPENDENCIES inside a
> >>     conditional is the root of all evil."
> >>Care to repeat after me ? :-)
> >
> >OK, I missed that in my review of v1...
> >/me hides in shame...
> >
> >But seriously though: why is that not caught by check-package?
> >
> >Because it is not actually an override: the variable is not yet set
> >there, so it is valid to unconditionally set it.
> >
> >check-package should probably recognise that the variable is
> ><FOO>_DEPENDENCIES and consider that conditional assignment should
> >still be forbidden in that case.
> 
>  I had a look at this, and it is not entirely trivial to test for in check-package.
> 
>  Basically, this is allowed:
> 
> ifeq(...)
> FOO_VAR = ...
> endif
> ... $(FOO_VAR) ... # Empty if condition not true
> 
>  And this as well:
> 
> ifeq(...)
> FOO_VAR = ...
> else
> FOO_VAR = ...
> endif
> 
>  But this is not:
> 
> ifeq(...)
> FOO_VAR = ...
> endif
> 
> ifeq(...)
> FOO_VAR = ...
> endif
> 
> 
>  So basically:
> - after a variable is assigned, it can't be assigned to any more in the same
> conditional branch;
> - it can be assigned to again in the else branch (including else ifeq constructs);
> - it can not be assigned to any more after the endif.
> 
>  In addition, conditions can be nested...
> 
>  The only way I can think of implementing this is by keeping a stack of
> conditionally assigned variables. In the else branch, the top of the stack
> is saved en empties. At the endif, the top of the stack is popped, merged
> with the saved one, and merged into the new top of the stack.
> 
>  Wow, by writing it down, I almost implemented it :-) Let's see if I can
> still find the time to actually do it tonight...

Sorry, but I think I got lost somewhere...

Currently, check-package does not whine for:

    ifeq(...)
    FOO_VAR = ...
    endif

but it does whine for:

    FOO_VAR = ...
    ifeq(...)
    FOO_VAR = ...
    endif

And these two behaviours are, I think, correct.

What I am arguing, is that FOO_DEPENDENCIES should be treated specially,
where check-package should whine for:

    ifeq(...)
    FOO_DEPENDENCIES = ...
    endif

as well as for:

    FOO_DEPENDENCIES = ...
    ifeq(...)
    FOO_DEPENDENCIES = ...
    endif

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 mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] conditional assigment check in check-package [was: Re: [PATCH v2, 2/2] package/knock: add knockd option]

[Arnout Vandecappelle]

On 21/10/2021 22:12, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2021-10-21 20:43 +0200, Arnout Vandecappelle spake thusly:
>> On 18/10/2021 22:18, Yann E. MORIN wrote:
>>> Thomas, All,
>>> On 2021-10-18 22:04 +0200, Thomas Petazzoni spake thusly:
>>>> On Sat, 16 Oct 2021 11:32:34 +0200
>>>> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>>>>> +ifeq ($(BR2_PACKAGE_KNOCK_KNOCKD),y)
>>>>> +KNOCK_DEPENDENCIES = libpcap
>>>>  From commit 97f3ad7af3dd54a15a10aa35786e7fa08cf5e7b1:
>>>>      Repeat after me: "Forcing the value of <pkg>_DEPENDENCIES inside a
>>>>      conditional is the root of all evil."
>>>> Care to repeat after me ? :-)
>>>
>>> OK, I missed that in my review of v1...
>>> /me hides in shame...
>>>
>>> But seriously though: why is that not caught by check-package?
>>>
>>> Because it is not actually an override: the variable is not yet set
>>> there, so it is valid to unconditionally set it.
>>>
>>> check-package should probably recognise that the variable is
>>> <FOO>_DEPENDENCIES and consider that conditional assignment should
>>> still be forbidden in that case.
>>
>>   I had a look at this, and it is not entirely trivial to test for in check-package.
>>
>>   Basically, this is allowed:
>>
>> ifeq(...)
>> FOO_VAR = ...
>> endif
>> ... $(FOO_VAR) ... # Empty if condition not true
>>
>>   And this as well:
>>
>> ifeq(...)
>> FOO_VAR = ...
>> else
>> FOO_VAR = ...
>> endif
>>
>>   But this is not:
>>
>> ifeq(...)
>> FOO_VAR = ...
>> endif
>>
>> ifeq(...)
>> FOO_VAR = ...
>> endif
>>
>>
>>   So basically:
>> - after a variable is assigned, it can't be assigned to any more in the same
>> conditional branch;
>> - it can be assigned to again in the else branch (including else ifeq constructs);
>> - it can not be assigned to any more after the endif.
>>
>>   In addition, conditions can be nested...
>>
>>   The only way I can think of implementing this is by keeping a stack of
>> conditionally assigned variables. In the else branch, the top of the stack
>> is saved en empties. At the endif, the top of the stack is popped, merged
>> with the saved one, and merged into the new top of the stack.
>>
>>   Wow, by writing it down, I almost implemented it :-) Let's see if I can
>> still find the time to actually do it tonight...
> 
> Sorry, but I think I got lost somewhere...
> 
> Currently, check-package does not whine for:
> 
>      ifeq(...)
>      FOO_VAR = ...
>      endif
> 
> but it does whine for:
> 
>      FOO_VAR = ...
>      ifeq(...)
>      FOO_VAR = ...
>      endif
> 
> And these two behaviours are, I think, correct.
> 
> What I am arguing, is that FOO_DEPENDENCIES should be treated specially,
> where check-package should whine for:
> 
>      ifeq(...)
>      FOO_DEPENDENCIES = ...
>      endif
> 
> as well as for:
> 
>      FOO_DEPENDENCIES = ...
>      ifeq(...)
>      FOO_DEPENDENCIES = ...
>      endif

  Yeah, special-casing DEPENDENCIES is definitely an option.

  My "wanted behaviour" was for other variables which can have the same problem. 
Probably doesn't happen very often though.

  Regards,
  Arnout

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot