Re: [PATCH 1/7] trace-cmd: Move trace-output.c into the library code

[Tzvetomir Stoyanov <tstoyanov@vmware.com>]

On Fri, Aug 9, 2019 at 2:37 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 11 Jul 2019 16:03:01 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Functions, implemented in trace-output.c file, do not depend on
> > trace-cmd application context and can be used standalone. The file
> > is moved from trace-cmd to libtracecmd. It also fixes a warning
> > while loading python modules from kernelshark:
> > ImportError: ctracecmd.so: undefined symbol: tracecmd_append_cpu_data
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  lib/trace-cmd/Makefile                     | 11 ++++++++++-
> >  {tracecmd => lib/trace-cmd}/trace-output.c |  0
> >  tracecmd/Makefile                          |  1 -
> >  3 files changed, 10 insertions(+), 2 deletions(-)
> >  rename {tracecmd => lib/trace-cmd}/trace-output.c (100%)
> >
> > diff --git a/lib/trace-cmd/Makefile b/lib/trace-cmd/Makefile
> > index 44c1332..9086eb6 100644
> > --- a/lib/trace-cmd/Makefile
> > +++ b/lib/trace-cmd/Makefile
> > @@ -4,12 +4,15 @@ include $(src)/scripts/utils.mk
> >
> >  bdir:=$(obj)/lib/trace-cmd
> >
> > -DEFAULT_TARGET = $(bdir)/libtracecmd.a
> > +TC_VERSION := $(bdir)/include/tc_version.h
> > +
> > +DEFAULT_TARGET = $(bdir)/libtracecmd.a $(TC_VERSION)
> >
> >  OBJS =
> >  OBJS += trace-hash.o
> >  OBJS += trace-hooks.o
> >  OBJS += trace-input.o
> > +OBJS += trace-output.o
> >  OBJS += trace-recorder.o
> >  OBJS += trace-util.o
> >  OBJS += trace-filter-hash.o
> > @@ -29,6 +32,9 @@ $(bdir):
> >  $(OBJS): | $(bdir)
> >  $(DEPS): | $(bdir)
> >
> > +$(TC_VERSION): force | $(bdir)/include
> > +     $(Q)$(call update_version.h)
> > +
>
> Hi Ceco,
>
> Why the addition of TC_VERISON here? It's not stated in the change log
> to why this was copied over. I ever removed all references from
> TC_VERSION and it still builds fine.
>
> -- Steve
>
There is a dependency: trace-output.c includes version.h, which
includes tc_version.h.
When trace-output.c is moved to the lib/trace-cmd directory, it
includes tc_version.h from
lib/trace-cmd/include. It makes sense, otherwise the libtracecmd will
depend on the trace-cmd
application. That's why I duplicated that TC_VERISON logic in
lib/trace-cmd/Makefile. It
forces lib/trace-cmd/include/tc_version.h to be generated on every
libtracecmd build.
Now we have two auto generated files:
    lib/trace-cmd/include/tc_version.h
    tracecmd/include/tc_version.h

May be we should move tc_version.h to be generated in include/, so we
can use the same file from
both the application and the library ?

>
> >  $(bdir)/libtracecmd.a: $(OBJS)
> >       $(Q)$(call do_build_static_lib)
> >
> > @@ -43,6 +49,8 @@ $(bdir)/trace-util.o: $(obj)/plugins/trace_plugin_dir
> >  $(DEPS): $(bdir)/.%.d: %.c
> >       $(Q)$(CC) -M $(CPPFLAGS) $(CFLAGS) $< > $@
> >
> > +$(DEPS): $(TC_VERSION)
> > +
> >  $(OBJS): $(bdir)/%.o : $(bdir)/.%.d
> >
> >  dep_includes := $(wildcard $(DEPS))
> > @@ -54,4 +62,5 @@ endif
> >  clean:
> >       $(RM) $(bdir)/*.a $(bdir)/*.so $(bdir)/*.o $(bdir)/.*.d
> >
> > +force:
> >  .PHONY: clean
> > diff --git a/tracecmd/trace-output.c b/lib/trace-cmd/trace-output.c
> > similarity index 100%
> > rename from tracecmd/trace-output.c
> > rename to lib/trace-cmd/trace-output.c
> > diff --git a/tracecmd/Makefile b/tracecmd/Makefile
> > index bcd437a..6968f83 100644
> > --- a/tracecmd/Makefile
> > +++ b/tracecmd/Makefile
> > @@ -29,7 +29,6 @@ TRACE_CMD_OBJS += trace-restore.o
> >  TRACE_CMD_OBJS += trace-check-events.o
> >  TRACE_CMD_OBJS += trace-show.o
> >  TRACE_CMD_OBJS += trace-list.o
> > -TRACE_CMD_OBJS += trace-output.o
> >  TRACE_CMD_OBJS += trace-usage.o
> >  TRACE_CMD_OBJS += trace-msg.o
> >
>


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center