On Mon, Aug 18, 2025 at 04:07:28PM +0100, Daniel P. Berrangé wrote: > On Thu, Aug 07, 2025 at 03:46:08PM -0400, Stefan Hajnoczi wrote: > > On Wed, Aug 06, 2025 at 05:48:29PM +0100, Daniel P. Berrangé wrote: > > > This avoids callers needing to use the UNIX-only /dev/stdout > > > workaround. > > > > > > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com> > > > --- > > > scripts/tracetool/__init__.py | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py > > > index 0f33758870..c8fd3a7ddc 100644 > > > --- a/scripts/tracetool/__init__.py > > > +++ b/scripts/tracetool/__init__.py > > > @@ -38,8 +38,12 @@ def error(*lines): > > > > > > def out_open(filename): > > > global out_filename, out_fobj > > > - out_filename = posix_relpath(filename) > > > - out_fobj = open(filename, 'wt') > > > + if filename == "-": > > > + out_filename = "[stdout]" > > > > A few lines above: > > > > out_filename = '<none>' > > out_fobj = sys.stdout > > > > Stick to '<none>' here for consistency? > > Curious - that suggests that it was intended to be able to write to > stdout by default, but tracetool.py unconditionally calls out_open() > so those default assignments are effectively dead code, unless this > internal code is called by something other than the tracetool.py main > entrypoint ? > > I guess I'd be inclined to change the global initialization to just > be 'None' to make it explicit that out_open is expected to always be > called ?
Originally the script wrote to stdout, but I added an explicit output
filename argument in commit c05012a365c2 ("tracetool: add output
filename command-line argument") because #line directives emitted by
tracetool need to know the output filename.
Your next patch tests/tracetool/tracetool-test.py uses "-" as the
output filename but leaves the existing meson.build files unchanged.
They will still specify an output filename.
This commit doesn't break anything, at least not in how this patch
series uses "-", but I see a contradiction with commit c05012a365c2
since we're now allowing the output filename to be effectively empty.
Could you avoid special casing stdout and instead pass a relative path
to the output file? The relative path is important so the test reference
output is portable across machines. Then you don't need this commit.
Stefan
>
> > > + out_fobj = sys.stdout
> > > + else:
> > > + out_filename = posix_relpath(filename)
> >
> > I have CCed Oleg in case he spots any portability issues, but I think
> > this should still work on Windows.
>
> This use of posix_relpath() was pre-existing, so there shouldn't be
> any new issues from this.
>
> >
> > > + out_fobj = open(filename, 'wt')
> > >
> > > def out(*lines, **kwargs):
> > > """Write a set of output lines.
>
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
signature.asc
Description: PGP signature
