2022-06-06 12:38:38

by David Howells

[permalink] [raw]
Subject: [PATCH] certs: Convert spaces in certs/Makefile to a tab

There's a rule in certs/Makefile for which the command begins with eight
spaces. This results in:

../certs/Makefile:21: FORCE prerequisite is missing
../certs/Makefile:21: *** missing separator. Stop.

Fix this by turning the spaces into a tab.

Fixes: addf466389d9 ("certs: Check that builtin blacklist hashes are valid")
Signed-off-by: David Howells <[email protected]>
cc: Mickaël Salaün <[email protected]>
cc: Jarkko Sakkinen <[email protected]>
cc: [email protected]
---

certs/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certs/Makefile b/certs/Makefile
index bb904f90f139..cb1a9da3fc58 100644
--- a/certs/Makefile
+++ b/certs/Makefile
@@ -18,7 +18,7 @@ CFLAGS_blacklist_hashes.o += -I$(srctree)

targets += blacklist_hashes_checked
$(obj)/blacklist_hashes_checked: $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) scripts/check-blacklist-hashes.awk FORCE
- $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
+ $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o
else
obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_nohashes.o



2022-06-06 17:19:49

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] certs: Convert spaces in certs/Makefile to a tab

On Mon, Jun 6, 2022 at 9:32 PM David Howells <[email protected]> wrote:
>
> There's a rule in certs/Makefile for which the command begins with eight
> spaces. This results in:
>
> ../certs/Makefile:21: FORCE prerequisite is missing
> ../certs/Makefile:21: *** missing separator. Stop.
>
> Fix this by turning the spaces into a tab.
>
> Fixes: addf466389d9 ("certs: Check that builtin blacklist hashes are valid")
> Signed-off-by: David Howells <[email protected]>
> cc: Mickaël Salaün <[email protected]>
> cc: Jarkko Sakkinen <[email protected]>
> cc: [email protected]


Not only 8-space indentation, but also:

- config_filename does not exist
- $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX) is always empty
- $(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) is always empty


> ---
>
> certs/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/certs/Makefile b/certs/Makefile
> index bb904f90f139..cb1a9da3fc58 100644
> --- a/certs/Makefile
> +++ b/certs/Makefile
> @@ -18,7 +18,7 @@ CFLAGS_blacklist_hashes.o += -I$(srctree)
>
> targets += blacklist_hashes_checked
> $(obj)/blacklist_hashes_checked: $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) scripts/check-blacklist-hashes.awk FORCE
> - $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
> + $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o
> else
> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_nohashes.o
>
>


--
Best Regards
Masahiro Yamada

2022-06-07 18:31:22

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] certs: Convert spaces in certs/Makefile to a tab

On Mon, Jun 06, 2022 at 01:31:59PM +0100, David Howells wrote:
> There's a rule in certs/Makefile for which the command begins with eight
> spaces. This results in:
>
> ../certs/Makefile:21: FORCE prerequisite is missing
> ../certs/Makefile:21: *** missing separator. Stop.
>
> Fix this by turning the spaces into a tab.
>
> Fixes: addf466389d9 ("certs: Check that builtin blacklist hashes are valid")
> Signed-off-by: David Howells <[email protected]>
> cc: Micka?l Sala?n <[email protected]>
> cc: Jarkko Sakkinen <[email protected]>
> cc: [email protected]
> ---
>
> certs/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/certs/Makefile b/certs/Makefile
> index bb904f90f139..cb1a9da3fc58 100644
> --- a/certs/Makefile
> +++ b/certs/Makefile
> @@ -18,7 +18,7 @@ CFLAGS_blacklist_hashes.o += -I$(srctree)
>
> targets += blacklist_hashes_checked
> $(obj)/blacklist_hashes_checked: $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) scripts/check-blacklist-hashes.awk FORCE
> - $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
> + $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o
> else
> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_nohashes.o
>
>


Reviewed-by: Jarkko Sakkinen <[email protected]>

BR, Jarkko

2022-06-09 17:31:59

by Mickaël Salaün

[permalink] [raw]
Subject: Re: [PATCH] certs: Convert spaces in certs/Makefile to a tab


On 06/06/2022 14:31, David Howells wrote:
> There's a rule in certs/Makefile for which the command begins with eight
> spaces. This results in:
>
> ../certs/Makefile:21: FORCE prerequisite is missing
> ../certs/Makefile:21: *** missing separator. Stop.
>
> Fix this by turning the spaces into a tab.

These spaces were not part of my patch but they are indeed in this file
now: https://lore.kernel.org/r/[email protected]

Reviewed-by: Mickaël Salaün <[email protected]>


