[Buildroot] [PATCH] new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <jwoltz@gmail.com>

[Buildroot] [PATCH] new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <jwoltz@gmail.com>

[J.C. Woltz]

From: "J.C. Woltz" <jwoltz@gmail.com>

---
 package/Config.in                                  |    1 +
 package/usb_modeswitch_data/Config.in              |    7 +++++
 .../usb_modeswitch_data-makefile.patch             |   24 ++++++++++++++++++
 package/usb_modeswitch_data/usb_modeswitch_data.mk |   26 ++++++++++++++++++++
 4 files changed, 58 insertions(+), 0 deletions(-)
 create mode 100644 package/usb_modeswitch_data/Config.in
 create mode 100644 package/usb_modeswitch_data/usb_modeswitch_data-makefile.patch
 create mode 100644 package/usb_modeswitch_data/usb_modeswitch_data.mk

diff --git a/package/Config.in b/package/Config.in
index c679652..ff11cd0 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -220,6 +220,7 @@ source "package/uboot-tools/Config.in"
 source "package/udev/Config.in"
 source "package/unionfs/Config.in"
 source "package/usb_modeswitch/Config.in"
+source "package/usb_modeswitch_data/Config.in"
 source "package/usbmount/Config.in"
 source "package/usbutils/Config.in"
 source "package/wipe/Config.in"
