diff mbox

pacakge : add trace-cmd package

Message ID 1375192983-29277-1-git-send-email-p.floury@gmail.com
State Superseded
Headers show

Commit Message

pierre July 30, 2013, 2:03 p.m. UTC 
From: pierre <pierre.floury@gmail.com>

---
 package/Config.in            |    1 +
 package/tracecmd/Config.in   |    9 +++++++++
 package/tracecmd/tracecmd.mk |   24 ++++++++++++++++++++++++
 3 files changed, 34 insertions(+)
 create mode 100644 package/tracecmd/Config.in
 create mode 100644 package/tracecmd/tracecmd.mk

Comments

Thomas De Schampheleire July 30, 2013, 4:59 p.m. UTC | #1 
Hi Pierre,

Thanks for your contribution! Below are some comments...

On Tue, Jul 30, 2013 at 4:03 PM,  <p.floury@gmail.com> wrote:
> From: pierre <pierre.floury@gmail.com>
>
> ---
>  package/Config.in            |    1 +
>  package/tracecmd/Config.in   |    9 +++++++++
>  package/tracecmd/tracecmd.mk |   24 ++++++++++++++++++++++++
>  3 files changed, 34 insertions(+)
>  create mode 100644 package/tracecmd/Config.in
>  create mode 100644 package/tracecmd/tracecmd.mk
>
> diff --git a/package/Config.in b/package/Config.in
> index d980871..4a7bc5d 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -39,6 +39,7 @@ source "package/memstat/Config.in"
>  source "package/netperf/Config.in"
>  source "package/oprofile/Config.in"
>  source "package/perf/Config.in"
> +source "package/tracecmd/Config.in"
>  source "package/ramspeed/Config.in"
>  source "package/rt-tests/Config.in"
>  source "package/strace/Config.in"
> diff --git a/package/tracecmd/Config.in b/package/tracecmd/Config.in
> new file mode 100644
> index 0000000..cb24816
> --- /dev/null
> +++ b/package/tracecmd/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_TRACECMD
> +       bool "trace-cmd"
> +       help
> +         trace-cmd - command line reader for ftrace
> +         need to have ftrace enable into your kernel:
> +
> +         http://git.kernel.org/cgit/linux/kernel/git/rostedt/trace-cmd.git

The help text should not repeat the name of the command.
Also, please remove the colon (:) and make the second line a real
sentence, for example:
"To do anything useful with this, you should enable ftrace in your
kernel configuration"

> +
> +comment "trace-cmd - command line reader for ftrace"

This comment should not be there. We only put comments when packages
are hidden due to missing dependencies.

> diff --git a/package/tracecmd/tracecmd.mk b/package/tracecmd/tracecmd.mk
> new file mode 100644
> index 0000000..3ddff28
> --- /dev/null
> +++ b/package/tracecmd/tracecmd.mk
> @@ -0,0 +1,24 @@
> +#############################################################
> +#
> +# tracecmd
> +#
> +#############################################################
> +
> +TRACECMD_VERSION = 8c10a774f1f8586cd8b0e
> +TRACECMD_SITE = http://git.kernel.org/cgit/linux/kernel/git/rostedt/trace-cmd.git

This version is tagged, so you'd preferable use that for the version:
trace-cmd-v2.2.1

> +TRACECMD_SITE_METHOD = git
> +TRACECMD_INSTALL_STAGING = YES
> +TRACECMD_LICENSE = GPL
> +# No license file, the license is in the installed header
> +TRACECMD_LICENSE_FILES = COPYING

What do you mean with this comment 'No license file'? There is a
COPYING file, so I don't see the point.

Also, the license should be as specific as possible, so GPLv2, GPLv2+
etc. See http://buildroot.uclibc.org/downloads/manual/manual.html#legal-info-list-licenses
for the list of known abbreviations.

In the case of tracecmd, there both are GPLv2 as LGPLv2.1 files, so
both licenses should be specified, as well as the license files
COPYING and COPYING.LIB.

