| Message ID | 20190720030344.1991393-3-chutzpah@gentoo.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | trace-cmd: Fixes for packaging | expand |
Hi Patrick, Thanks for sending these. Some questions below. On Fri, 19 Jul 2019 20:03:42 -0700 Patrick McLean <chutzpah@gentoo.org> wrote: > From: Patrick McLean <patrick.mclean@sony.com> > > Currently if udis86 is detected on the system, trace-cmd automatically > uses it. This is generally a problem for packagers since if the build > machine happens to have udis86, now there is a dependency on it that may > or may not be tracked. > > This adds a NO_UDIS86 variable that can be set by the packager to > disable using udis86 completely. > > Signed-off-by: Patrick McLean <patrick.mclean@sony.com> > --- > Makefile | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 3579f27..6b85e21 100644 > --- a/Makefile > +++ b/Makefile > @@ -136,8 +136,13 @@ export NO_PYTHON > test-build = $(if $(shell sh -c 'echo "$(1)" | \ > $(CC) -o /dev/null -c -x c - > /dev/null 2>&1 && echo y'), $2) > > +ifndef NO_UDIS86 > # have udis86 disassembler library? > -udis86-flags := $(call test-build,\#include <udis86.h>,-DHAVE_UDIS86 -ludis86) > +udis86-flags := -DHAVE_UDIS86 > +udis86-ldflags := -ludis86 Can we keep the test here? If NO_UDIS86 is not defined. That is, if you compile without this define, this change assumes you have it. It's fine to force not having it, but we shouldn't force having it if we don't. > +else > +udis86-flags := -UHAVE_UDIS86 I'm not sure this is needed, We shouldn't have anything defining HAVE_UDIS86. -- Steve > +endif # NO_UDIS86 > > define BLK_TC_FLUSH_SOURCE > #include <linux/blktrace_api.h> > @@ -237,6 +242,7 @@ endif > # Append required CFLAGS > override CFLAGS += $(INCLUDES) $(PLUGIN_DIR_SQ) $(VAR_DIR) > override CFLAGS += $(udis86-flags) $(blk-flags) > +override LDFLAGS += $(udis86-ldflags) > > CMD_TARGETS = trace-cmd $(BUILD_PYTHON) >
On Fri, 19 Jul 2019 23:17:01 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > Hi Patrick, > > Thanks for sending these. Some questions below. > > On Fri, 19 Jul 2019 20:03:42 -0700 > Patrick McLean <chutzpah@gentoo.org> wrote: > > > From: Patrick McLean <patrick.mclean@sony.com> > > > > Currently if udis86 is detected on the system, trace-cmd > > automatically uses it. This is generally a problem for packagers > > since if the build machine happens to have udis86, now there is a > > dependency on it that may or may not be tracked. > > > > This adds a NO_UDIS86 variable that can be set by the packager to > > disable using udis86 completely. > > > > Signed-off-by: Patrick McLean <patrick.mclean@sony.com> > > --- > > Makefile | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index 3579f27..6b85e21 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -136,8 +136,13 @@ export NO_PYTHON > > test-build = $(if $(shell sh -c 'echo "$(1)" | \ > > $(CC) -o /dev/null -c -x c - > /dev/null 2>&1 && echo y'), > > $2) > > +ifndef NO_UDIS86 > > # have udis86 disassembler library? > > -udis86-flags := $(call test-build,\#include > > <udis86.h>,-DHAVE_UDIS86 -ludis86) +udis86-flags := -DHAVE_UDIS86 > > +udis86-ldflags := -ludis86 > > Can we keep the test here? If NO_UDIS86 is not defined. That is, if > you compile without this define, this change assumes you have it. > > It's fine to force not having it, but we shouldn't force having it if > we don't. That sounds good to me, preserve the current behaviour if nothing is defined. I will attach an updated patch. > > +else > > +udis86-flags := -UHAVE_UDIS86 > > I'm not sure this is needed, We shouldn't have anything defining > HAVE_UDIS86. Sure, I will drop it. From: Patrick McLean <patrick.mclean@sony.com> Currently if udis86 is detected on the system, trace-cmd automatically uses it. This is generally a problem for packagers since if the build machine happens to have udis86, now there is a dependency on it that may or may not be tracked. This adds a NO_UDIS86 variable that can be set by the packager to disable using udis86 completely. Signed-off-by: Patrick McLean <patrick.mclean@sony.com> --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index 3579f27..ee3d8d9 100644 --- a/Makefile +++ b/Makefile @@ -136,8 +136,11 @@ export NO_PYTHON test-build = $(if $(shell sh -c 'echo "$(1)" | \ $(CC) -o /dev/null -c -x c - > /dev/null 2>&1 && echo y'), $2) +ifndef NO_UDIS86 # have udis86 disassembler library? udis86-flags := $(call test-build,\#include <udis86.h>,-DHAVE_UDIS86 -ludis86) +udis86-ldflags := -ludis86 +endif # NO_UDIS86 define BLK_TC_FLUSH_SOURCE #include <linux/blktrace_api.h> @@ -237,6 +240,7 @@ endif # Append required CFLAGS override CFLAGS += $(INCLUDES) $(PLUGIN_DIR_SQ) $(VAR_DIR) override CFLAGS += $(udis86-flags) $(blk-flags) +override LDFLAGS += $(udis86-ldflags) CMD_TARGETS = trace-cmd $(BUILD_PYTHON)
diff --git a/Makefile b/Makefile index 3579f27..6b85e21 100644 --- a/Makefile +++ b/Makefile @@ -136,8 +136,13 @@ export NO_PYTHON test-build = $(if $(shell sh -c 'echo "$(1)" | \ $(CC) -o /dev/null -c -x c - > /dev/null 2>&1 && echo y'), $2) +ifndef NO_UDIS86 # have udis86 disassembler library? -udis86-flags := $(call test-build,\#include <udis86.h>,-DHAVE_UDIS86 -ludis86) +udis86-flags := -DHAVE_UDIS86 +udis86-ldflags := -ludis86 +else +udis86-flags := -UHAVE_UDIS86 +endif # NO_UDIS86 define BLK_TC_FLUSH_SOURCE #include <linux/blktrace_api.h> @@ -237,6 +242,7 @@ endif # Append required CFLAGS override CFLAGS += $(INCLUDES) $(PLUGIN_DIR_SQ) $(VAR_DIR) override CFLAGS += $(udis86-flags) $(blk-flags) +override LDFLAGS += $(udis86-ldflags) CMD_TARGETS = trace-cmd $(BUILD_PYTHON)