diff mbox

add new package fxload

Message ID 30360444.162.1352137687822.JavaMail.rosen@pcrosen
State Superseded
Headers show

Commit Message

Jeremy Rosen Nov. 5, 2012, 5:48 p.m. UTC
Hello everybody

I recently had to add a small utility called fxload to buildroot

fxload is provided by the linux-hotplug project and is used to upload firmware into usb device
it is generally used as a helper function by udev.

This is my first patch to buildroot, so feel free to tell me if I didn't follow the proper protocol, I will gladly correct and resubmit

Regards
Jérémy Rosen



fight key loggers : write some perl using vim

Comments

Arnout Vandecappelle Nov. 6, 2012, 12:01 a.m. UTC | #1
On 11/05/12 18:48, Jeremy Rosen wrote:
> Hello everybody
>
> I recently had to add a small utility called fxload to buildroot
>
> fxload is provided by the linux-hotplug project and is used to upload firmware into usb device
> it is generally used as a helper function by udev.
>
> This is my first patch to buildroot, so feel free to tell me if I didn't follow the proper protocol, I will gladly correct and resubmit

  Here goes...

  First of all, we prefer the patches to be sent in-line rather than as
attachment.  That makes it easier to review by just replying to the mail.
git send-email is the easiest way to contribute.

  Also I'll warn you: it may take a while (months) before your patch is
accepted even if you make all necessary correction, and we may even forget
to include it in the end.  That's not because we're evil :-) but because
our contribution process is still sub-optimal.  Buildroot has grown
significantly in number of contributions over the last two years, and
our maintainer (Peter Korsgaard) is just saturated.  We're still experimenting
with ways to improve the process.

>
> Regards
> Jérémy Rosen
>
>
>
> fight key loggers : write some perl using vim
>
>
>
> 0001-add-new-package-fxload.patch
>
>
>  From a1d3bdcf48851dedeca9597816135f4c1589e37a Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Rosen?=<jeremy.rosen@openwide.fr>
> Date: Fri, 2 Nov 2012 11:43:42 +0100
> Subject: [PATCH] add new package fxload

  Add a Signed-off-by line for yourself.  This is a short way for you to
assert that you are entitled to contribute the patch under buildroot's
GPL license.  See  http://kerneltrap.org/files/Jeremy/DCO.txt for more
details.

  BTW, if you want to add additional comments that aren't supposed to
go in the git log message (e.g. "this is my first patch, please give
feedback", add it below the Signed-off-by line, separated by ---.
Then git-am will remove that part automatically.  If you send an
updated patch, it's nice if you can write in this comment section what
changed compared to the first one.  So typically you'll see:

Signed-off-by: ...
---
v2: Incorporated comments from Arnout, except for the foo because blah.


>
> ---
>   package/Config.in        |    1 +
>   package/fxload/Config.in |    9 +++++++++
>   package/fxload/fxload.mk |   20 ++++++++++++++++++++
>   3 files changed, 30 insertions(+)
>   create mode 100644 package/fxload/Config.in
>   create mode 100644 package/fxload/fxload.mk
>
> diff --git a/package/Config.in b/package/Config.in
> index 1df099b..f72a34b 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -211,6 +211,7 @@ source "package/flashrom/Config.in"
>   source "package/fconfig/Config.in"
>   source "package/fis/Config.in"
>   source "package/fmtools/Config.in"
> +source "package/fxload/Config.in"
>   source "package/gadgetfs-test/Config.in"
>   source "package/gdisk/Config.in"
>   source "package/gpsd/Config.in"
> diff --git a/package/fxload/Config.in b/package/fxload/Config.in
> new file mode 100644
> index 0000000..6fc28a7
> --- /dev/null
> +++ b/package/fxload/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_FXLOAD
> +        bool "fxload"
> +        help

  This should be indented with a single tab.

> +	  This program is conveniently able to download firmware into FX, FX2,
> +	  and FX2LP EZ-USB devices, as well as the original AnchorChips EZ-USB.
> +	  It is intended to be invoked by hotplug scripts when the unprogrammed
> +	  device appears on the bus.
> +
> +http://sourceforge.net/projects/linux-hotplug/

  The URL should be indented like the rest of the help text: 1 tab + 2 spaces.

> diff --git a/package/fxload/fxload.mk b/package/fxload/fxload.mk
> new file mode 100644
> index 0000000..a9c0249
> --- /dev/null
> +++ b/package/fxload/fxload.mk
> @@ -0,0 +1,20 @@
> +

  That empty line shouldn't be there.
> +#############################################################
> +#
> +# fxload
> +#
> +#############################################################
> +FXLOAD_VERSION = 2008_10_13
> +FXLOAD_SOURCE = fxload-$(FXLOAD_VERSION).tar.gz

  Since that is the default, we don't write that line.

> +FXLOAD_SITE =http://sourceforge.net/projects/linux-hotplug/files/fxload/$(FXLOAD_VERSION)

  sf.net URLs are normally
http://downloads.sourceforge.net/project/linux-hotplug/fxload/$(FXLOAD_VERSION)

  Maybe yours works as well, but it's nice to have the same pattern everywhere.

> +FXLOAD_LICENSE = GPLV2+

  It's GPLv2+ (small v).

  Also set FXLOAD_LICENSE_FILE = COPYING

> +
> +define FXLOAD_BUILD_CMDS
> +    $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)" -C $(@D) all

  For new packages, we try to consistently use

