| Message ID | 20241212092632.18538-3-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | system/meson: Restrict libSDL/libpmem/libdaxctl CPPFLAGS to vl/physmem | expand |
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 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', >
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. 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[]. > 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', >>
On 12/12/24 15:00, 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. > > 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 s/care/mind/ ;) > we reduce the flags applied to system_ss[]. > >> 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', >>> >
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. 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', >>> > >
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', >>>> >> >>
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',
Only physmem.c includes libpmem and libdaxctl headers. No need to pass them to all system_ss[] files. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- system/meson.build | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)