All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [v1 0/2] add bootpc package
@ 2023-05-23 17:21 Colin Foster
  2023-05-23 17:21 ` [Buildroot] [v1 1/2] package/bootpc: new package Colin Foster
  2023-05-23 17:21 ` [Buildroot] [v1 2/2] bootpc: add dhcp-like configuration ability Colin Foster
  0 siblings, 2 replies; 8+ messages in thread
From: Colin Foster @ 2023-05-23 17:21 UTC (permalink / raw)
  To: buildroot; +Cc: Thomas Petazzoni

Believe it or not, there are use cases for running BOOTPC instead of
DHCP or static IP addressing.

The /etc/network/interfaces configuration file allows for the
configuration option

```
iface eth0 inet bootp
```

However, this relies on two things. The first is the existence of the
bootpc application, which is added in patch 1.

That doesn't solve all the problems, however. `bootpc` only works with
interfaces that are up, which leads to a chicken-or-egg problem. Patch 2
utilizes an ancient technique to temporarily bring up an interface
before running the actual bootpc application.

The end result is an interface with the above configuration that
successfully gets an IP address and netmask from a BOOTP server.


Colin Foster (2):
  package/bootpc: new package
  bootpc: add dhcp-like configuration ability

 package/Config.in                             |  1 +
 package/bootpc/0001-main-fix-build.patch      | 37 ++++++++
 ...Makefile-remove-strip-from-the-build.patch | 28 ++++++
 package/bootpc/Config.in                      | 20 +++++
 package/bootpc/bootpc.mk                      | 35 ++++++++
 package/bootpc/bootpc_dhcp_script             | 86 +++++++++++++++++++
 6 files changed, 207 insertions(+)
 create mode 100644 package/bootpc/0001-main-fix-build.patch
 create mode 100644 package/bootpc/0002-Makefile-remove-strip-from-the-build.patch
 create mode 100644 package/bootpc/Config.in
 create mode 100644 package/bootpc/bootpc.mk
 create mode 100644 package/bootpc/bootpc_dhcp_script

-- 
2.25.1

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [Buildroot] [v1 1/2] package/bootpc: new package
  2023-05-23 17:21 [Buildroot] [v1 0/2] add bootpc package Colin Foster