$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) all

but that doesn't always work...  If it doesn't, your pattern
is OK.

> +endef
> +
> +define FXLOAD_INSTALL_TARGET_CMDS
> +    $(MAKE) prefix=$(TARGET_DIR)  -C $(@D) install

  We try to call make in exactly the same way in the install commands,
so that if for whatever reason something is still compiled there, it
will be compiled correctly.


  Overall, it looks very good, though.  Congratulations!

  Regards,
  Arnout
Jeremy Rosen Nov. 6, 2012, 8:58 a.m. UTC | #2
> 
>   Here goes...
> 
>   First of all, we prefer the patches to be sent in-line rather than
>   as
> attachment.  That makes it easier to review by just replying to the
> mail.
> git send-email is the easiest way to contribute.
> 

ok, i'll do take two of the patch using git-send-email, but I'm struggling a bit with the smtp server here at work. I'll try to get it to work...


>   Also I'll warn you: it may take a while (months) before your patch
>   is
> accepted even if you make all necessary correction, and we may even
> forget
> to include it in the end.  That's not because we're evil :-) but
> because
> our contribution process is still sub-optimal.  Buildroot has grown
> significantly in number of contributions over the last two years, and
> our maintainer (Peter Korsgaard) is just saturated.  We're still
> experimenting
> with ways to improve the process.
> 

Fair enough, i'm a maintainer for other FOSS projects, so I know how that sort of problem go...

is Peter the only one comitting ? couldn't someone help with simple patches (like updating version of existing packages) 

> 
>   Add a Signed-off-by line for yourself.  This is a short way for you
>   to
> assert that you are entitled to contribute the patch under
> buildroot's
> GPL license.  See  http://kerneltrap.org/files/Jeremy/DCO.txt for
> more
> details.

ok, will do .

BTW, your link is dead, a minute of googling and I found 

http://elinux.org/Developer_Certificate_Of_Origin
and
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches

which might be the same thing or close enough to point people to the next time my noob question is asked

> 
>   BTW, if you want to add additional comments that aren't supposed to
> go in the git log message (e.g. "this is my first patch, please give
> feedback", add it below the Signed-off-by line, separated by ---.
> Then git-am will remove that part automatically.  If you send an
> updated patch, it's nice if you can write in this comment section
> what
> changed compared to the first one.  So typically you'll see:
> 
> Signed-off-by: ...
> ---
> v2: Incorporated comments from Arnout, except for the foo because
> blah.
> 

I am a bit confused how the text of the message I give to git-send-email mix up with the commit message,

should I just put my personal text after the --- line directly in the commit message (before git-send-email) ?


[snip : comments on the patch itself]

ok, i will do all that for V2

> 
>   Overall, it looks very good, though.  Congratulations!
> 

thanks, V2 will be even better :D
Alex Bradbury Nov. 6, 2012, 3:33 p.m. UTC | #3
On 6 November 2012 08:58, Jeremy Rosen <jeremy.rosen@openwide.fr> wrote:
>>   Also I'll warn you: it may take a while (months) before your patch
>>   is
>> accepted even if you make all necessary correction, and we may even
>> forget
>> to include it in the end.  That's not because we're evil :-) but
>> because
>> our contribution process is still sub-optimal.  Buildroot has grown
>> significantly in number of contributions over the last two years, and
>> our maintainer (Peter Korsgaard) is just saturated.  We're still
>> experimenting
>> with ways to improve the process.
>>
>
> Fair enough, i'm a maintainer for other FOSS projects, so I know how that sort of problem go...
>
> is Peter the only one comitting ? couldn't someone help with simple patches (like updating version of existing packages)

