diff mbox series

build: target: improve UX of CONFIG_TARGET handling

Message ID 20220329063441.20595-1-ynezz@true.cz
State Superseded
Delegated to: Petr Štetiar
Headers show
Series build: target: improve UX of CONFIG_TARGET handling | expand

Commit Message

Petr Štetiar March 29, 2022, 6:34 a.m. UTC
Make it clear, that for `make kernel_{menu,old}config` it's possible to
use only following values for CONFIG_TARGET variable:

 * env
 * platform
 * subtarget
 * subtarget_platform

This should prevent misuse like `make kernel_menuconfig CONFIG_TARGET=bcm2710` etc.

Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
 include/target.mk | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Piotr Dymacz March 29, 2022, 8:10 a.m. UTC | #1
Hi Petr,

On 29.03.2022 08:34, Petr Štetiar wrote:
> Make it clear, that for `make kernel_{menu,old}config` it's possible to
> use only following values for CONFIG_TARGET variable:
> 
>   * env
>   * platform
>   * subtarget
>   * subtarget_platform

It seems I've been misusing 'target' for a long time and now I'm trying 
to understand how this leads to the same behavior as 'platform' :)

Some recent changes show also one more invalid value: 'generic':
4c5d5c66ca ipq806x: 5:15: refresh config
5e43dd1fa7 kernel: bump 5.10 to 5.10.99

There are references to 'target' on our forum and even on the Wiki (!):

https://openwrt.org/docs/guide-developer/toolchain/use-buildsystem#kernel_configuration_optional

Should we maybe make 'target' as a valid value?
Petr Štetiar March 29, 2022, 10:55 a.m. UTC | #2
Piotr Dymacz <pepe2k@gmail.com> [2022-03-29 10:10:00]:

Hi,

> It seems I've been misusing 'target' for a long time and now I'm trying to
> understand how this leads to the same behavior as 'platform' :)

comments right above that code states following:

 # defaults to subtarget if subtarget exists and target does not
 # defaults to target otherwise

> Should we maybe make 'target' as a valid value?

It's consumed by developers, so I wouldn't waste time and complicate the code
with such band aids for wrong documentation, I would simply fix the
documentation instead.

-- ynezz
Piotr Dymacz March 30, 2022, 1 p.m. UTC | #3
Hi Petr,

On 29.03.2022 12:55, Petr Štetiar wrote:
> Piotr Dymacz <pepe2k@gmail.com> [2022-03-29 10:10:00]:
> 
> Hi,
> 
>> It seems I've been misusing 'target' for a long time and now I'm trying to
>> understand how this leads to the same behavior as 'platform' :)
> 
> comments right above that code states following:
> 
>   # defaults to subtarget if subtarget exists and target does not
>   # defaults to target otherwise
> 
>> Should we maybe make 'target' as a valid value?
> 
> It's consumed by developers, so I wouldn't waste time and complicate the code
> with such band aids for wrong documentation, I would simply fix the
> documentation instead.

My suggestion was only about keeping things with reality. Even the 
comments above, in the code you mentioned, refer to 'target' and 
'subtarget', not 'platform'.

I might be simply wrong here but I believe most of us refer to 'target', 
not 'platform' these days. That was the reason for my idea about 
'{subtarget_}target', while keeping backward compatibility with the 
'platform' but I'm really fine keeping it this way and fixing Wiki.

btw, CONFIG_TARGET=platform|subtarget|env was introduced in2011, in 
caf4747f0c.
diff mbox series

Patch

diff --git a/include/target.mk b/include/target.mk
index 72fe493776b9..444cc032400f 100644
--- a/include/target.mk
+++ b/include/target.mk
@@ -177,18 +177,17 @@  LINUX_RECONFIG_TARGET = $(if $(USE_SUBTARGET_CONFIG),$(LINUX_SUBTARGET_CONFIG),$
 ifeq ($(CONFIG_TARGET),platform)
   LINUX_RECONFIG_LIST = $(wildcard $(GENERIC_LINUX_CONFIG) $(LINUX_TARGET_CONFIG))
   LINUX_RECONFIG_TARGET = $(LINUX_TARGET_CONFIG)
-endif
-ifeq ($(CONFIG_TARGET),subtarget)
+else ifeq ($(CONFIG_TARGET),subtarget)
   LINUX_RECONFIG_LIST = $(wildcard $(GENERIC_LINUX_CONFIG) $(LINUX_TARGET_CONFIG) $(LINUX_SUBTARGET_CONFIG))
   LINUX_RECONFIG_TARGET = $(LINUX_SUBTARGET_CONFIG)
-endif
-ifeq ($(CONFIG_TARGET),subtarget_platform)
+else ifeq ($(CONFIG_TARGET),subtarget_platform)
   LINUX_RECONFIG_LIST = $(wildcard $(GENERIC_LINUX_CONFIG) $(LINUX_SUBTARGET_CONFIG) $(LINUX_TARGET_CONFIG))
   LINUX_RECONFIG_TARGET = $(LINUX_TARGET_CONFIG)
-endif
-ifeq ($(CONFIG_TARGET),env)
+else ifeq ($(CONFIG_TARGET),env)
   LINUX_RECONFIG_LIST = $(LINUX_KCONFIG_LIST)
   LINUX_RECONFIG_TARGET = $(TOPDIR)/env/kernel-config
+else ifneq ($(strip $(CONFIG_TARGET)),)
+ $(error ERROR: CONFIG_TARGET="$(CONFIG_TARGET)" invalid, use one of `platform`, `subtarget`, `subtarget_platform` or `env`)
 endif
 
 __linux_confcmd = $(2) $(patsubst %,+,$(wordlist 2,9999,$(1))) $(1)