@ 2023-05-23 17:21 ` Colin Foster
  2023-05-23 20:43   ` Yann E. MORIN
  2023-05-23 17:21 ` [Buildroot] [v1 2/2] bootpc: add dhcp-like configuration ability Colin Foster
  1 sibling, 1 reply; 8+ messages in thread
From: Colin Foster @ 2023-05-23 17:21 UTC (permalink / raw)
  To: buildroot; +Cc: Thomas Petazzoni

Add the ability to select, build, and run bootpc - a client program for
BOOTP requests.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 package/Config.in                             |  1 +
 package/bootpc/0001-main-fix-build.patch      | 37 +++++++++++++++++++
 ...Makefile-remove-strip-from-the-build.patch | 28 ++++++++++++++
 package/bootpc/Config.in                      |  5 +++
 package/bootpc/bootpc.mk                      | 19 ++++++++++
 5 files changed, 90 insertions(+)
 create mode 100644 package/bootpc/0001-main-fix-build.patch
 create mode 100644 package/bootpc/0002-Makefile-remove-strip-from-the-build.patch
 create mode 100644 package/bootpc/Config.in
 create mode 100644 package/bootpc/bootpc.mk

diff --git a/package/Config.in b/package/Config.in
index 420ebaa370..07de6500bf 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2273,6 +2273,7 @@ endif
 	source "package/bmon/Config.in"
 	source "package/bmx7/Config.in"
 	source "package/boinc/Config.in"
+    source "package/bootpc/Config.in"
 	source "package/brcm-patchram-plus/Config.in"
 	source "package/bridge-utils/Config.in"
 	source "package/bwm-ng/Config.in"
diff --git a/package/bootpc/0001-main-fix-build.patch b/package/bootpc/0001-main-fix-build.patch
new file mode 100644
index 0000000000..cf4ad66e47
--- /dev/null
+++ b/package/bootpc/0001-main-fix-build.patch
@@ -0,0 +1,37 @@
+From 349164d3db2210fb51e3784dd1afbe2b120f32d6 Mon Sep 17 00:00:00 2001
+From: Colin Foster <colin.foster@in-advantage.com>
+Date: Mon, 20 Mar 2023 11:18:13 -0700
+Subject: [PATCH 1/2] main: fix build
+
+---
+ main.c | 8 +++-----
+ 1 file changed, 3 insertions(+), 5 deletions(-)
+
+diff --git a/main.c b/main.c
+index 6774cf1..0aa3767 100644
+--- a/main.c
++++ b/main.c
+@@ -21,8 +21,6 @@
+ 
+ #include "bootpc.h"
+ 
+-FILE * logfile = stderr ;
+-
+ int main(int argc,
+ 	 char *argv[])
+ {
+@@ -159,8 +157,8 @@ void doLogMessage(char * s, ...) {
+     va_list args;
+ 
+     va_start(args, s);
+-    fprintf(logfile, "* ");
+-    vfprintf(logfile, s, args);
+-    fprintf(logfile, "\n");
++    fprintf(stderr, "* ");
++    vfprintf(stderr, s, args);
++    fprintf(stderr, "\n");
+     va_end(args);
+ }
+-- 
+2.25.1
+
diff --git a/package/bootpc/0002-Makefile-remove-strip-from-the-build.patch b/package/bootpc/0002-Makefile-remove-strip-from-the-build.patch
new file mode 100644
index 0000000000..1f5edab2eb
--- /dev/null
+++ b/package/bootpc/0002-Makefile-remove-strip-from-the-build.patch
@@ -0,0 +1,28 @@
+From a58a27b82d2d822e338b46ce6eb270f121a8905f Mon Sep 17 00:00:00 2001
+From: Colin Foster <colin.foster@in-advantage.com>
+Date: Mon, 20 Mar 2023 11:35:12 -0700
+Subject: [PATCH 2/2] Makefile: remove strip from the build
+
+---
+ Makefile | 2 --
+ 1 file changed, 2 deletions(-)
+
+diff --git a/Makefile b/Makefile
+index 4189648..54562ba 100644
+--- a/Makefile
++++ b/Makefile
+@@ -33,11 +33,9 @@ clean:
+ 
+ ${PROG}: ${OBJS}
+ 	${CC} -o ${PROG} ${CFLAGS} ${OBJS}
+-	strip ${PROG}
+ 
+ ${PROG}-static: ${OBJS}
+ 	${CC} -static -o ${PROG} ${CFLAGS} ${OBJS}
+-	strip ${PROG}
+ 
+ # Make up a distribution file from the bits we want people to see
+ distrib:  $(CFILES) $(CHEADS) $(MISC)
+-- 
+2.25.1
+
diff --git a/package/bootpc/Config.in b/package/bootpc/Config.in
new file mode 100644
index 0000000000..2936142b7f
--- /dev/null
+++ b/package/bootpc/Config.in
@@ -0,0 +1,5 @@
+config BR2_PACKAGE_BOOTPC
+	bool "bootpc"
+	help
+		Application to perform BOOTP
+
diff --git a/package/bootpc/bootpc.mk b/package/bootpc/bootpc.mk
new file mode 100644
index 0000000000..d3c5ce5198
--- /dev/null
+++ b/package/bootpc/bootpc.mk
@@ -0,0 +1,19 @@
+################################################################################
+#
+# bootpc package
+#
+################################################################################
+
+BOOTPC_VERSION = 80c0811dc69f9f3923661be4f9f9c09a44313f62
+BOOTPC_SITE_METHOD = git
+BOOTPC_SITE = https://salsa.debian.org/debian/bootpc.git
+
+define BOOTPC_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) all
+endef
+
+define BOOTPC_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/bootpc $(TARGET_DIR)/sbin
+endef
+
+$(eval $(generic-package))
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [Buildroot] [v1 2/2] bootpc: add dhcp-like configuration ability
  2023-05-23 17:21 [Buildroot] [v1 0/2] add bootpc package Colin Foster
  2023-05-23 17:21 ` [Buildroot] [v1 1/2] package/bootpc: new package Colin Foster
@ 2023-05-23 17:21 ` Colin Foster
  2023-05-23 21:18   ` Yann E. MORIN
  1 sibling, 1 reply; 8+ messages in thread
From: Colin Foster @ 2023-05-23 17:21 UTC (permalink / raw)
  To: buildroot; +Cc: Thomas Petazzoni

