| Message ID | 20230523163811.30792-14-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk | expand |
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 <anthony.perard@citrix.com> > --- > > 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
> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote: > > CFLAGS is just from Config.mk, instead use the flags used to build > Xen. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > > 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 <luca.fancellu@arm.com> Tested-by: Luca Fancellu <luca.fancellu@arm.com> 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.
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 <anthony.perard@citrix.com> >> --- >> >> 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
On 24.05.2023 11:43, Luca Fancellu wrote: > > >> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote: >> >> CFLAGS is just from Config.mk, instead use the flags used to build >> Xen. >> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> --- >> >> 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 <luca.fancellu@arm.com> > Tested-by: Luca Fancellu <luca.fancellu@arm.com> > > 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 <jbeulich@suse.com> preferably with the $(CFLAGS) dropped, which again I'd be happy to do while committing. Jan
On 24.05.2023 11:43, Luca Fancellu wrote: > > >> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote: >> >> CFLAGS is just from Config.mk, instead use the flags used to build >> Xen. >> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> --- >> >> 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 <luca.fancellu@arm.com> > Tested-by: Luca Fancellu <luca.fancellu@arm.com> > > 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
> On 30 May 2023, at 11:14, Jan Beulich <jbeulich@suse.com> wrote: > > On 24.05.2023 11:43, Luca Fancellu wrote: >> >> >>> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote: >>> >>> CFLAGS is just from Config.mk, instead use the flags used to build >>> Xen. >>> >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >>> --- >>> >>> 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 <luca.fancellu@arm.com> >> Tested-by: Luca Fancellu <luca.fancellu@arm.com> >> >> 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
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' \
CFLAGS is just from Config.mk, instead use the flags used to build Xen. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- 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(-)