On Mon, Aug 18, 2025 at 01:55:48PM -0400, Stefan Hajnoczi wrote:
> 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.
If I copy the 'trace-events' file into the build-dir, then I can rely
on relative files for both the input & output, and avoid need to
support '-'.
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 :|