diff --git a/package/usb_modeswitch_data/Config.in b/package/usb_modeswitch_data/Config.in
new file mode 100644
index 0000000..cd584ce
--- /dev/null
+++ b/package/usb_modeswitch_data/Config.in
@@ -0,0 +1,7 @@
+config BR2_PACKAGE_USB_MODESWITCH_DATA
+	bool "usb_modeswitch_data"
+	select BR2_PACKAGE_USB_MODESWITCH
+	help
+	  USB mode switcher data
+	  Contains udev rules and other data 
+
diff --git a/package/usb_modeswitch_data/usb_modeswitch_data-makefile.patch b/package/usb_modeswitch_data/usb_modeswitch_data-makefile.patch
new file mode 100644
index 0000000..024bc76
--- /dev/null
+++ b/package/usb_modeswitch_data/usb_modeswitch_data-makefile.patch
@@ -0,0 +1,24 @@
+diff -Nura usb_modeswitch_data-20120120.orig/Makefile usb_modeswitch_data-20120120/Makefile
+--- usb_modeswitch_data-20120120.orig/Makefile	2012-01-20 17:25:32.000000000 -0500
++++ usb_modeswitch_data-20120120/Makefile	2012-02-06 14:20:47.000000000 -0500
+@@ -12,9 +12,9 @@
+ 
+ clean:
+ 
+-install: files-install db-install rules-reload
++install: files-install db-install
+ 
+-install-packed: files-install db-install-packed rules-reload
++install-packed: files-install db-install-packed
+ 
+ files-install:
+ 	install -d $(PREFIX)/share/usb_modeswitch
+@@ -50,7 +50,7 @@
+ 		fi \
+ 	fi
+ 
+-uninstall: files-uninstall rules-reload
++uninstall: files-uninstall
+ 
+ files-uninstall:
+ 	$(RM) $(RULESDIR)/40-usb_modeswitch.rules
diff --git a/package/usb_modeswitch_data/usb_modeswitch_data.mk b/package/usb_modeswitch_data/usb_modeswitch_data.mk
new file mode 100644
index 0000000..9f1b3ff
--- /dev/null
+++ b/package/usb_modeswitch_data/usb_modeswitch_data.mk
@@ -0,0 +1,26 @@
+#############################################################
+#
+# usb_modeswitch_data
+#
+#############################################################
+
+USB_MODESWITCH_DATA_VERSION = 20120120
+USB_MODESWITCH_DATA_SOURCE = usb-modeswitch-data-$(USB_MODESWITCH_DATA_VERSION).tar.bz2
+USB_MODESWITCH_DATA_SITE = http://www.draisberghof.de/usb_modeswitch
+USB_MODESWITCH_DATA_DEPENDENCIES = usb_modeswitch
+
+define USB_MODESWITCH_DATA_BUILD_CMDS
+	$(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)" -C $(@D)
+endef
+
+define USB_MODESWITCH_DATA_INSTALL_TARGET_CMDS
+	$(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
+endef
+
+
+define USB_MODESWITCH_DATA_CLEAN_CMDS
+	$(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) clean
+endef
+
+$(eval $(call GENTARGETS))
+
-- 
1.7.5.4

[Buildroot] [PATCH] new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <jwoltz@gmail.com>

[Thomas De Schampheleire]

Hi,

On Fri, Feb 10, 2012 at 12:51 AM, J.C. Woltz <jwoltz@gmail.com> wrote:
> From: "J.C. Woltz" <jwoltz@gmail.com>

You don't need the From: line.
Instead, the Signed-off-by: should appear here, and not in the Subject line.

>
> ---
> ?package/Config.in ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? ?1 +
> ?package/usb_modeswitch_data/Config.in ? ? ? ? ? ? ?| ? ?7 +++++
> ?.../usb_modeswitch_data-makefile.patch ? ? ? ? ? ? | ? 24 ++++++++++++++++++
> ?package/usb_modeswitch_data/usb_modeswitch_data.mk | ? 26 ++++++++++++++++++++
> ?4 files changed, 58 insertions(+), 0 deletions(-)
> ?create mode 100644 package/usb_modeswitch_data/Config.in
> ?create mode 100644 package/usb_modeswitch_data/usb_modeswitch_data-makefile.patch
> ?create mode 100644 package/usb_modeswitch_data/usb_modeswitch_data.mk
>
> diff --git a/package/Config.in b/package/Config.in
> index c679652..ff11cd0 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -220,6 +220,7 @@ source "package/uboot-tools/Config.in"
> ?source "package/udev/Config.in"
> ?source "package/unionfs/Config.in"
> ?source "package/usb_modeswitch/Config.in"
> +source "package/usb_modeswitch_data/Config.in"
> ?source "package/usbmount/Config.in"
> ?source "package/usbutils/Config.in"
> ?source "package/wipe/Config.in"
> diff --git a/package/usb_modeswitch_data/Config.in b/package/usb_modeswitch_data/Config.in
> new file mode 100644
> index 0000000..cd584ce
> --- /dev/null
> +++ b/package/usb_modeswitch_data/Config.in
> @@ -0,0 +1,7 @@
> +config BR2_PACKAGE_USB_MODESWITCH_DATA
> + ? ? ? bool "usb_modeswitch_data"
> + ? ? ? select BR2_PACKAGE_USB_MODESWITCH
> + ? ? ? help
> + ? ? ? ? USB mode switcher data
> + ? ? ? ? Contains udev rules and other data
> +

I think a 'depends on' clause is more appropriate than 'select' here.

As a general rule, the help text should contain the URL to the project.

> diff --git a/package/usb_modeswitch_data/usb_modeswitch_data-makefile.patch b/package/usb_modeswitch_data/usb_modeswitch_data-makefile.patch
> new file mode 100644
> index 0000000..024bc76
> --- /dev/null
> +++ b/package/usb_modeswitch_data/usb_modeswitch_data-makefile.patch
> @@ -0,0 +1,24 @@
> +diff -Nura usb_modeswitch_data-20120120.orig/Makefile usb_modeswitch_data-20120120/Makefile
> +--- usb_modeswitch_data-20120120.orig/Makefile 2012-01-20 17:25:32.000000000 -0500
> ++++ usb_modeswitch_data-20120120/Makefile ? ? ?2012-02-06 14:20:47.000000000 -0500
> +@@ -12,9 +12,9 @@
> +
> + clean:
> +
> +-install: files-install db-install rules-reload
> ++install: files-install db-install
> +
> +-install-packed: files-install db-install-packed rules-reload
> ++install-packed: files-install db-install-packed
> +
> + files-install:
> + ? ? ? install -d $(PREFIX)/share/usb_modeswitch
> +@@ -50,7 +50,7 @@
> + ? ? ? ? ? ? ? fi \
> + ? ? ? fi
> +
> +-uninstall: files-uninstall rules-reload
> ++uninstall: files-uninstall
> +
> + files-uninstall:
> + ? ? ? $(RM) $(RULESDIR)/40-usb_modeswitch.rules

I know that it's not yet the case for many patches in the buildroot
tree, but patches should also have a short description at the top, and
a Signed-off-by: clause.


> diff --git a/package/usb_modeswitch_data/usb_modeswitch_data.mk b/package/usb_modeswitch_data/usb_modeswitch_data.mk
> new file mode 100644
> index 0000000..9f1b3ff
> --- /dev/null
> +++ b/package/usb_modeswitch_data/usb_modeswitch_data.mk
> @@ -0,0 +1,26 @@
> +#############################################################
> +#
> +# usb_modeswitch_data
> +#
> +#############################################################
> +
> +USB_MODESWITCH_DATA_VERSION = 20120120
> +USB_MODESWITCH_DATA_SOURCE = usb-modeswitch-data-$(USB_MODESWITCH_DATA_VERSION).tar.bz2
> +USB_MODESWITCH_DATA_SITE = http://www.draisberghof.de/usb_modeswitch
> +USB_MODESWITCH_DATA_DEPENDENCIES = usb_modeswitch
> +
> +define USB_MODESWITCH_DATA_BUILD_CMDS
> + ? ? ? $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)" -C $(@D)
> +endef
> +
> +define USB_MODESWITCH_DATA_INSTALL_TARGET_CMDS
> + ? ? ? $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
> +endef
> +
> +
> +define USB_MODESWITCH_DATA_CLEAN_CMDS
> + ? ? ? $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) clean
> +endef
> +
> +$(eval $(call GENTARGETS))
> +
> --
> 1.7.5.4