Note it was decided at the recent Developer Days meeting that patches
would be accepted more aggressively. Presumably this means the chances
of patches that have been appropriately cleaned up to meet the
buildroot guidelines are less likely to be accidentally dropped.

Alex
Arnout Vandecappelle Nov. 7, 2012, 12:21 a.m. UTC | #4
On 06/11/12 09:58, Jeremy Rosen wrote:
>>    Add a Signed-off-by line for yourself.  This is a short way for you
>> >     to
>> >  assert that you are entitled to contribute the patch under
>> >  buildroot's
>> >  GPL license.  Seehttp://kerneltrap.org/files/Jeremy/DCO.txt  for
>> >  more
>> >  details.
> ok, will do .
>
> BTW, your link is dead, a minute of googling and I found
>
> http://elinux.org/Developer_Certificate_Of_Origin

  Thanks!  I've updated my template with that link.

  Regards,
  Arnout
diff mbox

Patch

From a1d3bdcf48851dedeca9597816135f4c1589e37a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Rosen?= <jeremy.rosen@openwide.fr>
Date: Fri, 2 Nov 2012 11:43:42 +0100
Subject: [PATCH] add new package fxload

---
 package/Config.in        |    1 +
 package/fxload/Config.in |    9 +++++++++
 package/fxload/fxload.mk |   20 ++++++++++++++++++++
 3 files changed, 30 insertions(+)
 create mode 100644 package/fxload/Config.in
 create mode 100644 package/fxload/fxload.mk

diff --git a/package/Config.in b/package/Config.in
index 1df099b..f72a34b 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -211,6 +211,7 @@  source "package/flashrom/Config.in"
 source "package/fconfig/Config.in"
 source "package/fis/Config.in"
 source "package/fmtools/Config.in"
+source "package/fxload/Config.in"
 source "package/gadgetfs-test/Config.in"
 source "package/gdisk/Config.in"
 source "package/gpsd/Config.in"
diff --git a/package/fxload/Config.in b/package/fxload/Config.in
new file mode 100644
index 0000000..6fc28a7
--- /dev/null
+++ b/package/fxload/Config.in
@@ -0,0 +1,9 @@ 
+config BR2_PACKAGE_FXLOAD
+        bool "fxload"
+        help
+	  This program is conveniently able to download firmware into FX, FX2,
+	  and FX2LP EZ-USB devices, as well as the original AnchorChips EZ-USB.
+	  It is intended to be invoked by hotplug scripts when the unprogrammed
+	  device appears on the bus.
+
+          http://sourceforge.net/projects/linux-hotplug/
diff --git a/package/fxload/fxload.mk b/package/fxload/fxload.mk
new file mode 100644
index 0000000..a9c0249
--- /dev/null
+++ b/package/fxload/fxload.mk
@@ -0,0 +1,20 @@ 
+
+#############################################################
+#
+# fxload
+#
+#############################################################
+FXLOAD_VERSION = 2008_10_13
+FXLOAD_SOURCE = fxload-$(FXLOAD_VERSION).tar.gz
+FXLOAD_SITE = http://sourceforge.net/projects/linux-hotplug/files/fxload/$(FXLOAD_VERSION)
+FXLOAD_LICENSE = GPLV2+
+
+define FXLOAD_BUILD_CMDS
+    $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)" -C $(@D) all
+endef
+
+define FXLOAD_INSTALL_TARGET_CMDS
+    $(MAKE) prefix=$(TARGET_DIR)  -C $(@D) install
+endef
+
+$(eval $(generic-package))
-- 
1.7.10.4