> +define TRACECMD_BUILD_CMDS
> +     $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)" -C $(@D) all
> +endef
> +
> +define TRACECMD_INSTALL_TARGET_CMDS
> +       $(INSTALL) -D -m 0755 $(@D)/trace-cmd $(TARGET_DIR)/usr/bin
> +       $(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/lib/trace-cmd/plugins
> +       $(INSTALL) -D -m 0755 $(@D)/plugin_*.so $(TARGET_DIR)/usr/lib/trace-cmd/plugins
> +endef
> +
> +$(eval $(generic-package))

If you send an updated version, please also fix the typo in the title:
pacakge -> package.

Best regards,
Thomas
pierre July 31, 2013, 8:56 a.m. UTC | #2 
Hi Thomas,

Thank you for your reply and your comments.

Here is a new version of the package.

Concerning the work flow, I sent the first patch via "git send-email",
which was a bit laconic.. Is it ok if I just attach the patch file to a
"regular" email ?

Best regards,

Pierre




On Tue, Jul 30, 2013 at 6:59 PM, Thomas De Schampheleire <
patrickdepinguin+buildroot@gmail.com> wrote:

> Hi Pierre,
>
> Thanks for your contribution! Below are some comments...
>
> On Tue, Jul 30, 2013 at 4:03 PM,  <p.floury@gmail.com> wrote:
> > From: pierre <pierre.floury@gmail.com>
> >
> > ---
> >  package/Config.in            |    1 +
> >  package/tracecmd/Config.in   |    9 +++++++++
> >  package/tracecmd/tracecmd.mk |   24 ++++++++++++++++++++++++
> >  3 files changed, 34 insertions(+)
> >  create mode 100644 package/tracecmd/Config.in
> >  create mode 100644 package/tracecmd/tracecmd.mk
> >
> > diff --git a/package/Config.in b/package/Config.in
> > index d980871..4a7bc5d 100644
> > --- a/package/Config.in
> > +++ b/package/Config.in
> > @@ -39,6 +39,7 @@ source "package/memstat/Config.in"
> >  source "package/netperf/Config.in"
> >  source "package/oprofile/Config.in"
> >  source "package/perf/Config.in"
> > +source "package/tracecmd/Config.in"
> >  source "package/ramspeed/Config.in"
> >  source "package/rt-tests/Config.in"
> >  source "package/strace/Config.in"
> > diff --git a/package/tracecmd/Config.in b/package/tracecmd/Config.in
> > new file mode 100644
> > index 0000000..cb24816
> > --- /dev/null
> > +++ b/package/tracecmd/Config.in
> > @@ -0,0 +1,9 @@
> > +config BR2_PACKAGE_TRACECMD
> > +       bool "trace-cmd"
> > +       help
> > +         trace-cmd - command line reader for ftrace
> > +         need to have ftrace enable into your kernel:
> > +
> > +
> http://git.kernel.org/cgit/linux/kernel/git/rostedt/trace-cmd.git
>
> The help text should not repeat the name of the command.
> Also, please remove the colon (:) and make the second line a real
> sentence, for example:
> "To do anything useful with this, you should enable ftrace in your
> kernel configuration"
>
> > +
> > +comment "trace-cmd - command line reader for ftrace"
>
> This comment should not be there. We only put comments when packages
> are hidden due to missing dependencies.
>
> > diff --git a/package/tracecmd/tracecmd.mk b/package/tracecmd/tracecmd.mk
> > new file mode 100644
> > index 0000000..3ddff28
> > --- /dev/null
> > +++ b/package/tracecmd/tracecmd.mk
> > @@ -0,0 +1,24 @@
> > +#############################################################
> > +#
> > +# tracecmd
> > +#
> > +#############################################################
> > +
> > +TRACECMD_VERSION = 8c10a774f1f8586cd8b0e
> > +TRACECMD_SITE =
> http://git.kernel.org/cgit/linux/kernel/git/rostedt/trace-cmd.git
>
> This version is tagged, so you'd preferable use that for the version:
> trace-cmd-v2.2.1
>
> > +TRACECMD_SITE_METHOD = git
> > +TRACECMD_INSTALL_STAGING = YES
> > +TRACECMD_LICENSE = GPL
> > +# No license file, the license is in the installed header
> > +TRACECMD_LICENSE_FILES = COPYING
>
> What do you mean with this comment 'No license file'? There is a
> COPYING file, so I don't see the point.
>
> Also, the license should be as specific as possible, so GPLv2, GPLv2+
> etc. See
> http://buildroot.uclibc.org/downloads/manual/manual.html#legal-info-list-licenses
> for the list of known abbreviations.
>
> In the case of tracecmd, there both are GPLv2 as LGPLv2.1 files, so
> both licenses should be specified, as well as the license files
> COPYING and COPYING.LIB.
>
> > +define TRACECMD_BUILD_CMDS
> > +     $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)" -C $(@D) all
> > +endef
> > +
> > +define TRACECMD_INSTALL_TARGET_CMDS
> > +       $(INSTALL) -D -m 0755 $(@D)/trace-cmd $(TARGET_DIR)/usr/bin
> > +       $(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/lib/trace-cmd/plugins
> > +       $(INSTALL) -D -m 0755 $(@D)/plugin_*.so
> $(TARGET_DIR)/usr/lib/trace-cmd/plugins
> > +endef
> > +
> > +$(eval $(generic-package))
>
> If you send an updated version, please also fix the typo in the title:
> pacakge -> package.
>
> Best regards,
> Thomas
>
Thomas Petazzoni July 31, 2013, 9:10 a.m. UTC | #3 
Dear pierre,

On Wed, 31 Jul 2013 10:56:00 +0200, pierre wrote:

> Concerning the work flow, I sent the first patch via "git send-email",
> which was a bit laconic.. Is it ok if I just attach the patch file to a
> "regular" email ?

No, the new version should again been sent with git send-email, with a
different subject name, such as '[PATCH v2] trace-cmd: new package',
and a changelog of the differences between v1 and v2, like:

trace-cmd: new package

This commit adds a new package for the trace-cmd tool, which does...
blabla blabla.

Signed-off-by: ....
---
Changes since v1:
 - this
 - that

Best regards,

Thomas
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index d980871..4a7bc5d 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -39,6 +39,7 @@  source "package/memstat/Config.in"
 source "package/netperf/Config.in"
 source "package/oprofile/Config.in"
 source "package/perf/Config.in"
+source "package/tracecmd/Config.in"
 source "package/ramspeed/Config.in"
 source "package/rt-tests/Config.in"
 source "package/strace/Config.in"
diff --git a/package/tracecmd/Config.in b/package/tracecmd/Config.in
new file mode 100644
index 0000000..cb24816
--- /dev/null
+++ b/package/tracecmd/Config.in
@@ -0,0 +1,9 @@ 
+config BR2_PACKAGE_TRACECMD
+	bool "trace-cmd"
+	help
+	  trace-cmd - command line reader for ftrace 
+	  need to have ftrace enable into your kernel:
+
+	  http://git.kernel.org/cgit/linux/kernel/git/rostedt/trace-cmd.git
+
+comment "trace-cmd - command line reader for ftrace"
diff --git a/package/tracecmd/tracecmd.mk b/package/tracecmd/tracecmd.mk
new file mode 100644
index 0000000..3ddff28
--- /dev/null
+++ b/package/tracecmd/tracecmd.mk
@@ -0,0 +1,24 @@ 
+#############################################################
+#
+# tracecmd
+#
+#############################################################
+
+TRACECMD_VERSION = 8c10a774f1f8586cd8b0e
+TRACECMD_SITE = http://git.kernel.org/cgit/linux/kernel/git/rostedt/trace-cmd.git
+TRACECMD_SITE_METHOD = git
+TRACECMD_INSTALL_STAGING = YES
+TRACECMD_LICENSE = GPL
+# No license file, the license is in the installed header
+TRACECMD_LICENSE_FILES = COPYING
+define TRACECMD_BUILD_CMDS
+     $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)" -C $(@D) all
+endef
+
+define TRACECMD_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/trace-cmd $(TARGET_DIR)/usr/bin
+	$(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/lib/trace-cmd/plugins
+	$(INSTALL) -D -m 0755 $(@D)/plugin_*.so $(TARGET_DIR)/usr/lib/trace-cmd/plugins
+endef
+
+$(eval $(generic-package))