On Wed, Jun 11, 2025 at 3:25 PM BALATON Zoltan <bala...@eik.bme.hu> wrote:
>
> On Wed, 11 Jun 2025, Stefan Hajnoczi wrote:
> > From: oltolm <oleg.tolmat...@gmail.com>
> >
> > Sorry, I forgot to cc the maintainers.
>
> Do we want comments like this end up in git log? This could have been
> fixed before a pull. Also the other pull request about uninitialised stack
> variables had hw/audio/gus twice which was pointed out by a comment before
> the pull that one of those should be different but the pull request still
> had this error. Did you miss these or aren't these important enough to fix
> before getting in git log forever or there is just no easy way to fix up
> commit messages in pull requests?
If another reviewer asks for the author to resend then I'll hold off
on merging, but I didn't see the comment about hw/audio/gus. Sorry!
I did see this "Sorry, I forgot to cc the maintainers" comment.
Although I'm not consistent, nowadays I generally do not fix these
issues when merging, provided it's a small issue that can be ignored
or understood from the context.
I don't really mind either way, so if there is a consensus that all
maintainers should be strict about this, I'm happy to join.
One related point I do have a strong opinion on is that the
qemu.git/master maintainer shouldn't be expected to do fixups on a
pull request they receive. Fixups should be done by subsystem
maintainers (and the pull request must be resent) or the original
patch authors. It doesn't scale when the qemu.git/master maintainer
has to make changes to code that they are unfamiliar with. That's not
the case here, but I just wanted to mention it because from time to
time someone requests this.
>
> Regards,
> BALATON Zoltan
>
> > The build failed when run on Windows. I replaced calls to Unix programs
> > like ´cat´, ´sed´ and ´true´ with calls to ´python´. I wrapped calls to
> > ´os.path.relpath´ in try-except because it can fail when the two paths
> > are on different drives. I made sure to convert the Windows paths to
> > Unix paths to prevent warnings in generated files.
> >
> > Signed-off-by: oltolm <oleg.tolmat...@gmail.com>
> > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
> > Acked-by: Alex Bennée <alex.ben...@linaro.org>
> > Message-id: 20250607094503.1307-2-oleg.tolmat...@gmail.com
> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> > ---
> > contrib/plugins/meson.build | 2 +-
> > plugins/meson.build | 2 +-
> > scripts/tracetool/__init__.py | 15 ++++++++++++---
> > scripts/tracetool/backend/ftrace.py | 4 +---
> > scripts/tracetool/backend/log.py | 4 +---
> > scripts/tracetool/backend/syslog.py | 4 +---
> > tests/functional/meson.build | 4 +---
> > tests/include/meson.build | 2 +-
> > tests/tcg/plugins/meson.build | 2 +-
> > trace/meson.build | 5 +++--
> > 10 files changed, 23 insertions(+), 21 deletions(-)
> >
> > diff --git a/contrib/plugins/meson.build b/contrib/plugins/meson.build
> > index fa8a426c8b..1876bc7843 100644
> > --- a/contrib/plugins/meson.build
> > +++ b/contrib/plugins/meson.build
> > @@ -24,7 +24,7 @@ endif
> > if t.length() > 0
> > alias_target('contrib-plugins', t)
> > else
> > - run_target('contrib-plugins', command: find_program('true'))
> > + run_target('contrib-plugins', command: [python, '-c', ''])
> > endif
> >
> > plugin_modules += t
> > diff --git a/plugins/meson.build b/plugins/meson.build
> > index b20edfbabc..62c991d87f 100644
> > --- a/plugins/meson.build
> > +++ b/plugins/meson.build
> > @@ -33,7 +33,7 @@ if host_os == 'windows'
> > input: qemu_plugin_symbols,
> > output: 'qemu_plugin_api.def',
> > capture: true,
> > - command: ['sed', '-e', '0,/^/s//EXPORTS/; s/[{};]//g', '@INPUT@'])
> > + command: [python, '-c', 'import fileinput, re; print("EXPORTS",
> > end=""); [print(re.sub(r"[{};]", "", line), end="") for line in
> > fileinput.input()]', '@INPUT@'])
> >
> > # then use dlltool to assemble a delaylib.
> > # The delaylib will have an "imaginary" name (qemu.exe), that is used by
> > the
> > diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> > index bc03238c0f..6dfcbf71e1 100644
> > --- a/scripts/tracetool/__init__.py
> > +++ b/scripts/tracetool/__init__.py
> > @@ -12,12 +12,14 @@
> > __email__ = "stefa...@redhat.com"
> >
> >
> > +import os
> > import re
> > import sys
> > import weakref
> > +from pathlib import PurePath
> >
> > -import tracetool.format
> > import tracetool.backend
> > +import tracetool.format
> >
> >
> > def error_write(*lines):
> > @@ -36,7 +38,7 @@ def error(*lines):
> >
> > def out_open(filename):
> > global out_filename, out_fobj
> > - out_filename = filename
> > + out_filename = posix_relpath(filename)
> > out_fobj = open(filename, 'wt')
> >
> > def out(*lines, **kwargs):
> > @@ -308,7 +310,7 @@ def build(line_str, lineno, filename):
> > fmt = [fmt_trans, fmt]
> > args = Arguments.build(groups["args"])
> >
> > - return Event(name, props, fmt, args, lineno, filename)
> > + return Event(name, props, fmt, args, lineno,
> > posix_relpath(filename))
> >
> > def __repr__(self):
> > """Evaluable string representation for this object."""
> > @@ -447,3 +449,10 @@ def generate(events, group, format, backends,
> > tracetool.backend.dtrace.PROBEPREFIX = probe_prefix
> >
> > tracetool.format.generate(events, format, backend, group)
> > +
> > +def posix_relpath(path, start=None):
> > + try:
> > + path = os.path.relpath(path, start)
> > + except ValueError:
> > + pass
> > + return PurePath(path).as_posix()
> > diff --git a/scripts/tracetool/backend/ftrace.py
> > b/scripts/tracetool/backend/ftrace.py
> > index baed2ae61c..5fa30ccc08 100644
> > --- a/scripts/tracetool/backend/ftrace.py
> > +++ b/scripts/tracetool/backend/ftrace.py
> > @@ -12,8 +12,6 @@
> > __email__ = "stefa...@redhat.com"
> >
> >
> > -import os.path
> > -
> > from tracetool import out
> >
> >
> > @@ -47,7 +45,7 @@ def generate_h(event, group):
> > args=event.args,
> > event_id="TRACE_" + event.name.upper(),
> > event_lineno=event.lineno,
> > - event_filename=os.path.relpath(event.filename),
> > + event_filename=event.filename,
> > fmt=event.fmt.rstrip("\n"),
> > argnames=argnames)
> >
> > diff --git a/scripts/tracetool/backend/log.py
> > b/scripts/tracetool/backend/log.py
> > index de27b7e62e..17ba1cd90e 100644
> > --- a/scripts/tracetool/backend/log.py
> > +++ b/scripts/tracetool/backend/log.py
> > @@ -12,8 +12,6 @@
> > __email__ = "stefa...@redhat.com"
> >
> >
> > -import os.path
> > -
> > from tracetool import out
> >
> >
> > @@ -55,7 +53,7 @@ def generate_h(event, group):
> > ' }',
> > cond=cond,
> > event_lineno=event.lineno,
> > - event_filename=os.path.relpath(event.filename),
> > + event_filename=event.filename,
> > name=event.name,
> > fmt=event.fmt.rstrip("\n"),
> > argnames=argnames)
> > diff --git a/scripts/tracetool/backend/syslog.py
> > b/scripts/tracetool/backend/syslog.py
> > index 012970f6cc..5a3a00fe31 100644
> > --- a/scripts/tracetool/backend/syslog.py
> > +++ b/scripts/tracetool/backend/syslog.py
> > @@ -12,8 +12,6 @@
> > __email__ = "stefa...@redhat.com"
> >
> >
> > -import os.path
> > -
> > from tracetool import out
> >
> >
> > @@ -43,7 +41,7 @@ def generate_h(event, group):
> > ' }',
> > cond=cond,
> > event_lineno=event.lineno,
> > - event_filename=os.path.relpath(event.filename),
> > + event_filename=event.filename,
> > name=event.name,
> > fmt=event.fmt.rstrip("\n"),
> > argnames=argnames)
> > diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> > index 557d59ddf4..4bce961c04 100644
> > --- a/tests/functional/meson.build
> > +++ b/tests/functional/meson.build
> > @@ -412,6 +412,4 @@ foreach speed : ['quick', 'thorough']
> > endforeach
> > endforeach
> >
> > -run_target('precache-functional',
> > - depends: precache_all,
> > - command: ['true'])
> > +alias_target('precache-functional', precache_all)
> > diff --git a/tests/include/meson.build b/tests/include/meson.build
> > index 9abba308fa..8e8d1ec4e6 100644
> > --- a/tests/include/meson.build
> > +++ b/tests/include/meson.build
> > @@ -13,4 +13,4 @@ test_qapi_outputs_extra = [
> > test_qapi_files_extra = custom_target('QAPI test (include)',
> > output: test_qapi_outputs_extra,
> > input: test_qapi_files,
> > - command: 'true')
> > + command: [python, '-c', ''])
> > diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
> > index 41f02f2c7f..029342282a 100644
> > --- a/tests/tcg/plugins/meson.build
> > +++ b/tests/tcg/plugins/meson.build
> > @@ -17,7 +17,7 @@ endif
> > if t.length() > 0
> > alias_target('test-plugins', t)
> > else
> > - run_target('test-plugins', command: find_program('true'))
> > + run_target('test-plugins', command: [python, '-c', ''])
> > endif
> >
> > plugin_modules += t
> > diff --git a/trace/meson.build b/trace/meson.build
> > index 3df4549355..9c42a57a05 100644
> > --- a/trace/meson.build
> > +++ b/trace/meson.build
> > @@ -4,7 +4,7 @@ trace_events_files = []
> > foreach item : [ '.' ] + trace_events_subdirs + qapi_trace_events
> > if item in qapi_trace_events
> > trace_events_file = item
> > - group_name = item.full_path().split('/')[-1].underscorify()
> > + group_name = fs.name(item).underscorify()
> > else
> > trace_events_file = meson.project_source_root() / item / 'trace-events'
> > group_name = item == '.' ? 'root' : item.underscorify()
> > @@ -57,10 +57,11 @@ foreach item : [ '.' ] + trace_events_subdirs +
> > qapi_trace_events
> > endif
> > endforeach
> >
> > +cat = [ python, '-c', 'import fileinput; [print(line, end="") for line in
> > fileinput.input()]', '@INPUT@' ]
> > trace_events_all = custom_target('trace-events-all',
> > output: 'trace-events-all',
> > input: trace_events_files,
> > - command: [ 'cat', '@INPUT@' ],
> > + command: cat,
> > capture: true,
> > install: get_option('trace_backends') != [
> > 'nop' ],
> > install_dir: qemu_datadir)
> > --
> > 2.49.0
> >
> >
> >
