Xi Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix $(@) shows empty issue ......................................................................
Makefile.inc: Fix $(@) shows empty issue
When passing $(@) to printf, $(@) is replaced by empty string, need to add extra '$'.
For example: cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ # ** @(@) is empty string instead of $(2) ** printf " CC+STRIP $(@) \n"; \ # ** $$(@) == $(2) ** printf " CC+STRIP $$(@) \n"; \ ......)
Signed-off-by: Xi Chen xixi.chen@mediatek.com Change-Id: Id6a66e25d7dfe8fe6410e517593ed22a438d2f82 --- M Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48201/1
diff --git a/Makefile.inc b/Makefile.inc index 9273961..38af658 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -336,7 +336,7 @@ # arg2: binary file cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ - printf " CC+STRIP $(@)\n"; \ + printf " CC+STRIP $$(<) --> $$(@) \n"; \ $(CC_ramstage) -MMD $(CPPFLAGS_ramstage) $(CFLAGS_ramstage) $$(ramstage-c-ccopts) -include $(KCONFIG_AUTOHEADER) -MT $(2) -o $(2).tmp -c $(1) && \ $(OBJCOPY_ramstage) -O binary $(2).tmp $(2); \ rm -f $(2).tmp) \
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix $(@) shows empty issue ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48201/1/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/1/Makefile.inc@339 PS1, Line 339: $$(<) --> the $$(<) --> should not be included?
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix $(@) shows empty issue ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48201/1/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/1/Makefile.inc@339 PS1, Line 339: $$(<) -->
the $$(<) --> should not be included?
"$$(<) -->" is added by myself. The building log shows that $(@) may be a random file name, developers can't match the file with $(1) easily.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix $(@) shows empty issue ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48201/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/1//COMMIT_MSG@7 PS1, Line 7: Fix $(@) shows empty issue Fix empty output when processing C struct files in CBFS
https://review.coreboot.org/c/coreboot/+/48201/1//COMMIT_MSG@15 PS1, Line 15: @ $
https://review.coreboot.org/c/coreboot/+/48201/1/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/1/Makefile.inc@339 PS1, Line 339: $$(<) -->
"$$(<) -->" is added by myself. […]
then you should describe that in the commit description as well, or put that into a separate change.
Hello Hung-Te Lin, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48201
to look at the new patch set (#2).
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Makefile.inc: Fix empty output when processing C struct files in CBFS
When passing $(@) to eval command, $(@) is replaced by empty string, need to add extra '$'. Another way, add $(<) to output C file name which is match with $(@).
For example: cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ # ** $(@) is empty string instead of $(2) ** printf " CC+STRIP $(@) \n"; \ # ** $$(@) == $(2) ** printf " CC+STRIP $$(@) \n"; \ ......)
Signed-off-by: Xi Chen xixi.chen@mediatek.com Change-Id: Id6a66e25d7dfe8fe6410e517593ed22a438d2f82 --- M Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48201/2
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48201/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/1//COMMIT_MSG@7 PS1, Line 7: Fix $(@) shows empty issue
Fix empty output when processing C struct files in CBFS
Done
https://review.coreboot.org/c/coreboot/+/48201/1//COMMIT_MSG@15 PS1, Line 15: @
$
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48201/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/2//COMMIT_MSG@10 PS2, Line 10: Another way, Also, the $(@) in cbfs-files-processor-struct is a temporary file name so we want to replace by $(<).
https://review.coreboot.org/c/coreboot/+/48201/2//COMMIT_MSG@9 PS2, Line 9: , : need to add extra '$' and we need to quote it by an extra '$'.
https://review.coreboot.org/c/coreboot/+/48201/2/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/2/Makefile.inc@339 PS2, Line 339: $$(<) --> $$(@) if $(@) is a random file name then I think what we really want is $(<) instead of $(@).
Hello Hung-Te Lin, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48201
to look at the new patch set (#3).
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Makefile.inc: Fix empty output when processing C struct files in CBFS
When passing $(@) to eval command, $(@) is replaced by empty string, Also, the $(@) in cbfs-files-processor-struct is a temporary file name so we want to replace by $(<) and need to quote it by an extra '$'.
For example: cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ # ** $(@) is empty string instead of $(2) ** printf " CC+STRIP $(@) \n"; \ # ** $$(@) == $(2) ** printf " CC+STRIP $$(@) \n"; \ ......)
Signed-off-by: Xi Chen xixi.chen@mediatek.com Change-Id: Id6a66e25d7dfe8fe6410e517593ed22a438d2f82 --- M Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48201/3
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48201/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/2//COMMIT_MSG@9 PS2, Line 9: , : need to add extra '$'
and we need to quote it by an extra '$'.
Ack
https://review.coreboot.org/c/coreboot/+/48201/2//COMMIT_MSG@10 PS2, Line 10: Another way,
Also, the $(@) in cbfs-files-processor-struct is a temporary file name so we want to replace by $(<) […]
Ack
https://review.coreboot.org/c/coreboot/+/48201/2/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/2/Makefile.inc@339 PS2, Line 339: $$(<) --> $$(@)
if $(@) is a random file name then I think what we really want is $(<) instead of $(@).
Understand! but sometimes, output $(@) helps to debug the random temp file if unexpected happens.
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48201/2/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/2/Makefile.inc@339 PS2, Line 339: $$(<) --> $$(@)
Understand! but sometimes, output $(@) helps to debug the random temp file if unexpected happens.
Done
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 3:
(1 comment)
Thanks for fixing this. I also noticed this when doing CB:47903, but haven't had a chance to fix it.
https://review.coreboot.org/c/coreboot/+/48201/3/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/3/Makefile.inc@339 PS3, Line 339: $$(<) What if we use $(2) here?
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48201/3/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/3/Makefile.inc@339 PS3, Line 339: $$(<)
What if we use $(2) here?
Yes, $(2) is ok. The output string of patchset 1 is: $$(<) --> $$(@), can output the name of .c and .out file. So, you'd prefer to output temporary file name $(2)?
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 3: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/48201/2/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/2/Makefile.inc@339 PS2, Line 339: $$(<) --> $$(@)
Done
Done
https://review.coreboot.org/c/coreboot/+/48201/3/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/3/Makefile.inc@339 PS3, Line 339: $$(<)
Yes, $(2) is ok. The output string of patchset 1 is: $$(<) --> $$(@), can output the name of . […]
I wasn't aware that $(2) is a temp file name. Let's use what you wrote here.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 5: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336: Why add the whitespace?
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336: < I guess we could also use $(1) directly.
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336: <
I guess we could also use $(1) directly.
Yes.
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336:
Why add the whitespace?
$$(<) \n $$(<)\n
As my view, the first line is more clear, so add one more blank.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336:
$$(<) \n […]
Can't say that I follow.
I would not block the change over this, but be warned: people write patches just to remove such unneeded whitespace. So it might not last.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336: <
Yes.
So maybe just use $(1) instead? It may be more clear than $@ and $< given the order was reversed in the rule.
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336:
Can't say that I follow. […]
I agree with Nico - having a space before \n is usually considered as wrong. Yes it may be ugly but that's how printf and debug messages already look like. So I'd also recommend removing the extra space.
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336:
I agree with Nico - having a space before \n is usually considered as wrong. […]
ok, agree it.
Hello Hung-Te Lin, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Julius Werner, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48201
to look at the new patch set (#6).
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Makefile.inc: Fix empty output when processing C struct files in CBFS
When passing $(@) to eval command, $(@) is replaced by empty string, Also, the $(@) in cbfs-files-processor-struct is a temporary file name so we want to replace by $(<) and need to quote it by an extra '$'.
For example: cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ # ** $(@) is empty string instead of $(2) ** printf " CC+STRIP $(@) \n"; \ # ** $$(@) == $(2) ** printf " CC+STRIP $$(@) \n"; \ ......)
Signed-off-by: Xi Chen xixi.chen@mediatek.com Change-Id: Id6a66e25d7dfe8fe6410e517593ed22a438d2f82 --- M Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48201/6
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 6:
(3 comments)
Just some nits on description. Thanks!
https://review.coreboot.org/c/coreboot/+/48201/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/6//COMMIT_MSG@11 PS6, Line 11: so we want to replace by $(<) and need to quote it by an extra '$'. so we should quote it by an extra '$' or use the arg ($1 or $2) directly.
Also, $(@)=$2 is actually a temporary file and what we want is the source file so using $1 is more correct.
https://review.coreboot.org/c/coreboot/+/48201/6//COMMIT_MSG@18 PS6, Line 18: ** $$(@) == $(2) ** $1 contains the name of source file.
https://review.coreboot.org/c/coreboot/+/48201/6//COMMIT_MSG@19 PS6, Line 19: $$(@) $1
Hello Hung-Te Lin, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Julius Werner, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48201
to look at the new patch set (#7).
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Makefile.inc: Fix empty output when processing C struct files in CBFS
When passing $(@) to eval command, $(@) is replaced by empty string, Also, the $(@) in cbfs-files-processor-struct is a temporary file name, so we should quote it by an extra '$' or use the arg ($1 or $2) directly.
Also, $(@)=$2 is actually a temporary file and what we want is the source file so using $1 is more correct.
For example: cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ # ** $(@) is empty string instead of $(2) ** printf " CC+STRIP $(@) \n"; \ # ** $(1) contains the name of source file ** printf " CC+STRIP $(1) \n"; \ ......)
Signed-off-by: Xi Chen xixi.chen@mediatek.com Change-Id: Id6a66e25d7dfe8fe6410e517593ed22a438d2f82 --- M Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48201/7
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48201/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/6//COMMIT_MSG@11 PS6, Line 11: so we want to replace by $(<) and need to quote it by an extra '$'.
so we should quote it by an extra '$' or use the arg ($1 or $2) directly. […]
Ack
Hello Hung-Te Lin, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Julius Werner, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48201
to look at the new patch set (#8).
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Makefile.inc: Fix empty output when processing C struct files in CBFS
When passing $(@) to eval command, $(@) is replaced by empty string, Also, the $(@) in cbfs-files-processor-struct is a temporary file name, so we should quote it by an extra '$' or use the arg ($1 or $2) directly.
Also, $(@)=$2 is actually a temporary file and what we want is the source file so using $1 is more correct.
For example: cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ # ** $(@) is empty string instead of $(2) ** printf " CC+STRIP $(@) \n"; \ # ** $(1) contains the name of source file ** printf " CC+STRIP $(1) \n"; \ ......)
Signed-off-by: Xi Chen xixi.chen@mediatek.com Change-Id: Id6a66e25d7dfe8fe6410e517593ed22a438d2f82 --- M Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48201/8
Hello Hung-Te Lin, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Julius Werner, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48201
to look at the new patch set (#9).
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Makefile.inc: Fix empty output when processing C struct files in CBFS
When passing $(@) to eval command, $(@) is replaced by empty string, Also, the $(@) in cbfs-files-processor-struct is a temporary file name, so we should quote it by an extra '$' or use the arg ($1 or $2) directly.
Also, $(@)=$2 is actually a temporary file and what we want is the source file so using $1 is more correct.
For example: cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ # ** $(@) is empty string instead of $(2) ** printf " CC+STRIP $(@) \n"; \ # ** $(1) contains the name of source file ** printf " CC+STRIP $(1) \n"; \ ......)
Signed-off-by: Xi Chen xixi.chen@mediatek.com Change-Id: Id6a66e25d7dfe8fe6410e517593ed22a438d2f82 --- M Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48201/9
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 9:
(7 comments)
https://review.coreboot.org/c/coreboot/+/48201/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/6//COMMIT_MSG@18 PS6, Line 18: ** $$(@) == $(2) **
$1 contains the name of source file.
Done
https://review.coreboot.org/c/coreboot/+/48201/6//COMMIT_MSG@19 PS6, Line 19: $$(@)
$1
Done
https://review.coreboot.org/c/coreboot/+/48201/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/9//COMMIT_MSG@11 PS9, Line 11: directly Line too long.
https://review.coreboot.org/c/coreboot/+/48201/9//COMMIT_MSG@13 PS9, Line 13: $(@)=$2 is actually a temporary file Isn't this already mentioned in the first paragraph? Also, isn't $(@) an empty string?
https://review.coreboot.org/c/coreboot/+/48201/1/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/1/Makefile.inc@339 PS1, Line 339: $$(<) -->
then you should describe that in the commit description as well, or put that into a separate change.
Done
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336:
ok, agree it.
Done
https://review.coreboot.org/c/coreboot/+/48201/5/Makefile.inc@336 PS5, Line 336: <
So maybe just use $(1) instead? It may be more clear than $@ and $< given the order was reversed in […]
Done
Hello Hung-Te Lin, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Julius Werner, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48201
to look at the new patch set (#10).
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Makefile.inc: Fix empty output when processing C struct files in CBFS
When passing $(@) to eval command, $(@) is replaced by empty string, Also, the $(@) in cbfs-files-processor-struct is a temporary file name, so we should quote it by an extra '$' or use the arg ($1 or $2) directly.
For example: cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ # ** $(@) is empty string instead of $(2) ** printf " CC+STRIP $(@) \n"; \ # ** $(1) contains the name of source file ** printf " CC+STRIP $(1) \n"; \ ......)
Signed-off-by: Xi Chen xixi.chen@mediatek.com Change-Id: Id6a66e25d7dfe8fe6410e517593ed22a438d2f82 --- M Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48201/10
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48201/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/9//COMMIT_MSG@11 PS9, Line 11: directly
Line too long.
Ack
https://review.coreboot.org/c/coreboot/+/48201/9//COMMIT_MSG@13 PS9, Line 13: $(@)=$2 is actually a temporary file
Isn't this already mentioned in the first paragraph? Also, isn't $(@) an empty string?
done, remove it.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 10: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/48201/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48201/9//COMMIT_MSG@13 PS9, Line 13: $(@)=$2 is actually a temporary file
done, remove it.
Ack
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Patch Set 10: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48201 )
Change subject: Makefile.inc: Fix empty output when processing C struct files in CBFS ......................................................................
Makefile.inc: Fix empty output when processing C struct files in CBFS
When passing $(@) to eval command, $(@) is replaced by empty string, Also, the $(@) in cbfs-files-processor-struct is a temporary file name, so we should quote it by an extra '$' or use the arg ($1 or $2) directly.
For example: cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ # ** $(@) is empty string instead of $(2) ** printf " CC+STRIP $(@) \n"; \ # ** $(1) contains the name of source file ** printf " CC+STRIP $(1) \n"; \ ......)
Signed-off-by: Xi Chen xixi.chen@mediatek.com Change-Id: Id6a66e25d7dfe8fe6410e517593ed22a438d2f82 Reviewed-on: https://review.coreboot.org/c/coreboot/+/48201 Reviewed-by: Yu-Ping Wu yupingso@google.com Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Yu-Ping Wu: Looks good to me, approved
diff --git a/Makefile.inc b/Makefile.inc index 8cba96b..dee4a2e 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -333,7 +333,7 @@ # arg2: binary file cbfs-files-processor-struct= \ $(eval $(2): $(1) $(obj)/build.h $(KCONFIG_AUTOHEADER); \ - printf " CC+STRIP $(@)\n"; \ + printf " CC+STRIP $(1)\n"; \ $(CC_ramstage) -MMD $(CPPFLAGS_ramstage) $(CFLAGS_ramstage) $$(ramstage-c-ccopts) -include $(KCONFIG_AUTOHEADER) -MT $(2) -o $(2).tmp -c $(1) && \ $(OBJCOPY_ramstage) -O binary --set-section-flags .bss*=alloc,contents,load $(2).tmp $(2); \ rm -f $(2).tmp) \
-
Hung-Te Lin (Code Review) -
Nico Huber (Code Review)
-
Xi Chen (Code Review)
-
Yu-Ping Wu (Code Review)