Re: [PATCH 2/2] system: Restrict libpmem and libdaxctl CPPFLAGS to physmem.c

[Philippe Mathieu-Daudé]

On 12/12/24 19:55, BALATON Zoltan wrote:
> On Thu, 12 Dec 2024, Philippe Mathieu-Daudé wrote:
> > On 12/12/24 14:11, BALATON Zoltan wrote:
> > > On Thu, 12 Dec 2024, Philippe Mathieu-Daudé wrote:
> > > > Only physmem.c includes libpmem and libdaxctl headers.
> > > > No need to pass them to all system_ss[] files.
> > >
> > > I think doing this patch first would leave the other one unnecessary so 
> >
> > This one is about libpmem / libdaxctl in physmem.c,
> > the previous one is about libsdl in vl.c. I'm missing
> > what inverting the order would change.
>
> It seems odd to add the libpmem, libdaxctl libs to one set in the first 
> patch then remove it right away in the next patch. Swapping patches 
> would avoid that and move these to the final place without churn then 
> the sdl change is simpler.
>
> > Besides in 2 patches it is simpler to check what CPPFLAGS are applied.
> >
> > Anyhow if you insist, I can squash. I don't care much as long as
> > we reduce the flags applied to system_ss[].
>
> I don't insist and don't mind much either but I see others also 
> suggested squashing patches so this will be resolved by that.

Nah, you are right, I didn't notice. Besides, Paolo and Richard
showed me I don't understand clearly meson dependencies. My bad.

> Regards,
> BALATON Zoltan
>
> > > you could do both in one patch with less churn and maybe reduce this 
> > > series to a single patch.
> > >
> > > Regards,
> > > BALATON Zoltan
> > >
> > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > > ---
> > > > system/meson.build | 7 +++++--
> > > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/system/meson.build b/system/meson.build
> > > > index f7e2c8b826f..50d915bd80c 100644
> > > > --- a/system/meson.build
> > > > +++ b/system/meson.build
> > > > @@ -2,10 +2,13 @@ specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', 
> > > > if_true: [files(
> > > >   'arch_init.c',
> > > >   'ioport.c',
> > > >   'memory.c',
> > > > -  'physmem.c',
> > > >   'watchpoint.c',
> > > > )])
> > > >
> > > > +specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
> > > > +  'physmem.c',
> > > > +), libpmem, libdaxctl])
> > > > +
> > > > system_ss.add(files(
> > > >   'balloon.c',
> > > >   'bootdevice.c',
> > > > @@ -23,7 +26,7 @@ system_ss.add(files(
> > > >   'runstate-hmp-cmds.c',
> > > >   'runstate.c',
> > > >   'tpm-hmp-cmds.c',
> > > > -), libpmem, libdaxctl)
> > > > +))
> > > >
> > > > system_ss.add(files(
> > > >   'vl.c',