Message ID | 20210403191609.1168606-1-asafka7@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [1/1] package/uftrace: new package | expand |
Asaf, All, On 2021-04-03 22:16 +0300, Asaf Kahlon spake thusly: > The uftrace tool is to trace and analyze execution of a > program written in C/C++. It was heavily inspired by the > ftrace framework of the Linux kernel (especially function > graph tracer) and supports userspace programs. > It supports various kind of commands and filters to help > analysis of the program execution and performance. Rather than duplicate the package description in the commit log, I think it makes more sense that it contains explanations about the packaging in Buildroot. See below. > Signed-off-by: Asaf Kahlon <asafka7@gmail.com> > --- > DEVELOPERS | 1 + > package/Config.in | 1 + > package/busybox/busybox. | 0 Uh? ;-) [--SNIP--] > diff --git a/package/uftrace/Config.in b/package/uftrace/Config.in > new file mode 100644 > index 0000000000..ca3041195e > --- /dev/null > +++ b/package/uftrace/Config.in > @@ -0,0 +1,12 @@ > +config BR2_PACKAGE_UFTRACE > + bool "uftrace" > + depends on BR2_arm || BR2_aarch64 || BR2_x86_64 AFAICS, it also support i386... Even though the architecture dependencies are pretty trivial today, I think it would be good to introduce a _ARCH_SUPPORTS symbol: config BR2_PACKAGE_UFTRACE_ARCH_SUPPORTS bool default y if BR2_arm default y if BR2_aarch64 default y if BR2_i386 default y if BR2_x86_64 config BR2_PACKAGE_UFTRACE bool "uftrace" depends on BR2_PACKAGE_UFTRACE_ARCH_SUPPORTS [--SNIP--] > diff --git a/package/uftrace/uftrace.mk b/package/uftrace/uftrace.mk > new file mode 100644 > index 0000000000..b696ac4c6e > --- /dev/null > +++ b/package/uftrace/uftrace.mk > @@ -0,0 +1,26 @@ > +################################################################################ > +# > +# uftrace > +# > +################################################################################ > + > +UFTRACE_VERSION = 0.9.4 > +UFTRACE_SITE = https://github.com/namhyung/uftrace/archive/refs/tags Please use the github macro; see e.g. boot/shim/shim.mk and the manual: https://buildroot.org/downloads/manual/manual.html#github-download-url > +UFTRACE_SOURCE = v0.9.4.tar.gz No need to specify _SOURCE when using the github macro. > +UFTRACE_LICENSE = GPL-2.0 > +UFTRACE_LICENSE_FILES = COPYING > + > +ifeq ($(BR2_PACKAGE_ELFUTILS),y) > +UFTRACE_DEPENDENCIES += elfutils > +endif > + > +define UFTRACE_BUILD_CMDS > + $(TARGET_CONFIGURE_OPTS) ARCH=$(BR2_ARCH) $(MAKE) -C $(@D) $(UFTRACE_MAKE_OPTS) You are not setting UFTRACE_MAKE_OPTS anywhere that I can see... > +endef > > +define UFTRACE_INSTALL_TARGET_CMDS > + $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) \ > + $(UFTRACE_MAKE_OPTS) install > +endef > + > +$(eval $(generic-package)) There is a ./configure script (but it is not autotools, so this truly is a generic-package indeed): any reason why you are not using it? A little note about it in the commit log would be nice. I was wondering if having a host variant would not be interesting too. Indeed, uftrace has a record mode, where it has a 'record' mode where it stores all the traces into a file, and replay/report/graph modes where it reads from that file to do off-sire analysis. So one might be interested in running in record mode in the target, and doing the analysis on the development machine... Thoughts? Regards, Yann E. MORIN. > -- > 2.27.0 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hello, (In addition to Yann comments) On Sun, 4 Apr 2021 11:03:38 +0200 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > +UFTRACE_LICENSE = GPL-2.0 > > +UFTRACE_LICENSE_FILES = COPYING > > + > > +ifeq ($(BR2_PACKAGE_ELFUTILS),y) > > +UFTRACE_DEPENDENCIES += elfutils > > +endif This optional dependency is a bit odd. How does the uftrace build system detects the availability of elfutils? You're not passing any specific option when elfutils is available or not available. Best regards, Thomas
Hello Thomas, On Sun, Apr 4, 2021 at 1:07 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > (In addition to Yann comments) > > On Sun, 4 Apr 2021 11:03:38 +0200 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > > +UFTRACE_LICENSE = GPL-2.0 > > > +UFTRACE_LICENSE_FILES = COPYING > > > + > > > +ifeq ($(BR2_PACKAGE_ELFUTILS),y) > > > +UFTRACE_DEPENDENCIES += elfutils > > > +endif > > This optional dependency is a bit odd. How does the uftrace build > system detects the availability of elfutils? You're not passing any > specific option when elfutils is available or not available. uftrace does it strangely - it has a bunch of C files that check if a possible extension exists. Those test files are compiled anyway, or at least uftrace tries to compile them and ignore the error in case of a failure. After that, it checks if an executable with a specific name was created. If yes, it adds the dependency. So there's no flag that enables this dependency, it just has to be installed and uftrace will try to add it. > > Best regards, > > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com Regards, Asaf
Hello Yann, On Sun, Apr 4, 2021 at 12:03 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > Asaf, All, > > On 2021-04-03 22:16 +0300, Asaf Kahlon spake thusly: > > The uftrace tool is to trace and analyze execution of a > > program written in C/C++. It was heavily inspired by the > > ftrace framework of the Linux kernel (especially function > > graph tracer) and supports userspace programs. > > It supports various kind of commands and filters to help > > analysis of the program execution and performance. > > Rather than duplicate the package description in the commit log, I > think it makes more sense that it contains explanations about the > packaging in Buildroot. See below. I'll amend the message, thanks. > > > Signed-off-by: Asaf Kahlon <asafka7@gmail.com> > > --- > > DEVELOPERS | 1 + > > package/Config.in | 1 + > > package/busybox/busybox. | 0 > > Uh? ;-) Oops :) Will be fixed. > > [--SNIP--] > > diff --git a/package/uftrace/Config.in b/package/uftrace/Config.in > > new file mode 100644 > > index 0000000000..ca3041195e > > --- /dev/null > > +++ b/package/uftrace/Config.in > > @@ -0,0 +1,12 @@ > > +config BR2_PACKAGE_UFTRACE > > + bool "uftrace" > > + depends on BR2_arm || BR2_aarch64 || BR2_x86_64 > > AFAICS, it also support i386... > > Even though the architecture dependencies are pretty trivial today, I > think it would be good to introduce a _ARCH_SUPPORTS symbol: > > config BR2_PACKAGE_UFTRACE_ARCH_SUPPORTS > bool > default y if BR2_arm > default y if BR2_aarch64 > default y if BR2_i386 > default y if BR2_x86_64 > > config BR2_PACKAGE_UFTRACE > bool "uftrace" > depends on BR2_PACKAGE_UFTRACE_ARCH_SUPPORTS > Done on v2. > [--SNIP--] > > diff --git a/package/uftrace/uftrace.mk b/package/uftrace/uftrace.mk > > new file mode 100644 > > index 0000000000..b696ac4c6e > > --- /dev/null > > +++ b/package/uftrace/uftrace.mk > > @@ -0,0 +1,26 @@ > > +################################################################################ > > +# > > +# uftrace > > +# > > +################################################################################ > > + > > +UFTRACE_VERSION = 0.9.4 > > +UFTRACE_SITE = https://github.com/namhyung/uftrace/archive/refs/tags > > Please use the github macro; see e.g. boot/shim/shim.mk and the manual: > https://buildroot.org/downloads/manual/manual.html#github-download-url > Done in v2. > > +UFTRACE_SOURCE = v0.9.4.tar.gz > > No need to specify _SOURCE when using the github macro. > > > +UFTRACE_LICENSE = GPL-2.0 > > +UFTRACE_LICENSE_FILES = COPYING > > + > > +ifeq ($(BR2_PACKAGE_ELFUTILS),y) > > +UFTRACE_DEPENDENCIES += elfutils > > +endif > > + > > +define UFTRACE_BUILD_CMDS > > + $(TARGET_CONFIGURE_OPTS) ARCH=$(BR2_ARCH) $(MAKE) -C $(@D) $(UFTRACE_MAKE_OPTS) > > You are not setting UFTRACE_MAKE_OPTS anywhere that I can see... You're right, removed. > > > +endef > > > > +define UFTRACE_INSTALL_TARGET_CMDS > > + $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) \ > > + $(UFTRACE_MAKE_OPTS) install > > +endef > > + > > +$(eval $(generic-package)) > > There is a ./configure script (but it is not autotools, so this truly is > a generic-package indeed): any reason why you are not using it? A little > note about it in the commit log would be nice. Well, the ./configure script is called from the Makefile, and actually I don't see any reason to run it separately. But I agree a proper commit message should be added in order to make the compilation process clean. > > I was wondering if having a host variant would not be interesting too. > Indeed, uftrace has a record mode, where it has a 'record' mode where it > stores all the traces into a file, and replay/report/graph modes where > it reads from that file to do off-sire analysis. So one might be > interested in running in record mode in the target, and doing the > analysis on the development machine... That's an interesting idea. I admit I didn't try this use case. I'll send a v2 soon, and I'll invest some time later to check the option to add a host. Anyway, thanks for the review! > > Thoughts? > > Regards, > Yann E. MORIN. > > > -- > > 2.27.0 > > > > _______________________________________________ > > buildroot mailing list > > buildroot@busybox.net > > http://lists.busybox.net/mailman/listinfo/buildroot > > -- > .-----------------.--------------------.------------------.--------------------. > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | > '------------------------------^-------^------------------^--------------------' Regards, Asaf.
diff --git a/DEVELOPERS b/DEVELOPERS index c6d4f1919f..33607f8b30 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -240,6 +240,7 @@ F: package/python* F: package/snmpclitools/ F: package/spdlog/ F: package/uftp/ +F: package/uftrace/ F: package/uvw/ F: package/zeromq/ diff --git a/package/Config.in b/package/Config.in index 1269bc7b51..cd5cd17576 100644 --- a/package/Config.in +++ b/package/Config.in @@ -144,6 +144,7 @@ menu "Debugging, profiling and benchmark" source "package/trace-cmd/Config.in" source "package/trinity/Config.in" source "package/uclibc-ng-test/Config.in" + source "package/uftrace/Config.in" source "package/valgrind/Config.in" source "package/vmtouch/Config.in" source "package/whetstone/Config.in" diff --git a/package/busybox/busybox. b/package/busybox/busybox. new file mode 100644 index 0000000000..e69de29bb2 diff --git a/package/uftrace/Config.in b/package/uftrace/Config.in new file mode 100644 index 0000000000..ca3041195e --- /dev/null +++ b/package/uftrace/Config.in @@ -0,0 +1,12 @@ +config BR2_PACKAGE_UFTRACE + bool "uftrace" + depends on BR2_arm || BR2_aarch64 || BR2_x86_64 + help + The uftrace tool is to trace and analyze execution of a + program written in C/C++. It was heavily inspired by the + ftrace framework of the Linux kernel (especially function + graph tracer) and supports userspace programs. + It supports various kind of commands and filters to help + analysis of the program execution and performance. + + https://github.com/namhyung/uftrace diff --git a/package/uftrace/uftrace.hash b/package/uftrace/uftrace.hash new file mode 100644 index 0000000000..be0464d8e9 --- /dev/null +++ b/package/uftrace/uftrace.hash @@ -0,0 +1,3 @@ +# Locally computed +sha256 418d30c959d3b6d0dcafd55e588a5d414a9984b30f2522a5af004a268824a5a2 v0.9.4.tar.gz +sha256 8177f97513213526df2cf6184d8ff986c675afb514d4e68a404010521b880643 COPYING diff --git a/package/uftrace/uftrace.mk b/package/uftrace/uftrace.mk new file mode 100644 index 0000000000..b696ac4c6e --- /dev/null +++ b/package/uftrace/uftrace.mk @@ -0,0 +1,26 @@ +################################################################################ +# +# uftrace +# +################################################################################ + +UFTRACE_VERSION = 0.9.4 +UFTRACE_SITE = https://github.com/namhyung/uftrace/archive/refs/tags +UFTRACE_SOURCE = v0.9.4.tar.gz +UFTRACE_LICENSE = GPL-2.0 +UFTRACE_LICENSE_FILES = COPYING + +ifeq ($(BR2_PACKAGE_ELFUTILS),y) +UFTRACE_DEPENDENCIES += elfutils +endif + +define UFTRACE_BUILD_CMDS + $(TARGET_CONFIGURE_OPTS) ARCH=$(BR2_ARCH) $(MAKE) -C $(@D) $(UFTRACE_MAKE_OPTS) +endef + +define UFTRACE_INSTALL_TARGET_CMDS + $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) \ + $(UFTRACE_MAKE_OPTS) install +endef + +$(eval $(generic-package))
The uftrace tool is to trace and analyze execution of a program written in C/C++. It was heavily inspired by the ftrace framework of the Linux kernel (especially function graph tracer) and supports userspace programs. It supports various kind of commands and filters to help analysis of the program execution and performance. Signed-off-by: Asaf Kahlon <asafka7@gmail.com> --- DEVELOPERS | 1 + package/Config.in | 1 + package/busybox/busybox. | 0 package/uftrace/Config.in | 12 ++++++++++++ package/uftrace/uftrace.hash | 3 +++ package/uftrace/uftrace.mk | 26 ++++++++++++++++++++++++++ 6 files changed, 43 insertions(+) create mode 100644 package/busybox/busybox. create mode 100644 package/uftrace/Config.in create mode 100644 package/uftrace/uftrace.hash create mode 100644 package/uftrace/uftrace.mk