bootpc only works on interfaces that are up. If BOOTP is desired to get
an IP address to a running kernel, bootpc won't work.

This scenario was brought up as a bug in Debian forums [1] in 2007. The
response was to run the attached script to bring up the network without
an IP address. Sure enough, it still works!

Implement this bootpc script to allow IP configuration by way of
"iface eth0 inet bootp" in /etc/network/interfaces.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=436443

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 package/bootpc/Config.in          | 15 ++++++
 package/bootpc/bootpc.mk          | 20 ++++++-
 package/bootpc/bootpc_dhcp_script | 86 +++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 2 deletions(-)
 create mode 100644 package/bootpc/bootpc_dhcp_script

diff --git a/package/bootpc/Config.in b/package/bootpc/Config.in
index 2936142b7f..fd3cd58d40 100644
--- a/package/bootpc/Config.in
+++ b/package/bootpc/Config.in
@@ -3,3 +3,18 @@ config BR2_PACKAGE_BOOTPC
 	help
 		Application to perform BOOTP
 
+if BR2_PACKAGE_BOOTPC
+
+config BR2_PACKAGE_BOOTPC_FULL_CONFIG
+	bool "bootpc interface configuration script"
+	help
+		BOOTP can be used to fully configure a network interface in a
+		similar manner to DHCP. bootpc requires the network interface to
+		be configured before this can happen.
+
+		Replace the bootpc binary with a script that allows an
+		unconfigured network interface to work as a BOOTP client. The
+		setting can be applied with "iface eth0 inet bootp" in
+		/etc/network/interfaces.
+
+endif
diff --git a/package/bootpc/bootpc.mk b/package/bootpc/bootpc.mk
index d3c5ce5198..ebd7490ea1 100644
--- a/package/bootpc/bootpc.mk
+++ b/package/bootpc/bootpc.mk
@@ -8,12 +8,28 @@ BOOTPC_VERSION = 80c0811dc69f9f3923661be4f9f9c09a44313f62
 BOOTPC_SITE_METHOD = git
 BOOTPC_SITE = https://salsa.debian.org/debian/bootpc.git
 
+ifeq ($(BR2_PACKAGE_BOOTPC_FULL_CONFIG),y)
+BOOTPC_TARGET_BIN_NAME = bootpc-bin
+else
+BOOTPC_TARGET_BIN_NAME = bootpc
+endif
+
 define BOOTPC_BUILD_CMDS
-	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) all
+	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) PROG=$(BOOTPC_TARGET_BIN_NAME) -C $(@D) all
+endef
+
+ifeq ($(BR2_PACKAGE_BOOTPC_FULL_CONFIG),y)
+define BOOTPC_INSTALL_CONFIG_SCRIPT
+	$(INSTALL) -m 0755 $(BR2_EXTERNAL_BR2INADVANTAGE_PATH)/package/bootpc/bootpc_dhcp_script $(TARGET_DIR)/sbin/bootpc
+endef
+else
+define BOOTPC_INSTALL_CONFIG_SCRIPT ""
 endef
+endif
 
 define BOOTPC_INSTALL_TARGET_CMDS
-	$(INSTALL) -D -m 0755 $(@D)/bootpc $(TARGET_DIR)/sbin
+	$(INSTALL) -D -m 0755 $(@D)/$(BOOTPC_TARGET_BIN_NAME) $(TARGET_DIR)/sbin
+	$(BOOTPC_INSTALL_CONFIG_SCRIPT)
 endef
 
 $(eval $(generic-package))
