Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line
> On 30 May 2023, at 11:14, Jan Beulich wrote: > > On 24.05.2023 11:43, Luca Fancellu wrote: >> >> >>> On 23 May 2023, at 17:38, Anthony PERARD wrote: >>> >>> CFLAGS is just from Config.mk, instead use the flags used to build >>> Xen. >>> >>> Signed-off-by: Anthony PERARD >>> --- >>> >>> Notes: >>> I don't know if CFLAGS is even useful there, just --version without the >>> flags might produce the same result. >>> >>> xen/build.mk | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/xen/build.mk b/xen/build.mk >>> index e2a78aa806..d468bb6e26 100644 >>> --- a/xen/build.mk >>> +++ b/xen/build.mk >>> @@ -23,7 +23,7 @@ define cmd_compile.h >>> -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \ >>> -e 's/@@domain@@/$(XEN_DOMAIN)/g' \ >>> -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \ >>> --e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head >>> -1)!g' \ >>> +-e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head >>> -1)!g' \ >>> -e 's/@@version@@/$(XEN_VERSION)/g' \ >>> -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \ >>> -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \ >>> -- >>> Anthony PERARD >>> >>> >> >> Yes I think Andrew is right, so I guess $(XEN_CFLAGS) can be dropped? >> >> Reviewed-by: Luca Fancellu >> Tested-by: Luca Fancellu >> >> I’ve tested this patch with and without the $(XEN_CFLAGS), so if you drop it >> you can >> retain my r-by if you want. > > I'm sorry, I didn't look back here to spot this extra sentence before > committing the edited patch, which as a result I've now put in without > your tags. > No problem! > Jan
Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line
On 24.05.2023 11:43, Luca Fancellu wrote: > > >> On 23 May 2023, at 17:38, Anthony PERARD wrote: >> >> CFLAGS is just from Config.mk, instead use the flags used to build >> Xen. >> >> Signed-off-by: Anthony PERARD >> --- >> >> Notes: >>I don't know if CFLAGS is even useful there, just --version without the >>flags might produce the same result. >> >> xen/build.mk | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/xen/build.mk b/xen/build.mk >> index e2a78aa806..d468bb6e26 100644 >> --- a/xen/build.mk >> +++ b/xen/build.mk >> @@ -23,7 +23,7 @@ define cmd_compile.h >>-e 's/@@whoami@@/$(XEN_WHOAMI)/g' \ >>-e 's/@@domain@@/$(XEN_DOMAIN)/g' \ >>-e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \ >> --e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' >> \ >> +-e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head >> -1)!g' \ >>-e 's/@@version@@/$(XEN_VERSION)/g' \ >>-e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \ >>-e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \ >> -- >> Anthony PERARD >> >> > > Yes I think Andrew is right, so I guess $(XEN_CFLAGS) can be dropped? > > Reviewed-by: Luca Fancellu > Tested-by: Luca Fancellu > > I’ve tested this patch with and without the $(XEN_CFLAGS), so if you drop it > you can > retain my r-by if you want. I'm sorry, I didn't look back here to spot this extra sentence before committing the edited patch, which as a result I've now put in without your tags. Jan
Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line
On 24.05.2023 11:43, Luca Fancellu wrote: > > >> On 23 May 2023, at 17:38, Anthony PERARD wrote: >> >> CFLAGS is just from Config.mk, instead use the flags used to build >> Xen. >> >> Signed-off-by: Anthony PERARD >> --- >> >> Notes: >>I don't know if CFLAGS is even useful there, just --version without the >>flags might produce the same result. >> >> xen/build.mk | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/xen/build.mk b/xen/build.mk >> index e2a78aa806..d468bb6e26 100644 >> --- a/xen/build.mk >> +++ b/xen/build.mk >> @@ -23,7 +23,7 @@ define cmd_compile.h >>-e 's/@@whoami@@/$(XEN_WHOAMI)/g' \ >>-e 's/@@domain@@/$(XEN_DOMAIN)/g' \ >>-e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \ >> --e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' >> \ >> +-e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head >> -1)!g' \ >>-e 's/@@version@@/$(XEN_VERSION)/g' \ >>-e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \ >>-e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \ >> -- >> Anthony PERARD >> >> > > Yes I think Andrew is right, so I guess $(XEN_CFLAGS) can be dropped? > > Reviewed-by: Luca Fancellu > Tested-by: Luca Fancellu > > I’ve tested this patch with and without the $(XEN_CFLAGS), so if you drop it > you can > retain my r-by if you want. Acked-by: Jan Beulich preferably with the $(CFLAGS) dropped, which again I'd be happy to do while committing. Jan
Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line
On 23.05.2023 20:14, Andrew Cooper wrote: > On 23/05/2023 5:38 pm, Anthony PERARD wrote: >> CFLAGS is just from Config.mk, instead use the flags used to build >> Xen. >> >> Signed-off-by: Anthony PERARD >> --- >> >> Notes: >> I don't know if CFLAGS is even useful there, just --version without the >> flags might produce the same result. > > I can't think of any legitimate reason for CFLAGS to be here. > > Any compiler which does differ its output based on CLFAGS is probably > one we don't want to be using... Well, I wouldn't go quite as far in general, but I agree for the --version case. Actually at least with gcc it's even "better": I've tried a couple of 32-bit compilers with "-m64 --version", which would normally choke on the -m64. But that options is ignored altogether when --version is there. (Which has up- and downsides of course; the command failing might also be useful, in telling us that the compiler isn't usable in the first place.) Jan
Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line
> On 23 May 2023, at 17:38, Anthony PERARD wrote: > > CFLAGS is just from Config.mk, instead use the flags used to build > Xen. > > Signed-off-by: Anthony PERARD > --- > > Notes: >I don't know if CFLAGS is even useful there, just --version without the >flags might produce the same result. > > xen/build.mk | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/build.mk b/xen/build.mk > index e2a78aa806..d468bb6e26 100644 > --- a/xen/build.mk > +++ b/xen/build.mk > @@ -23,7 +23,7 @@ define cmd_compile.h >-e 's/@@whoami@@/$(XEN_WHOAMI)/g' \ >-e 's/@@domain@@/$(XEN_DOMAIN)/g' \ >-e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \ > --e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \ > +-e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head > -1)!g' \ >-e 's/@@version@@/$(XEN_VERSION)/g' \ >-e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \ >-e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \ > -- > Anthony PERARD > > Yes I think Andrew is right, so I guess $(XEN_CFLAGS) can be dropped? Reviewed-by: Luca Fancellu Tested-by: Luca Fancellu I’ve tested this patch with and without the $(XEN_CFLAGS), so if you drop it you can retain my r-by if you want.
Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line
On 23/05/2023 5:38 pm, Anthony PERARD wrote: > CFLAGS is just from Config.mk, instead use the flags used to build > Xen. > > Signed-off-by: Anthony PERARD > --- > > Notes: > I don't know if CFLAGS is even useful there, just --version without the > flags might produce the same result. I can't think of any legitimate reason for CFLAGS to be here. Any compiler which does differ its output based on CLFAGS is probably one we don't want to be using... ~Andrew
[XEN PATCH 13/15] build: fix compile.h compiler version command line
CFLAGS is just from Config.mk, instead use the flags used to build Xen. Signed-off-by: Anthony PERARD --- Notes: I don't know if CFLAGS is even useful there, just --version without the flags might produce the same result. xen/build.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/build.mk b/xen/build.mk index e2a78aa806..d468bb6e26 100644 --- a/xen/build.mk +++ b/xen/build.mk @@ -23,7 +23,7 @@ define cmd_compile.h -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \ -e 's/@@domain@@/$(XEN_DOMAIN)/g' \ -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \ - -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \ + -e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head -1)!g' \ -e 's/@@version@@/$(XEN_VERSION)/g' \ -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \ -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \ -- Anthony PERARD
