diff mbox series

[1/1] build: restore missing libwpa_client.so

Message ID 20220219192558.3440228-1-geomatsi@gmail.com
State Superseded, archived
Headers show
Series [1/1] build: restore missing libwpa_client.so | expand

Commit Message

Sergey Matyukevich Feb. 19, 2022, 7:25 p.m. UTC
Commit a41a29192e5d ("build: Pull common fragments into a build.rules
file") introduced regression into wpa_supplicant build process. Build
target libwpa_client.so is not built regardless of whether the option
CONFIG_BUILD_WPA_CLIENT_SO is set or not. This happens because config
option is used before it is imported from the configuration file.
Moving its usage after including build.rules does not help: variable
ALL is processed by build.rules and further changes are not applied.
This commit moves imports of hostapd/wpa_s configuration files from
build.rules back into Makefiles.

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 hostapd/Makefile        | 7 ++++++-
 src/build.rules         | 1 -
 wpa_supplicant/Makefile | 8 +++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

Comments

Johannes Berg Feb. 19, 2022, 8:05 p.m. UTC | #1
> Commit a41a29192e5d ("build: Pull common fragments into a build.rules
> file") introduced regression into wpa_supplicant build process. Build
> target libwpa_client.so is not built regardless of whether the option
> CONFIG_BUILD_WPA_CLIENT_SO is set or not. This happens because config
> option is used before it is imported from the configuration file.

Hmm, oops.

> Moving its usage after including build.rules does not help: variable
> ALL is processed by build.rules and further changes are not applied.

Right.

> This commit moves imports of hostapd/wpa_s configuration files from
> build.rules back into Makefiles.

That's kind of annoying. What do you think of this instead? It makes it
rely on the "_all" internal, but still feels somewhat better to me.

johannes


--- a/wpa_supplicant/Makefile
+++ b/wpa_supplicant/Makefile
@@ -10,15 +10,18 @@ ALL += systemd/wpa_supplicant@.service
 ALL += systemd/wpa_supplicant-nl80211@.service
 ALL += systemd/wpa_supplicant-wired@.service
 ALL += dbus/fi.w1.wpa_supplicant1.service
-ifdef CONFIG_BUILD_WPA_CLIENT_SO
-ALL += libwpa_client.so
-endif
 
 EXTRA_TARGETS=dynamic_eap_methods
 
 CONFIG_FILE=.config
 include ../src/build.rules
 
+ifdef CONFIG_BUILD_WPA_CLIENT_SO
+# add the dependency this way to allow CONFIG_BUILD_WPA_CLIENT_SO
+# being set in the config which is read by build.rules
+_all: libwpa_client.so
+endif
+
 ifdef LIBS
 # If LIBS is set with some global build system defaults, clone those for
 # LIBS_c and LIBS_p to cover wpa_passphrase and wpa_cli as well.


johannes
Sergey Matyukevich Feb. 19, 2022, 8:20 p.m. UTC | #2
Hello Johannes,

> > Commit a41a29192e5d ("build: Pull common fragments into a build.rules
> > file") introduced regression into wpa_supplicant build process. Build
> > target libwpa_client.so is not built regardless of whether the option
> > CONFIG_BUILD_WPA_CLIENT_SO is set or not. This happens because config
> > option is used before it is imported from the configuration file.
> 
> Hmm, oops.
> 
> > Moving its usage after including build.rules does not help: variable
> > ALL is processed by build.rules and further changes are not applied.
> 
> Right.
> 
> > This commit moves imports of hostapd/wpa_s configuration files from
> > build.rules back into Makefiles.
> 
> That's kind of annoying. What do you think of this instead? It makes it
> rely on the "_all" internal, but still feels somewhat better to me.
> 
> johannes
> 
> 
> --- a/wpa_supplicant/Makefile
> +++ b/wpa_supplicant/Makefile
> @@ -10,15 +10,18 @@ ALL += systemd/wpa_supplicant@.service
>  ALL += systemd/wpa_supplicant-nl80211@.service
>  ALL += systemd/wpa_supplicant-wired@.service
>  ALL += dbus/fi.w1.wpa_supplicant1.service
> -ifdef CONFIG_BUILD_WPA_CLIENT_SO
> -ALL += libwpa_client.so
> -endif
>  
>  EXTRA_TARGETS=dynamic_eap_methods
>  
>  CONFIG_FILE=.config
>  include ../src/build.rules
>  
> +ifdef CONFIG_BUILD_WPA_CLIENT_SO
> +# add the dependency this way to allow CONFIG_BUILD_WPA_CLIENT_SO
> +# being set in the config which is read by build.rules
> +_all: libwpa_client.so
> +endif
> +
>  ifdef LIBS
>  # If LIBS is set with some global build system defaults, clone those for
>  # LIBS_c and LIBS_p to cover wpa_passphrase and wpa_cli as well.

I am fine with this approach. But it looks like my commit message
is incomplete and wpa_passphrase is also impacted. So we should
have something like that:


diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile
index cb66defac..92ba3eace 100644
--- a/wpa_supplicant/Makefile
+++ b/wpa_supplicant/Makefile
@@ -1,24 +1,27 @@
 BINALL=wpa_supplicant wpa_cli
 
-ifndef CONFIG_NO_WPA_PASSPHRASE
-BINALL += wpa_passphrase
-endif
-
 ALL = $(BINALL)
 ALL += systemd/wpa_supplicant.service
 ALL += systemd/wpa_supplicant@.service
 ALL += systemd/wpa_supplicant-nl80211@.service
 ALL += systemd/wpa_supplicant-wired@.service
 ALL += dbus/fi.w1.wpa_supplicant1.service
-ifdef CONFIG_BUILD_WPA_CLIENT_SO
-ALL += libwpa_client.so
-endif
 
 EXTRA_TARGETS=dynamic_eap_methods
 
 CONFIG_FILE=.config
 include ../src/build.rules
 
+ifdef CONFIG_BUILD_WPA_CLIENT_SO
+# add the dependency this way to allow CONFIG_BUILD_WPA_CLIENT_SO
+# being set in the config which is read by build.rules
+_all: libwpa_client.so
+endif
+
+ifndef CONFIG_NO_WPA_PASSPHRASE
+_all: wpa_passphrase
+endif
+
 ifdef LIBS
 # If LIBS is set with some global build system defaults, clone those for
 # LIBS_c and LIBS_p to cover wpa_passphrase and wpa_cli as well.


Regards,
Sergey
diff mbox series

Patch

diff --git a/hostapd/Makefile b/hostapd/Makefile
index e37c13b27..7570d567a 100644
--- a/hostapd/Makefile
+++ b/hostapd/Makefile
@@ -1,6 +1,11 @@ 
-ALL=hostapd hostapd_cli
 CONFIG_FILE = .config
 
+ifneq ($(CONFIG_FILE),)
+-include $(CONFIG_FILE)
+endif
+
+ALL=hostapd hostapd_cli
+
 include ../src/build.rules
 
 ifdef LIBS
diff --git a/src/build.rules b/src/build.rules
index acda88472..e22f5d538 100644
--- a/src/build.rules
+++ b/src/build.rules
@@ -35,7 +35,6 @@  CFLAGS = -MMD -O2 -Wall -g
 endif
 
 ifneq ($(CONFIG_FILE),)
--include $(CONFIG_FILE)
 
 # export for sub-makefiles
 export CONFIG_CODE_COVERAGE
diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile
index cb66defac..21fe92bda 100644
--- a/wpa_supplicant/Makefile
+++ b/wpa_supplicant/Makefile
@@ -1,3 +1,9 @@ 
+CONFIG_FILE = .config
+
+ifneq ($(CONFIG_FILE),)
+-include $(CONFIG_FILE)
+endif
+
 BINALL=wpa_supplicant wpa_cli
 
 ifndef CONFIG_NO_WPA_PASSPHRASE
@@ -10,13 +16,13 @@  ALL += systemd/wpa_supplicant@.service
 ALL += systemd/wpa_supplicant-nl80211@.service
 ALL += systemd/wpa_supplicant-wired@.service
 ALL += dbus/fi.w1.wpa_supplicant1.service
+
 ifdef CONFIG_BUILD_WPA_CLIENT_SO
 ALL += libwpa_client.so
 endif
 
 EXTRA_TARGETS=dynamic_eap_methods
 
-CONFIG_FILE=.config
 include ../src/build.rules
 
 ifdef LIBS