diff --git a/package/bootpc/bootpc_dhcp_script b/package/bootpc/bootpc_dhcp_script
new file mode 100644
index 0000000000..c7de8280f1
--- /dev/null
+++ b/package/bootpc/bootpc_dhcp_script
@@ -0,0 +1,86 @@
+#!/bin/sh
+#
+# script to initialize a network interface using bootp
+#
+# Copyright (C) 2008 by Fuji Xerox Co., Ltd. All rights reserved.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# written by Kohei Tanaka <Kohei.Tanaka@fujixerox.co.jp>
+
+INTERFACE=eth0
+
+# load configuration file
+[ -f /etc/default/bootpc ] && . /etc/default/bootpc
+
+# to get the interface looping over arguments
+while [ $# -ne 0 ]
+do
+	case $1 in
+	--dev)	INTERFACE=$2
+		break;;
+	*)	shift
+		continue;;
+	esac
+done
+	
+# up the interface for sending broadcast packet
+ifconfig $INTERFACE up 0.0.0.0
+route add default dev $INTERFACE
+
+TMPFILE=`mktemp /tmp/bootpc.XXXXXX`
+
+/sbin/bootpc-bin --dev $INTERFACE --returniffail --serverbcast > $TMPFILE
+RET=$?
+route del default
+ifconfig $INTERFACE down
+if [ $RET -ne 0 ]
+then
+	rm -rf $TMPFILE
+	exit 1
+fi
+
+. $TMPFILE
+rm -rf $TMPFILE
+
+[ -z "$IPADDR" ] && exit 1
+
+OPT_NETMASK=""
+OPT_BROADCAST=""
+if [ -n "$NETMASK" ]
+then
+	OPT_NETMASK="netmask $NETMASK"
+fi
+if [ -n "$BROADCAST" ]
+then
+	OPT_BROADCAST="broadcast $BROADCAST"
+fi
+ifconfig $INTERFACE $IPADDR $OPT_NETMASK $OPT_BROADCAST
+
+# already added a route to the network.  just add default gateway.
+for i in $GATEWAYS
+do
+	route add default gw $i
+done
+
+[ x"$SETDNS" == xno ] && exit 0
+if [ -n "$DOMAIN" ] || [ -n "$DNSSRVS" ]
+then
+	mv -f /etc/resolv.conf /etc/resolv.conf.old
+	if [ -n "$DOMAIN" ]
+	then
+		echo "search $DOMAIN" >> /etc/resolv.conf
+	fi
+	for i in $DNSSRVS
+	do
+		echo "nameserver $i" >> /etc/resolv.conf
+	done
+fi
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [Buildroot] [v1 1/2] package/bootpc: new package
  2023-05-23 17:21 ` [Buildroot] [v1 1/2] package/bootpc: new package Colin Foster
@ 2023-05-23 20:43   ` Yann E. MORIN
  2023-05-24 17:49     ` Colin Foster
  0 siblings, 1 reply; 8+ messages in thread
From: Yann E. MORIN @ 2023-05-23 20:43 UTC (permalink / raw)
  To: Colin Foster; +Cc: Thomas Petazzoni, buildroot

Colin, All,

On 2023-05-23 10:21 -0700, Colin Foster spake thusly:
> Add the ability to select, build, and run bootpc - a client program for
> BOOTP requests.

Thanks for this new package.

Here is a purely mechanical review:

    $ ./utils/docker-run make check-package
    package/bootpc/0001-main-fix-build.patch:4: generate your patches with 'git format-patch -N'
    package/bootpc/0001-main-fix-build.patch:0: missing Signed-off-by in the header (http://nightly.buildroot.org/#_format_and_licensing_of_the_package_patches)
    package/bootpc/0001-main-fix-build.patch:0: missing Upstream in the header (http://nightly.buildroot.org/#_additional_patch_documentation)
    package/bootpc/0002-Makefile-remove-strip-from-the-build.patch:4: generate your patches with 'git format-patch -N'
    package/bootpc/0002-Makefile-remove-strip-from-the-build.patch:0: missing Signed-off-by in the header (http://nightly.buildroot.org/#_format_and_licensing_of_the_package_patches)
    package/bootpc/0002-Makefile-remove-strip-from-the-build.patch:0: missing Upstream in the header (http://nightly.buildroot.org/#_additional_patch_documentation)
    package/bootpc/Config.in:4: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
    package/bootpc/Config.in:5: empty line at end of file

Also, see below for a few additional comments...

> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> ---
>  package/Config.in                             |  1 +
>  package/bootpc/0001-main-fix-build.patch      | 37 +++++++++++++++++++
>  ...Makefile-remove-strip-from-the-build.patch | 28 ++++++++++++++
>  package/bootpc/Config.in                      |  5 +++
>  package/bootpc/bootpc.mk                      | 19 ++++++++++
>  5 files changed, 90 insertions(+)
>  create mode 100644 package/bootpc/0001-main-fix-build.patch
>  create mode 100644 package/bootpc/0002-Makefile-remove-strip-from-the-build.patch
>  create mode 100644 package/bootpc/Config.in
>  create mode 100644 package/bootpc/bootpc.mk
> 
> diff --git a/package/Config.in b/package/Config.in
> index 420ebaa370..07de6500bf 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -2273,6 +2273,7 @@ endif
>  	source "package/bmon/Config.in"
>  	source "package/bmx7/Config.in"
>  	source "package/boinc/Config.in"
> +    source "package/bootpc/Config.in"

Indentation with TAB, not spaces.

(not sure why this one did not get caught by check-package...)

[--SNIP--]
> diff --git a/package/bootpc/bootpc.mk b/package/bootpc/bootpc.mk
> new file mode 100644
> index 0000000000..d3c5ce5198
> --- /dev/null
> +++ b/package/bootpc/bootpc.mk
> @@ -0,0 +1,19 @@
> +################################################################################
> +#
> +# bootpc package
> +#
> +################################################################################
> +
> +BOOTPC_VERSION = 80c0811dc69f9f3923661be4f9f9c09a44313f62
> +BOOTPC_SITE_METHOD = git
> +BOOTPC_SITE = https://salsa.debian.org/debian/bootpc.git

Nit-picking, but we usually put _SITE and _SITE_METHOD in that order:

    BOOTPC_SITE = https://salsa.debian.org/debian/bootpc.git
    BOOTPC_SITE_METHOD = git

You need to also provide the licening terms. It looks like:

    BOOTPC_LICENSE = GPL-2.0-or-later
    BOOTPC_LICENSE_FILES = LICENSE NOTICE

Also, please provide a hash file with hashes for the source tarball [0]
and for each license files; see:
    https://buildroot.org/downloads/manual/manual.html#adding-packages-hash

[0] source tarballs generated from a git checkout have been reproducible
for quite some time now, so we can, and have to, check their hashes.

Also, the upstream README states:

    I'll repeat that.  set up /etc/host.conf or this bit won't work.

We do not provide such a file in Buildroot, so:
 1. is the comment above still valide nowadays?
 2. should the bootpc package install an /etc/host.conf file?

Regards,
Yann E. MORIN.

> +define BOOTPC_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) all
> +endef
> +
> +define BOOTPC_INSTALL_TARGET_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/bootpc $(TARGET_DIR)/sbin
> +endef
> +
> +$(eval $(generic-package))
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Buildroot] [v1 2/2] bootpc: add dhcp-like configuration ability
  2023-05-23 17:21 ` [Buildroot] [v1 2/2] bootpc: add dhcp-like configuration ability Colin Foster
