| Message ID | 165451871967.1941436.17828809503267245815.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | certs: Convert spaces in certs/Makefile to a tab | expand |
On Mon, Jun 6, 2022 at 9:32 PM David Howells <dhowells@redhat.com> 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 <dhowells@redhat.com> > cc: Mickaël Salaün <mic@linux.microsoft.com> > cc: Jarkko Sakkinen <jarkko@kernel.org> > cc: keyrings@vger.kernel.org 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
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 <dhowells@redhat.com> > cc: Mickaël Salaün <mic@linux.microsoft.com> > cc: Jarkko Sakkinen <jarkko@kernel.org> > cc: keyrings@vger.kernel.org > --- > > 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 <jarkko@kernel.org> BR, Jarkko
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/20210712170313.884724-3-mic@digikod.net Reviewed-by: Mickaël Salaün <mic@linux.microsoft.com> > > Fixes: addf466389d9 ("certs: Check that builtin blacklist hashes are valid") > Signed-off-by: David Howells <dhowells@redhat.com> > cc: Mickaël Salaün <mic@linux.microsoft.com> > cc: Jarkko Sakkinen <jarkko@kernel.org> > cc: keyrings@vger.kernel.org > --- > > 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 > >
On 06/06/2022 18:49, Masahiro Yamada wrote: > On Mon, Jun 6, 2022 at 9:32 PM David Howells <dhowells@redhat.com> 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 <dhowells@redhat.com> >> cc: Mickaël Salaün <mic@linux.microsoft.com> >> cc: Jarkko Sakkinen <jarkko@kernel.org> >> cc: keyrings@vger.kernel.org > > > 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
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 <dhowells@redhat.com> 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 <dhowells@redhat.com> >>> cc: Mickaël Salaün <mic@linux.microsoft.com> >>> cc: Jarkko Sakkinen <jarkko@kernel.org> >>> cc: keyrings@vger.kernel.org >> >> >> 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
On Fri, Jun 10, 2022 at 2:05 AM Mickaël Salaün <mic@digikod.net> 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/20210712170313.884724-3-mic@digikod.net > > Reviewed-by: Mickaël Salaün <mic@linux.microsoft.com> 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
On Fri, Jun 10, 2022 at 2:17 AM Mickaël Salaün <mic@digikod.net> 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 <dhowells@redhat.com> 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 <dhowells@redhat.com> > >>> cc: Mickaël Salaün <mic@linux.microsoft.com> > >>> cc: Jarkko Sakkinen <jarkko@kernel.org> > >>> cc: keyrings@vger.kernel.org > >> > >> > >> 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 <mic@linux.microsoft.com> AuthorDate: Mon Jul 12 19:03:10 2021 +0200 Commit: Jarkko Sakkinen <jarkko@kernel.org> 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
On Fri, Jun 10, 2022 at 2:17 AM Mickaël Salaün <mic@digikod.net> 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 <dhowells@redhat.com> 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 <dhowells@redhat.com> > >>> cc: Mickaël Salaün <mic@linux.microsoft.com> > >>> cc: Jarkko Sakkinen <jarkko@kernel.org> > >>> cc: keyrings@vger.kernel.org > >> > >> > >> 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/20220611172233.1494073-1-masahiroy@kernel.org/T/#t
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
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 <dhowells@redhat.com> cc: Mickaël Salaün <mic@linux.microsoft.com> cc: Jarkko Sakkinen <jarkko@kernel.org> cc: keyrings@vger.kernel.org --- certs/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)