Re: [PATCH kvmtool] Makefile: 'lvm version' works incorrect. Because CFLAGS can not get sub-make variable $(KVMTOOLS_VERSION)

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi,

On Thu, Dec 09, 2021 at 11:31:58AM +0800, haibiao.xiao wrote:
> Hi,
> 
> Thanks for your reply. I'd like to changed the subject line 
> as you suggested. But I don't know how to deal with it, 
> should I send another patch mail?

Yes, please send version 2 of the patch. The subject line for the email
should changed to (notice the extra v2):

[PATCH v2 kvmtool] <your summary here>

You can put the Tested-by tag after your Signed-off.

Thanks,
Alex

> 
> Thanks,
> haibiao.xiao
> 
> On Wed, 8 Dec 2021 16:38:17 +0000, Alexandru Elisei wrote:
> > Hi,
> > 
> > On Sat, Dec 04, 2021 at 02:14:36PM +0800, haibiao.xiao wrote:
> >> From: "haibiao.xiao" <xiaohaibiao331@outlook.com>
> > 
> > The subject line should be a summary of what the patch does (and perhaps
> > why it does it), not a description of what is broken. How about this:
> > 
> > Makefile: Calculate the correct kvmtool version
> > 
> > or something else that you prefer. Tested the patch and it works as
> > advertised:
> > 
> > Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > 
> > Thanks,
> > Alex
> > 
> >>
> >> Command 'lvm version' works incorrect.
> >> It is expected to print:
> >>
> >>     # ./lvm version
> >>     # kvm tool [KVMTOOLS_VERSION]
> >>
> >> but the KVMTOOLS_VERSION is missed:
> >>
> >>     # ./lvm version
> >>     # kvm tool
> >>
> >> The KVMTOOLS_VERSION is defined in the KVMTOOLS-VERSION-FILE file which
> >> is included at the end of Makefile. Since the CFLAGS is a 'Simply
> >> expanded variables' which means CFLAGS is only scanned once. So the
> >> definetion of KVMTOOLS_VERSION at the end of Makefile would not scanned
> >> by CFLAGS. So the '-DKVMTOOLS_VERSION=' remains empty.
> >>
> >> I fixed the bug by moving the '-include $(OUTPUT)KVMTOOLS-VERSION-FILE'
> >> before the CFLAGS.
> >>
> >> Signed-off-by: haibiao.xiao <xiaohaibiao331@outlook.com>
> >> ---
> >>  Makefile | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Makefile b/Makefile
> >> index bb7ad3e..9afb5e3 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -17,6 +17,7 @@ export E Q
> >>  
> >>  include config/utilities.mak
> >>  include config/feature-tests.mak
> >> +-include $(OUTPUT)KVMTOOLS-VERSION-FILE
> >>  
> >>  CC	:= $(CROSS_COMPILE)gcc
> >>  CFLAGS	:=
> >> @@ -559,5 +560,4 @@ ifneq ($(MAKECMDGOALS),clean)
> >>  
> >>  KVMTOOLS-VERSION-FILE:
> >>  	@$(SHELL_PATH) util/KVMTOOLS-VERSION-GEN $(OUTPUT)
> >> --include $(OUTPUT)KVMTOOLS-VERSION-FILE
> >> -endif
> >> +endif
> >> \ No newline at end of file
> >> -- 
> >> 2.32.0
> >>