Best regards,
Thomas

[Buildroot] [PATCH] new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <jwoltz@gmail.com>

[J.C. Woltz]

On Fri, Feb 10, 2012 at 2:37 AM, Thomas De Schampheleire <
patrickdepinguin+buildroot@gmail.com> wrote:

> Hi,
>
> On Fri, Feb 10, 2012 at 12:51 AM, J.C. Woltz <jwoltz@gmail.com> wrote:
> > From: "J.C. Woltz" <jwoltz@gmail.com>
>
> You don't need the From: line.
> Instead, the Signed-off-by: should appear here, and not in the Subject
> line.
>

I'm sorry, I thought I used the -s switch. I'll look into not getting the
from line.


>
> I think a 'depends on' clause is more appropriate than 'select' here.
>
> As a general rule, the help text should contain the URL to the project.
>

I'll fix the URL. I thought a select was appropriate since
usb_modeswitch_data is not vital for usb_modeswitch, but
usb_modeswitch_data is nice when using udev.


> I know that it's not yet the case for many patches in the buildroot
> tree, but patches should also have a short description at the top, and
> a Signed-off-by: clause.
>
>
I can add a description. I thought I used -s, but I will make sure I do. Is
there any other changes I should make. I am not a programmer or developer,
but trying to help where I can.


>
> Best regards,
> Thomas
>

Thank you,
J.C. Woltz
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20120210/4a9d037a/attachment-0001.html>

[Buildroot] [PATCH] new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <jwoltz@gmail.com>

[Thomas De Schampheleire]

On Fri, Feb 10, 2012 at 3:21 PM, J.C. Woltz <jwoltz@gmail.com> wrote:
>
>
> On Fri, Feb 10, 2012 at 2:37 AM, Thomas De Schampheleire
> <patrickdepinguin+buildroot@gmail.com> wrote:
>>
>> Hi,
>>
>> On Fri, Feb 10, 2012 at 12:51 AM, J.C. Woltz <jwoltz@gmail.com> wrote:
>> > From: "J.C. Woltz" <jwoltz@gmail.com>
>>
>> You don't need the From: line.
>> Instead, the Signed-off-by: should appear here, and not in the Subject
>> line.
>
>
> I'm sorry, I thought I used the -s switch. I'll look into not getting the
> from line.
>
>>
>>
>> I think a 'depends on' clause is more appropriate than 'select' here.
>>
>> As a general rule, the help text should contain the URL to the project.
>
>
> I'll fix the URL. I thought a select was appropriate since
> usb_modeswitch_data is not vital for usb_modeswitch, but usb_modeswitch_data
> is nice when using udev.

+config BR2_PACKAGE_USB_MODESWITCH_DATA
+       select BR2_PACKAGE_USB_MODESWITCH

What you express with a select like this is that when you select
'usb_modeswitch_data' (which can be always selected), the
configuration script should automatically select usb_modeswitch for
you. The problem with this approach is that when usb_modeswitch
depends on something, it will not enable these dependencies.
See http://kernel.org/doc/Documentation/kbuild/kconfig-language.txt
for more information.