>
> Fixes: addf466389d9 ("certs: Check that builtin blacklist hashes are valid")
> Signed-off-by: David Howells <[email protected]>
> cc: Mickaël Salaün <[email protected]>
> cc: Jarkko Sakkinen <[email protected]>
> cc: [email protected]
> ---
>
> certs/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/certs/Makefile b/certs/Makefile
> index bb904f90f139..cb1a9da3fc58 100644
> --- a/certs/Makefile
> +++ b/certs/Makefile
> @@ -18,7 +18,7 @@ CFLAGS_blacklist_hashes.o += -I$(srctree)
>
> targets += blacklist_hashes_checked
> $(obj)/blacklist_hashes_checked: $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) scripts/check-blacklist-hashes.awk FORCE
> - $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
> + $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o
> else
> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_nohashes.o
>
>

2022-06-09 18:17:26

by Mickaël Salaün

[permalink] [raw]
Subject: Re: [PATCH] certs: Convert spaces in certs/Makefile to a tab



On 09/06/2022 19:12, Mickaël Salaün wrote:
>
> On 06/06/2022 18:49, Masahiro Yamada wrote:
>> On Mon, Jun 6, 2022 at 9:32 PM David Howells <[email protected]> wrote:
>>>
>>> There's a rule in certs/Makefile for which the command begins with eight
>>> spaces.  This results in:
>>>
>>>          ../certs/Makefile:21: FORCE prerequisite is missing
>>>          ../certs/Makefile:21: *** missing separator.  Stop.
>>>
>>> Fix this by turning the spaces into a tab.
>>>
>>> Fixes: addf466389d9 ("certs: Check that builtin blacklist hashes are
>>> valid")
>>> Signed-off-by: David Howells <[email protected]>
>>> cc: Mickaël Salaün <[email protected]>
>>> cc: Jarkko Sakkinen <[email protected]>
>>> cc: [email protected]
>>
>>
>> Not only 8-space indentation, but also:
>>
>>    - config_filename does not exist
>>    - $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX) is always empty
>>    - $(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) is always empty
>
> These are imported helpers (not only used for this hash list BTW), hence
> not defined in this Makefile.

Well, they were defined in scripts/Kbuild.include but they are gone
since your commit b8c96a6b466c ("certs: simplify $(srctree)/ handling
and remove config_filename macro").

I guess it just happens during the merge. We need to fix that.


>
>>
>>
>>> ---
>>>
>>>   certs/Makefile |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/certs/Makefile b/certs/Makefile
>>> index bb904f90f139..cb1a9da3fc58 100644
>>> --- a/certs/Makefile
>>> +++ b/certs/Makefile
>>> @@ -18,7 +18,7 @@ CFLAGS_blacklist_hashes.o += -I$(srctree)
>>>
>>>   targets += blacklist_hashes_checked
>>>   $(obj)/blacklist_hashes_checked:
>>> $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(SYSTEM_BLACKLIST_HASH_LIST_FILENAME)
>>> scripts/check-blacklist-hashes.awk FORCE
>>> -       $(call
>>> if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
>>>
>>> +       $(call
>>> if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
>>>
>>>   obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o
>>>   else
>>>   obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_nohashes.o
>>>
>>>
>>
>>
>> --
>> Best Regards
>> Masahiro Yamada

2022-06-09 18:20:57

by Mickaël Salaün

[permalink] [raw]
Subject: Re: [PATCH] certs: Convert spaces in certs/Makefile to a tab


On 06/06/2022 18:49, Masahiro Yamada wrote:
> On Mon, Jun 6, 2022 at 9:32 PM David Howells <[email protected]> wrote:
>>
>> There's a rule in certs/Makefile for which the command begins with eight
>> spaces. This results in:
>>
>> ../certs/Makefile:21: FORCE prerequisite is missing
>> ../certs/Makefile:21: *** missing separator. Stop.
>>
>> Fix this by turning the spaces into a tab.
>>
>> Fixes: addf466389d9 ("certs: Check that builtin blacklist hashes are valid")
>> Signed-off-by: David Howells <[email protected]>
>> cc: Mickaël Salaün <[email protected]>
>> cc: Jarkko Sakkinen <[email protected]>
>> cc: [email protected]
>
>
> Not only 8-space indentation, but also:
>
> - config_filename does not exist
> - $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX) is always empty
> - $(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) is always empty

These are imported helpers (not only used for this hash list BTW), hence
not defined in this Makefile.

>
>
>> ---
>>
>> certs/Makefile | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/certs/Makefile b/certs/Makefile
>> index bb904f90f139..cb1a9da3fc58 100644
>> --- a/certs/Makefile
>> +++ b/certs/Makefile
>> @@ -18,7 +18,7 @@ CFLAGS_blacklist_hashes.o += -I$(srctree)
>>
>> targets += blacklist_hashes_checked
>> $(obj)/blacklist_hashes_checked: $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) scripts/check-blacklist-hashes.awk FORCE
>> - $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
>> + $(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
>> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o
>> else
>> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_nohashes.o
>>
>>
>
>
> --
> Best Regards
> Masahiro Yamada

2022-06-09 18:57:53

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] certs: Convert spaces in certs/Makefile to a tab

On Fri, Jun 10, 2022 at 2:05 AM Mickaël Salaün <[email protected]> wrote:
>
>
> On 06/06/2022 14:31, David Howells wrote:
> > There's a rule in certs/Makefile for which the command begins with eight
> > spaces. This results in:
> >
> > ../certs/Makefile:21: FORCE prerequisite is missing
> > ../certs/Makefile:21: *** missing separator. Stop.
> >
> > Fix this by turning the spaces into a tab.
>
> These spaces were not part of my patch but they are indeed in this file
> now: https://lore.kernel.org/r/[email protected]
>
> Reviewed-by: Mickaël Salaün <[email protected]>


Indeed, you used a tab,
but the applied code (commit addf466389d9) uses 8 spaces.

I think something bad happened
when the committer locally modified the code.






--
Best Regards
Masahiro Yamada

2022-06-09 19:20:37

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] certs: Convert spaces in certs/Makefile to a tab

