new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <[email protected]>

PATCH New J.C. Woltz 4 participants 7 messages Feb 9, 2012 – Feb 11, 2012

lore

Share & tools

lore

J.C. Woltz

author

2012-02-09 23:51 UTC #0 lore raw

PATCH 1/1 new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <[email protected]>

From: "J.C. Woltz" <[email protected]>

---
 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

package/Config.in +1 -0

index c679652..ff11cd0 100644
@@ -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"

package/usb_modeswitch_data/Config.in new file +7 -0

new file mode 100644
index 0000000..cd584ce
@@ -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 
+

package/usb_modeswitch_data/usb_modeswitch_data-makefile.patch new file +24 -0

new file mode 100644
index 0000000..024bc76
@@ -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

package/usb_modeswitch_data/usb_modeswitch_data.mk new file +26 -0

new file mode 100644
index 0000000..9f1b3ff
@@ -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))
+

Signature · 1 line

1.7.5.4

J.C. Woltz

author

2012-02-10 14:21 UTC #2 lore raw

PATCH 1/1 new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <[email protected]>

Quoted 9 lines · Thomas De Schampheleire

Hi,
Quoted 1 line · J.C. Woltz
From: "J.C. Woltz" <[email protected]>
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.

Quoted 5 lines

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.

Quoted 5 lines

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.

Quoted 4 lines

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>

Thomas De Schampheleire

2012-02-10 14:46 UTC #3 lore raw

PATCH 1/1 new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <[email protected]>

Quoted 20 lines · J.C. Woltz

Quoted 9 lines · Thomas De Schampheleire
Hi,
Quoted 1 line · J.C. Woltz
From: "J.C. Woltz" <[email protected]>
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.
Quoted 2 lines

Quoted 8 lines

Quoted 3 lines
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.

Quoted 10 lines

Quoted 6 lines
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

J.C. Woltz

author

2012-02-10 15:02 UTC #4 lore raw

PATCH 1/1 new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <[email protected]>

Quoted 10 lines

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>

Yegor Yefremov

2012-02-10 15:10 UTC #5 lore raw

PATCH 1/1 new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <[email protected]>

Am 10.02.2012 16:02, schrieb J.C. Woltz:

Quoted 12 lines

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

Quoted 6 lines

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

Quoted 1 line

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

Arnout Vandecappelle

2012-02-11 16:41 UTC #6 lore raw

PATCH 1/1 new program: usb_modeswitch_data Signed-off-by: J.C. Woltz <[email protected]>

In reply to Thomas De Schampheleire

Quoted 7 lines · On Friday 10 February 2012 15:46:05 Thomas De Schampheleire wrote:

I thought a select was appropriate since
Quoted 2 lines
usb_modeswitch_data is not vital for usb_modeswitch, but usb_modeswitch_data is nice when using udev.
package/usb_modeswitch_data/Config.in new file +2 -0
+config BR2_PACKAGE_USB_MODESWITCH_DATA
+       select BR2_PACKAGE_USB_MODESWITCH

Quoted 7 lines

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.

Quoted 10 lines

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

Signature · 6 lines

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