Since usb_modeswitch_data only makes sense when usb_modeswitch is
already enabled (at least that's what I assume), you could better use:

config BR2_PACKAGE_USB_MODESWITCH_DATA
        depends on BR2_PACKAGE_USB_MODESWITCH

With this expression, usb_modeswitch_data will not be visible unless
usb_modeswitch has already been enabled. This makes sense. Any
dependency that usb_modeswitch has will also be met when
usb_modeswitch_data has been selected.


>
>>
>> I know that it's not yet the case for many patches in the buildroot
>> tree, but patches should also have a short description at the top, and
>> a Signed-off-by: clause.
>>
>
> I can add a description. I thought I used -s, but I will make sure I do. Is
> there any other changes I should make. I am not a programmer or developer,
> but trying to help where I can.

It's nice to have people like you contribute changes, thanks for that.
I reviewed your patch and it looks good apart from the things I
mentioned, but I have not tested it.

I suggest you send it again after fixing my comments. Maybe other
people have further comments.

By the way, you may be interested in using a version control system
like git or mercurial for managing your buildroot developments. This
will make it easier to generate and mail patches.

Best regards,
Thomas

[Buildroot] [PATCH] new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <jwoltz@gmail.com>

[J.C. Woltz]

> I suggest you send it again after fixing my comments. Maybe other
> people have further comments.
>
> By the way, you may be interested in using a version control system
> like git or mercurial for managing your buildroot developments. This
> will make it easier to generate and mail patches.
>
> Best regards,
> Thomas
>

Thomas (or anyone else),

I am using git, but the from line is still in my patch. How do I remove it?
Below are the commands I am using:
git add package/Config.in
git add package/usb_modeswitch_data/
git commit -s
git format-patch --subject-prefix="PATCH v2" origin

At this point, I look at the file, but it has two from lines.

Eventually will use:
git send-email ?to=buildroot at busybox.net

Thank you,
J.C. Woltz
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20120210/aafc41cb/attachment.html>

[Buildroot] [PATCH] new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <jwoltz@gmail.com>

[Yegor Yefremov]

Am 10.02.2012 16:02, schrieb J.C. Woltz:
> 
>     I suggest you send it again after fixing my comments. Maybe other
>     people have further comments.
> 
>     By the way, you may be interested in using a version control system
>     like git or mercurial for managing your buildroot developments. This
>     will make it easier to generate and mail patches.
> 
>     Best regards,
>     Thomas
> 
> 
> Thomas (or anyone else),
> 
> I am using git, but the from line is still in my patch. How do I remove it? Below are the commands I am using:
> git add package/Config.in
> git add package/usb_modeswitch_data/
> git commit -s

try "git commit --amend" and delete everything you don't need

> git format-patch --subject-prefix="PATCH v2" origin

[Buildroot] [PATCH] new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <jwoltz@gmail.com>

[Arnout Vandecappelle]

On Friday 10 February 2012 15:46:05 Thomas De Schampheleire wrote:
>  I thought a select was appropriate since
> > usb_modeswitch_data is not vital for usb_modeswitch, but usb_modeswitch_data
> > is nice when using udev.
> 
> +config BR2_PACKAGE_USB_MODESWITCH_DATA
> +       select BR2_PACKAGE_USB_MODESWITCH
> 
> What you express with a select like this is that when you select
> 'usb_modeswitch_data' (which can be always selected), the
> configuration script should automatically select usb_modeswitch for
> you. The problem with this approach is that when usb_modeswitch
> depends on something, it will not enable these dependencies.
> See http://kernel.org/doc/Documentation/kbuild/kconfig-language.txt
> for more information.

 That's not entirely true.  select itself is transitive; it just
doesn't check if the select'd item depends on something.  But it will
recursively select other packages.  So with the select statement,
USB_MODESWITCH_DATA will select USB_MODESWITCH, which will select
LIBUSB_COMPAT, which will select LIBUSB.


> Since usb_modeswitch_data only makes sense when usb_modeswitch is
> already enabled (at least that's what I assume), you could better use:
> 
> config BR2_PACKAGE_USB_MODESWITCH_DATA
>         depends on BR2_PACKAGE_USB_MODESWITCH
> 
> With this expression, usb_modeswitch_data will not be visible unless
> usb_modeswitch has already been enabled. This makes sense. Any
> dependency that usb_modeswitch has will also be met when
> usb_modeswitch_data has been selected.

 I think the general trend is to prefer select rather than depends when
they are separate packages - precisely because this avoids the potential
inconsistencies you mentioned.  Take libusb-compat, for instance.  It's
obviously an extension of libusb, but because libusb-compat selects libusb,
it makes it easy and safe for other packages to select libusb-compat.

 Sorry for the inconsistent feedback, J.C.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F