Change in coreboot[master]: Makefile: Switch to `.config` as input for the Ada `CB.Config`

[Nico Huber (Code Review)]

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36320 )

Change subject: Makefile: Switch to `.config` as input for the Ada `CB.Config`
......................................................................

Makefile: Switch to `.config` as input for the Ada `CB.Config`

So, this is odd in multiple ways. First of all, we fix something:
We work around a weirdness in `make oldconfig` that adds spurious
entries into the `auto.conf` for choices that were given a symbol
name (I don't know why it is possible at all to give them a name).

When introducing the Ada config package, it seemed reasonable to
use `auto.conf` as source, but it turned out that we didn't use it
as input, only `config.h` and the original `.config` were used. As
the syntax for `.config` is the same as for `auto.conf` we use the
former now as input for Ada, too. One question remains: If `.config`
already contains all required information, what is this `auto.conf`
and what does it want?

Alternatively, we could try to fix `oldconfig` or add a linter to
forbid named choices. I thought, our build test would reject the
latter already. But the `oldconfig` behaviour is to subtle.

We keep a dependency on the `oldconfig` step, to make sure it runs
first.

Change-Id: If3fe6bc782251cdbd696395d3069a1c0bb0ae802
Signed-off-by: Nico Huber 
---
M Makefile
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/36320/1

diff --git a/Makefile b/Makefile
index 14ec2bf..0172b09 100644
--- a/Makefile
+++ b/Makefile
@@ -194,7 +194,7 @@
 $(KCONFIG_AUTOCONFIG): $(KCONFIG_AUTOHEADER)
    true

-$(KCONFIG_AUTOADS): $(KCONFIG_AUTOCONFIG) $(objutil)/kconfig/toada
+$(KCONFIG_AUTOADS): $(KCONFIG_CONFIG) $(KCONFIG_AUTOHEADER) $(objutil)/kconfig/toada
    $(objutil)/kconfig/toada CB.Config <$< >$@

$(obj)/%/$(notdir $(KCONFIG_AUTOADS)): $(KCONFIG_AUTOADS)

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36320 )

Change subject: Makefile: Switch to `.config` as input for the Ada `CB.Config`
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36320/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/36320/1//COMMIT_MSG@12 
PS1, Line 12: I don't know why it is possible at all to give them a name
See https://review.coreboot.org/c/coreboot/+/29813/3

[Nico Huber (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36320

to look at the new patch set (#2).

Change subject: Makefile: Switch to `.config` as input for the Ada `CB.Config`
......................................................................

Makefile: Switch to `.config` as input for the Ada `CB.Config`

So, this is odd in multiple ways. First of all, we fix something:
We work around a weirdness in `make oldconfig` that adds spurious
entries into the `auto.conf` for choices that were given a symbol
name.

When introducing the Ada config package, it seemed reasonable to
use `auto.conf` as source, but it turned out that we didn't use it
as input, only `config.h` and the original `.config` were used. As
the syntax for `.config` is the same as for `auto.conf` we use the
former now as input for Ada, too. One question remains: If `.config`
already contains all required information, what is this `auto.conf`
and what does it want?

Alternatively, we could try to fix `oldconfig` or add a linter to
forbid named choices. I thought, our build test would reject the
latter already. But the `oldconfig` behaviour is to subtle.

We keep a dependency on the `oldconfig` step, to make sure it runs
first.

Change-Id: If3fe6bc782251cdbd696395d3069a1c0bb0ae802
Signed-off-by: Nico Huber 
---
M Makefile
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/36320/2

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36320 )

Change subject: Makefile: Switch to `.config` as input for the Ada `CB.Config`
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36320/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/36320/1//COMMIT_MSG@12 
PS1, Line 12: I don't know why it is possible at all to give them a name
> See https://review.coreboot. […]

thanks

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36320 )

Change subject: Makefile: Switch to `.config` as input for the Ada `CB.Config`
......................................................................

Patch Set 2: Code-Review+2

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36320 )

Change subject: Makefile: Switch to `.config` as input for the Ada `CB.Config`
......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/36320/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/36320/2//COMMIT_MSG@24 
PS2, Line 24: to
too

[Nico Huber (Code Review)]

Hello Angel Pons, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36320

to look at the new patch set (#3).

Change subject: Makefile: Switch to `.config` as input for the Ada `CB.Config`
......................................................................

Makefile: Switch to `.config` as input for the Ada `CB.Config`

So, this is odd in multiple ways. First of all, we fix something:
We work around a weirdness in `make oldconfig` that adds spurious
entries into the `auto.conf` for choices that were given a symbol
name.

When introducing the Ada config package, it seemed reasonable to
use `auto.conf` as source, but it turned out that we didn't use it
as input, only `config.h` and the original `.config` were used. As
the syntax for `.config` is the same as for `auto.conf` we use the
former now as input for Ada, too. One question remains: If `.config`
already contains all required information, what is this `auto.conf`
and what does it want?

Alternatively, we could try to fix `oldconfig` or add a linter to
forbid named choices. I thought, our build test would reject the
latter already. But the `oldconfig` behaviour is too subtle.

We keep a dependency on the `oldconfig` step, to make sure it runs
first.

Change-Id: If3fe6bc782251cdbd696395d3069a1c0bb0ae802
Signed-off-by: Nico Huber 
---
M Makefile
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/36320/3

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36320 )

Change subject: Makefile: Switch to `.config` as input for the Ada `CB.Config`
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36320/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/36320/2//COMMIT_MSG@24 
PS2, Line 24: to
> too

Thanks

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36320 )

Change subject: Makefile: Switch to `.config` as input for the Ada `CB.Config`
......................................................................

Makefile: Switch to `.config` as input for the Ada `CB.Config`

So, this is odd in multiple ways. First of all, we fix something:
We work around a weirdness in `make oldconfig` that adds spurious
entries into the `auto.conf` for choices that were given a symbol
name.

When introducing the Ada config package, it seemed reasonable to
use `auto.conf` as source, but it turned out that we didn't use it
as input, only `config.h` and the original `.config` were used. As
the syntax for `.config` is the same as for `auto.conf` we use the
former now as input for Ada, too. One question remains: If `.config`
already contains all required information, what is this `auto.conf`
and what does it want?

Alternatively, we could try to fix `oldconfig` or add a linter to
forbid named choices. I thought, our build test would reject the
latter already. But the `oldconfig` behaviour is too subtle.

We keep a dependency on the `oldconfig` step, to make sure it runs
first.

Change-Id: If3fe6bc782251cdbd696395d3069a1c0bb0ae802
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36320
Reviewed-by: Arthur Heymans 
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M Makefile
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Arthur Heymans: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/Makefile b/Makefile
index 14ec2bf..0172b09 100644
--- a/Makefile
+++ b/Makefile
@@ -194,7 +194,7 @@
 $(KCONFIG_AUTOCONFIG): $(KCONFIG_AUTOHEADER)
    true

-$(KCONFIG_AUTOADS): $(KCONFIG_AUTOCONFIG) $(objutil)/kconfig/toada
+$(KCONFIG_AUTOADS): $(KCONFIG_CONFIG) $(KCONFIG_AUTOHEADER) $(objutil)/kconfig/toada
    $(objutil)/kconfig/toada CB.Config <$< >$@

$(obj)/%/$(notdir $(KCONFIG_AUTOADS)): $(KCONFIG_AUTOADS)