@ 2023-05-23 21:18   ` Yann E. MORIN
  2023-05-24 17:58     ` Colin Foster
  0 siblings, 1 reply; 8+ messages in thread
From: Yann E. MORIN @ 2023-05-23 21:18 UTC (permalink / raw)
  To: Colin Foster; +Cc: Thomas Petazzoni, buildroot

Colin, All,

On 2023-05-23 10:21 -0700, Colin Foster spake thusly:
> bootpc only works on interfaces that are up. If BOOTP is desired to get
> an IP address to a running kernel, bootpc won't work.
> 
> This scenario was brought up as a bug in Debian forums [1] in 2007. The
> response was to run the attached script to bring up the network without
> an IP address. Sure enough, it still works!
> 
> Implement this bootpc script to allow IP configuration by way of
> "iface eth0 inet bootp" in /etc/network/interfaces.

Can't one just use a pre-up command ?

    iface eth0 inet bootp
        pre-up ip l set dev eth0 up
        pre-up ip r add default dev eth0
        post-up ip r del default dev eth0

Not sure that would work, but maybe worth a try?

> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=436443
> 
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>

Ditto as for patch 1/2: run check-package.

[--SNIP--]
> diff --git a/package/bootpc/bootpc.mk b/package/bootpc/bootpc.mk
> index d3c5ce5198..ebd7490ea1 100644
> --- a/package/bootpc/bootpc.mk
> +++ b/package/bootpc/bootpc.mk
> @@ -8,12 +8,28 @@ BOOTPC_VERSION = 80c0811dc69f9f3923661be4f9f9c09a44313f62
>  BOOTPC_SITE_METHOD = git
>  BOOTPC_SITE = https://salsa.debian.org/debian/bootpc.git
>  
> +ifeq ($(BR2_PACKAGE_BOOTPC_FULL_CONFIG),y)
> +BOOTPC_TARGET_BIN_NAME = bootpc-bin
> +else
> +BOOTPC_TARGET_BIN_NAME = bootpc
> +endif
> +
>  define BOOTPC_BUILD_CMDS
> -	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) all
> +	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) PROG=$(BOOTPC_TARGET_BIN_NAME) -C $(@D) all