On Fri, Jun 10, 2022 at 2:17 AM Mickaël Salaün <[email protected]> wrote:
>
>
>
> On 09/06/2022 19:12, Mickaël Salaün wrote:
> >
> > On 06/06/2022 18:49, Masahiro Yamada wrote:
> >> On Mon, Jun 6, 2022 at 9:32 PM David Howells <[email protected]> wrote:
> >>>
> >>> There's a rule in certs/Makefile for which the command begins with eight
> >>> spaces. This results in:
> >>>
> >>> ../certs/Makefile:21: FORCE prerequisite is missing
> >>> ../certs/Makefile:21: *** missing separator. Stop.
> >>>
> >>> Fix this by turning the spaces into a tab.
> >>>
> >>> Fixes: addf466389d9 ("certs: Check that builtin blacklist hashes are
> >>> valid")
> >>> Signed-off-by: David Howells <[email protected]>
> >>> cc: Mickaël Salaün <[email protected]>
> >>> cc: Jarkko Sakkinen <[email protected]>
> >>> cc: [email protected]
> >>
> >>
> >> Not only 8-space indentation, but also:
> >>
> >> - config_filename does not exist
> >> - $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX) is always empty
> >> - $(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) is always empty
> >
> > These are imported helpers (not only used for this hash list BTW), hence
> > not defined in this Makefile.
>
> Well, they were defined in scripts/Kbuild.include but they are gone
> since your commit b8c96a6b466c ("certs: simplify $(srctree)/ handling
> and remove config_filename macro").
>
> I guess it just happens during the merge. We need to fix that.
>




Right, it seems your patch was flying for a long time.





$ git show --pretty=fuller addf466389d9d78f255e8b15ac44ab4791029852
commit addf466389d9d78f255e8b15ac44ab4791029852
Author: Mickaël Salaün <[email protected]>
AuthorDate: Mon Jul 12 19:03:10 2021 +0200
Commit: Jarkko Sakkinen <[email protected]>
CommitDate: Mon May 23 18:47:49 2022 +0300

certs: Check that builtin blacklist hashes are valid




It was committed 8 months after the patch submission.

The base code changed during the gap.




--
Best Regards
Masahiro Yamada

2022-06-11 18:08:55

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] certs: Convert spaces in certs/Makefile to a tab

On Fri, Jun 10, 2022 at 2:17 AM Mickaël Salaün <[email protected]> wrote:
>
>
>
> On 09/06/2022 19:12, Mickaël Salaün wrote:
> >
> > On 06/06/2022 18:49, Masahiro Yamada wrote:
> >> On Mon, Jun 6, 2022 at 9:32 PM David Howells <[email protected]> wrote:
> >>>
> >>> There's a rule in certs/Makefile for which the command begins with eight
> >>> spaces. This results in:
> >>>
> >>> ../certs/Makefile:21: FORCE prerequisite is missing
> >>> ../certs/Makefile:21: *** missing separator. Stop.
> >>>
> >>> Fix this by turning the spaces into a tab.
> >>>
> >>> Fixes: addf466389d9 ("certs: Check that builtin blacklist hashes are
> >>> valid")
> >>> Signed-off-by: David Howells <[email protected]>
> >>> cc: Mickaël Salaün <[email protected]>
> >>> cc: Jarkko Sakkinen <[email protected]>
> >>> cc: [email protected]
> >>
> >>
> >> Not only 8-space indentation, but also:
> >>
> >> - config_filename does not exist
> >> - $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX) is always empty
> >> - $(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) is always empty
> >
> > These are imported helpers (not only used for this hash list BTW), hence
> > not defined in this Makefile.
>
> Well, they were defined in scripts/Kbuild.include but they are gone
> since your commit b8c96a6b466c ("certs: simplify $(srctree)/ handling
> and remove config_filename macro").
>
> I guess it just happens during the merge. We need to fix that.

Today, I took a closer look at it.
I volunteered to fix the build issues with some refactoring.

https://lore.kernel.org/keyrings/[email protected]/T/#t







--
Best Regards
Masahiro Yamada