Please keep lines below the ~80 char limit, e.g.:

        $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) \
            -C $(@D) \
            PROG=$(BOOTPC_TARGET_BIN_NAME) \
            all

(I like to see '-C $(@D)' to be the first thing right after the call to
make).

> +endef
> +
> +ifeq ($(BR2_PACKAGE_BOOTPC_FULL_CONFIG),y)
> +define BOOTPC_INSTALL_CONFIG_SCRIPT
> +	$(INSTALL) -m 0755 $(BR2_EXTERNAL_BR2INADVANTAGE_PATH)/package/bootpc/bootpc_dhcp_script $(TARGET_DIR)/sbin/bootpc
> +endef
> +else
> +define BOOTPC_INSTALL_CONFIG_SCRIPT ""
>  endef

No need to define an empty macro.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Buildroot] [v1 1/2] package/bootpc: new package
  2023-05-23 20:43   ` Yann E. MORIN
@ 2023-05-24 17:49     ` Colin Foster
  0 siblings, 0 replies; 8+ messages in thread
From: Colin Foster @ 2023-05-24 17:49 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: Thomas Petazzoni, buildroot

Hi Yann,

On Tue, May 23, 2023 at 10:43:35PM +0200, Yann E. MORIN wrote:
> Colin, All,
> 
> On 2023-05-23 10:21 -0700, Colin Foster spake thusly:
> > Add the ability to select, build, and run bootpc - a client program for
> > BOOTP requests.
> 
> Thanks for this new package.
> 
> Here is a purely mechanical review:
> 
>     $ ./utils/docker-run make check-package
>     package/bootpc/0001-main-fix-build.patch:4: generate your patches with 'git format-patch -N'
>     package/bootpc/0001-main-fix-build.patch:0: missing Signed-off-by in the header (http://nightly.buildroot.org/#_format_and_licensing_of_the_package_patches)
>     package/bootpc/0001-main-fix-build.patch:0: missing Upstream in the header (http://nightly.buildroot.org/#_additional_patch_documentation)
>     package/bootpc/0002-Makefile-remove-strip-from-the-build.patch:4: generate your patches with 'git format-patch -N'
>     package/bootpc/0002-Makefile-remove-strip-from-the-build.patch:0: missing Signed-off-by in the header (http://nightly.buildroot.org/#_format_and_licensing_of_the_package_patches)
>     package/bootpc/0002-Makefile-remove-strip-from-the-build.patch:0: missing Upstream in the header (http://nightly.buildroot.org/#_additional_patch_documentation)
>     package/bootpc/Config.in:4: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
>     package/bootpc/Config.in:5: empty line at end of file

I didn't know about all these tools. I'll run them before v2, and thanks
for the feedback.

> 
> Also, see below for a few additional comments...
> 
> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> > ---
> >  package/Config.in                             |  1 +
> >  package/bootpc/0001-main-fix-build.patch      | 37 +++++++++++++++++++
> >  ...Makefile-remove-strip-from-the-build.patch | 28 ++++++++++++++
> >  package/bootpc/Config.in                      |  5 +++
> >  package/bootpc/bootpc.mk                      | 19 ++++++++++
> >  5 files changed, 90 insertions(+)
> >  create mode 100644 package/bootpc/0001-main-fix-build.patch
> >  create mode 100644 package/bootpc/0002-Makefile-remove-strip-from-the-build.patch
> >  create mode 100644 package/bootpc/Config.in
> >  create mode 100644 package/bootpc/bootpc.mk
> > 
> > diff --git a/package/Config.in b/package/Config.in
> > index 420ebaa370..07de6500bf 100644
> > --- a/package/Config.in
> > +++ b/package/Config.in
> > @@ -2273,6 +2273,7 @@ endif
> >  	source "package/bmon/Config.in"
> >  	source "package/bmx7/Config.in"
> >  	source "package/boinc/Config.in"
> > +    source "package/bootpc/Config.in"
> 
> Indentation with TAB, not spaces.
> 
> (not sure why this one did not get caught by check-package...)

Thanks. I'll fix this up as well. Boneheaded mistake on my part.

> 
> [--SNIP--]
> > diff --git a/package/bootpc/bootpc.mk b/package/bootpc/bootpc.mk
> > new file mode 100644
> > index 0000000000..d3c5ce5198
> > --- /dev/null
> > +++ b/package/bootpc/bootpc.mk
> > @@ -0,0 +1,19 @@
> > +################################################################################
> > +#
> > +# bootpc package
> > +#
> > +################################################################################
> > +
> > +BOOTPC_VERSION = 80c0811dc69f9f3923661be4f9f9c09a44313f62
> > +BOOTPC_SITE_METHOD = git
> > +BOOTPC_SITE = https://salsa.debian.org/debian/bootpc.git
> 
> Nit-picking, but we usually put _SITE and _SITE_METHOD in that order:
> 
>     BOOTPC_SITE = https://salsa.debian.org/debian/bootpc.git
>     BOOTPC_SITE_METHOD = git
> 
> You need to also provide the licening terms. It looks like:
> 
>     BOOTPC_LICENSE = GPL-2.0-or-later
>     BOOTPC_LICENSE_FILES = LICENSE NOTICE
> 
> Also, please provide a hash file with hashes for the source tarball [0]
> and for each license files; see:
>     https://buildroot.org/downloads/manual/manual.html#adding-packages-hash

Ahh... I poked around readme, developers... I didn't even think to check
the manual. Apologies.

> 
> [0] source tarballs generated from a git checkout have been reproducible
> for quite some time now, so we can, and have to, check their hashes.
> 
> Also, the upstream README states:
> 
>     I'll repeat that.  set up /etc/host.conf or this bit won't work.
> 
> We do not provide such a file in Buildroot, so:
>  1. is the comment above still valide nowadays?
>  2. should the bootpc package install an /etc/host.conf file?

I fought bootpc for a while to get it to work. I don't think the comment
is valid, as I certainly don't have a host.conf file. It seemed like a
red herring. There might be some use cases that need it (comments
surrounding gethostbyaddr suggest that might be the case) but I don't
need it.

> 
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Buildroot] [v1 2/2] bootpc: add dhcp-like configuration ability
  2023-05-23 21:18   ` Yann E. MORIN
@ 2023-05-24 17:58     ` Colin Foster
  2023-05-25 15:31       ` Colin Foster
  0 siblings, 1 reply; 8+ messages in thread
From: Colin Foster @ 2023-05-24 17:58 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: Thomas Petazzoni, buildroot

Hi Yann,

On Tue, May 23, 2023 at 11:18:51PM +0200, Yann E. MORIN wrote:
> Colin, All,
> 
> On 2023-05-23 10:21 -0700, Colin Foster spake thusly:
> > bootpc only works on interfaces that are up. If BOOTP is desired to get
> > an IP address to a running kernel, bootpc won't work.
> > 
> > This scenario was brought up as a bug in Debian forums [1] in 2007. The
> > response was to run the attached script to bring up the network without
> > an IP address. Sure enough, it still works!
> > 
> > Implement this bootpc script to allow IP configuration by way of
> > "iface eth0 inet bootp" in /etc/network/interfaces.
> 
> Can't one just use a pre-up command ?
> 
>     iface eth0 inet bootp
>         pre-up ip l set dev eth0 up
>         pre-up ip r add default dev eth0
>         post-up ip r del default dev eth0
> 
> Not sure that would work, but maybe worth a try?

I'll give it a shot. I tried a bunch of pre-up / post-up commands before
I found the script, but can't say I tried exactly this. I fully agree
though, if the script + rename implementation isn't needed, it should be
avoided.

> 
> Ditto as for patch 1/2: run check-package.

Apologies again. I'll clean up v2 and appreciate the feedback! And I'm
ack-ing everything you said below.

> 
> [--SNIP--]
> > diff --git a/package/bootpc/bootpc.mk b/package/bootpc/bootpc.mk
> > index d3c5ce5198..ebd7490ea1 100644
> > --- a/package/bootpc/bootpc.mk
> > +++ b/package/bootpc/bootpc.mk
> > @@ -8,12 +8,28 @@ BOOTPC_VERSION = 80c0811dc69f9f3923661be4f9f9c09a44313f62
> >  BOOTPC_SITE_METHOD = git
> >  BOOTPC_SITE = https://salsa.debian.org/debian/bootpc.git
> >  
> > +ifeq ($(BR2_PACKAGE_BOOTPC_FULL_CONFIG),y)
> > +BOOTPC_TARGET_BIN_NAME = bootpc-bin
> > +else
> > +BOOTPC_TARGET_BIN_NAME = bootpc
> > +endif
> > +
> >  define BOOTPC_BUILD_CMDS
> > -	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) all
> > +	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) PROG=$(BOOTPC_TARGET_BIN_NAME) -C $(@D) all
> 
> Please keep lines below the ~80 char limit, e.g.:
> 
>         $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) \
>             -C $(@D) \
>             PROG=$(BOOTPC_TARGET_BIN_NAME) \
>             all
> 
> (I like to see '-C $(@D)' to be the first thing right after the call to
> make).
> 
> > +endef
> > +
> > +ifeq ($(BR2_PACKAGE_BOOTPC_FULL_CONFIG),y)
> > +define BOOTPC_INSTALL_CONFIG_SCRIPT
> > +	$(INSTALL) -m 0755 $(BR2_EXTERNAL_BR2INADVANTAGE_PATH)/package/bootpc/bootpc_dhcp_script $(TARGET_DIR)/sbin/bootpc
> > +endef
> > +else
> > +define BOOTPC_INSTALL_CONFIG_SCRIPT ""
> >  endef
> 
> No need to define an empty macro.
> 
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Buildroot] [v1 2/2] bootpc: add dhcp-like configuration ability
  2023-05-24 17:58     ` Colin Foster
@ 2023-05-25 15:31       ` Colin Foster
  0 siblings, 0 replies; 8+ messages in thread
From: Colin Foster @ 2023-05-25 15:31 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: Thomas Petazzoni, buildroot

On Wed, May 24, 2023 at 10:58:30AM -0700, Colin Foster wrote:
> Hi Yann,
> 
> On Tue, May 23, 2023 at 11:18:51PM +0200, Yann E. MORIN wrote:
> > Colin, All,
> > 
> > On 2023-05-23 10:21 -0700, Colin Foster spake thusly:
> > > bootpc only works on interfaces that are up. If BOOTP is desired to get
> > > an IP address to a running kernel, bootpc won't work.
> > > 
> > > This scenario was brought up as a bug in Debian forums [1] in 2007. The
> > > response was to run the attached script to bring up the network without
> > > an IP address. Sure enough, it still works!
> > > 
> > > Implement this bootpc script to allow IP configuration by way of
> > > "iface eth0 inet bootp" in /etc/network/interfaces.
> > 
> > Can't one just use a pre-up command ?
> > 
> >     iface eth0 inet bootp
> >         pre-up ip l set dev eth0 up
> >         pre-up ip r add default dev eth0
> >         post-up ip r del default dev eth0
> > 
> > Not sure that would work, but maybe worth a try?
> 
> I'll give it a shot. I tried a bunch of pre-up / post-up commands before
> I found the script, but can't say I tried exactly this. I fully agree
> though, if the script + rename implementation isn't needed, it should be
> avoided.
> 

I'll elaborate on this. Yes, the pre-up and post-up scripts definitely
do cause bootpc to run at boot! Unfortunately I still need the `mktemp`
operation to apply the output of bootpc to the actual interface,
switching it to use "ip addr" instead of "ifconfig", because I think use
of the latter is frowned upon.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-05-25 15:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-23 17:21 [Buildroot] [v1 0/2] add bootpc package Colin Foster
2023-05-23 17:21 ` [Buildroot] [v1 1/2] package/bootpc: new package Colin Foster
2023-05-23 20:43   ` Yann E. MORIN
2023-05-24 17:49     ` Colin Foster
2023-05-23 17:21 ` [Buildroot] [v1 2/2] bootpc: add dhcp-like configuration ability Colin Foster
2023-05-23 21:18   ` Yann E. MORIN
2023-05-24 17:58     ` Colin Foster
2023-05-25 15:31       ` Colin Foster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.