usbip: add a new package
PATCH 1/1 usbip: add a new package
Add usbip tools (usbip, usbipd) from the running linux source
--- package/Config.in | 1 + package/usbip/Config.in | 9 +++++++++ package/usbip/usbip.mk | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 package/usbip/Config.in create mode 100644 package/usbip/usbip.mk
package/Config.in +1 -0
@@ -476,6 +476,7 @@ endmenu source "package/usb_modeswitch/Config.in" source "package/usb_modeswitch_data/Config.in" source "package/usbmount/Config.in" + source "package/usbip/Config.in" source "package/usbutils/Config.in" source "package/w_scan/Config.in" source "package/wf111/Config.in"
package/usbip/Config.in new file +9 -0
@@ -0,0 +1,9 @@ +config BR2_PACKAGE_USBIP + bool "usbip" + depends on !BR2_STATIC_LIBS + help + epic stuff + +if BR2_PACKAGE_USBIP + +endif
package/usbip/usbip.mk new file +18 -0
@@ -0,0 +1,18 @@ +################################################################################ +# +# usbib +# +################################################################################ + +USBIP_SITE = $(LINUX_DIR)/tools/usb/usbip +USBIP_SITE_METHOD = local +USBIP_LICENSE = GPLv2+ +USBIP_LICENSE_FILES = COPYING +USBIP_INSTALL_STAGING = YES +USBIP_DEPENDENCIES = linux + +USBIP_AUTORECONF = YES + +USBIP_DEPENDENCIES += host-automake host-autoconf host-libtool + +$(eval $(autotools-package))
Signature · 1 line
2.7.4
PATCH 1/1 usbip: add a new package
Hello,
Quoted 3 lines · Tal Shorer
Add usbip tools (usbip, usbipd) from the running linux source
Signed-off-by Tal Shorer <[email protected]>
Thanks for your contribution. However, since usbip tools is provided as part of the Linux kernel source code, it should be supported by the package/linux-tools/ package, which already handles similar tools (perf and others).
However, it is true that the linux-tools infrastructure currently only handles tools that just need to be built/installed, and not tools that use autoconf/automake. So most likely, we want to extend package/linux-tools/linux-tools.mk? Or should we, because usbip is different, have a separate package?
Cc'ing Yann E. Morin, who did the work on the linux-tools package.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 usbip: add a new package
Quoted 12 lines · Thomas Petazzoni
Hello,
Quoted 3 lines · Tal Shorer
Add usbip tools (usbip, usbipd) from the running linux source
Signed-off-by Tal Shorer <[email protected]>Thanks for your contribution. However, since usbip tools is provided as part of the Linux kernel source code, it should be supported by the package/linux-tools/ package, which already handles similar tools (perf and others).
Quoted 5 lines
However, it is true that the linux-tools infrastructure currently only handles tools that just need to be built/installed, and not tools that use autoconf/automake. So most likely, we want to extend
package/linux-tools/linux-tools.mk? Or should we, because usbip is different, have a separate package?
Thanks for your reply.
I managed to compile usbip as part of linux-tools, but it's quite a hack, doing configuration stuff in USBIP_BUILD_CMDS before actually calling make. The other option I see is defining LINUX_TOOLS_POST_CONFIGURE_HOOKS and doing that there, but it's still duplicating what's already being done in pkg-autotools.
Either way, I noticed I sent a silly placeholder for the help string in Config.in in my patch, so even if you decide to have usbip external to linux-tools, don't apply just yet . I'll conjure a real help string and send a v2 soon :)
PATCH 1/1 usbip: add a new package
Tal, All,
On 2016-12-10 20:39 +0200, Tal Shorer spake thusly:
Quoted 19 lines
Quoted 12 lines · Thomas Petazzoni
Hello,
Quoted 3 lines · Tal Shorer
Add usbip tools (usbip, usbipd) from the running linux source
Signed-off-by Tal Shorer <[email protected]>Thanks for your contribution. However, since usbip tools is provided as part of the Linux kernel source code, it should be supported by the package/linux-tools/ package, which already handles similar tools (perf and others).
Quoted 5 lines
However, it is true that the linux-tools infrastructure currently only handles tools that just need to be built/installed, and not tools that use autoconf/automake. So most likely, we want to extend
package/linux-tools/linux-tools.mk? Or should we, because usbip is different, have a separate package?
Quoted 7 lines
Thanks for your reply.
I managed to compile usbip as part of linux-tools, but it's quite a hack, doing configuration stuff in
USBIP_BUILD_CMDSbefore actually calling make. The other option I see is definingLINUX_TOOLS_POST_CONFIGURE_HOOKSand doing that there, but it's still duplicating what's already being done in pkg-autotools.
Yes, that's not clean...
However, I think it is netter to introduce this configure step in the linux-tools infra. Then you can hand-write a configure rule for usbip.
Then, we can see whether we make this infra more like any other package infra. But I'm warry to add even more complexity just for a single package...
So, could you please send a patch series that;
1- introduces LINUX_TOOLS_POST_CONFIGURE_HOOKS, 2- adds usbip with hand-written commands, duplicated from
pkg-autotools.
On my side, I'll see what we can do about the infra itself and see if we can re-use the existing infra...
Regards,
Yann E. MORIN.
Quoted 8 lines
Either way, I noticed I sent a silly placeholder for the help string in Config.in in my patch, so even if you decide to have usbip external to linux-tools, don't apply just yet . I'll conjure a real help string and send a v2 soon :)
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
In reply to Tal ShorerTal, Thomas, All,
On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
Quoted 1 line
Add usbip tools (usbip, usbipd) from the running linux source
Adter discussin on IRC with Thomas, we've concluded that the approach you originally took is correct: it is much easier to treat it as a separate package.
However, here are a few comments....
Quoted 14 lines
Signed-off-by Tal Shorer <[email protected]>--- package/Config.in | 1 + package/usbip/Config.in | 9 +++++++++ package/usbip/usbip.mk | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 package/usbip/Config.in create mode 100644 package/usbip/usbip.mkpackage/Config.in +0
-0
package/Config.in endmenu · line 479
Quoted 4 lines
package/Config.in +1
-0@@ -476,6 +476,7 @@ endmenu source "package/usb_modeswitch/Config.in" source "package/usb_modeswitch_data/Config.in" source "package/usbmount/Config.in" + source "package/usbip/Config.in"
Even though we use a separate package, I wonder if the option should still be within the linux-tools menu, to avoid people wondering what is so different with that one tool, compared to the others.
So, maybe keep the prompt in package/linux-tools/Config.in:
# Here only for the menuconfig; it's a real package config BR2_PACKAGE_USBIP
bool "usbip"
and in package/usbip/Config.in:
# Prompt in the linux-tools package
config BR2_PACKAGE_USBIP
Thomas?
package/usbip/Config.in line 3
Quoted 12 lines
package/Config.in +0
-0source "package/usbutils/Config.in" source "package/w_scan/Config.in" source "package/wf111/Config.in"package/usbip/Config.in new file +3
-0@@ -0,0 +1,9 @@ +config BR2_PACKAGE_USBIP + bool "usbip" + depends on !BR2_STATIC_LIBS
Why does it need to depend on !static ?
You forgot to depend on a kernel being enabled. But if you move the prompt to the linux-tools package, it is automatically done.
package/usbip/Config.in line 5
Quoted 2 lines
package/usbip/Config.in new file +2
-0+ help + epic stuff
You already spot this help text. ;-)
package/usbip/Config.in line 9
Quoted 3 lines
package/usbip/Config.in new file +3
-0+if BR2_PACKAGE_USBIP + +endif
No option, so no need for this if-block.
However, there is a need for it, see below.
package/usbip/usbip.mk line 7
Quoted 13 lines
package/usbip/usbip.mk new file +7
-0@@ -0,0 +1,18 @@ +################################################################################ +# +# usbib +# +################################################################################ + +USBIP_SITE = $(LINUX_DIR)/tools/usb/usbip
This location is only valid since linux-3.17. Before that, it was in drivers/staging/usbip/userspace/ so maybe you want to allow for the two cases.
In Config.in:
if BR2_PACKAGE_USBIP
config BR2_PACKAGE_USBIP_3_17_OR_LATER
bool "Linux kernel >= 3.17"
endifAnd then in usb.mk:
ifeq ($(BR2_PACKAGE_USBIP_3_17_OR_LATER),y)
USBIP_BASE_DIR = tools/usb/usbip
else
USBIP_BASE_DIR = drivers/staging/usbip/userspace/
endif
USBIP_SITE = $(LINUX_DIR)/$(USBIP_BASE_DIR)
define USBIP_CHECK_SRC
if [ ! -d $(USBIP_SITE) ]; then \
echo "Your kernel does not have usbip in $(USBIP_BASE_DIR)" >&2; \
exit 1; \
fi
endef
USBIP_PRE_EXTRACT_HOOKS += USBIP_CHECK_SRCpackage/usbip/usbip.mk line 12
Quoted 5 lines
package/usbip/usbip.mk new file +5
-0+USBIP_SITE_METHOD = local +USBIP_LICENSE = GPLv2+ +USBIP_LICENSE_FILES = COPYING +USBIP_INSTALL_STAGING = YES +USBIP_DEPENDENCIES = linux
I think it is better to use USBIP_PATCH_DEPENDENCIES here, because we don't really need the kernel to be built; we just need it to be extracted and patched.
For the records, we introduced the linux-tools package because we encountered a circular dependency chain, and we want to avoid that in the future; see 20b1446 (linux/tools: make it a real, separate package) for the explanations.
package/usbip/usbip.mk line 16
Quoted 4 lines
package/usbip/usbip.mk new file +4
-0+ +USBIP_AUTORECONF = YES + +USBIP_DEPENDENCIES += host-automake host-autoconf host-libtool
You don't need those dependencies; they are brought in automatically by the autotools-package infrastructure.
Regards,
Yann E. MORIN.
Quoted 8 lines
package/usbip/usbip.mk new file +1
-0+$(eval $(autotools-package))Signature · 6 lines
2.7.4 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
Quoted 97 lines · Yann E. MORIN
Tal, Thomas, All,
On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
Quoted 1 line
Add usbip tools (usbip, usbipd) from the running linux source
Adter discussin on IRC with Thomas, we've concluded that the approach you originally took is correct: it is much easier to treat it as a separate package.
However, here are a few comments....
Quoted 14 lines
Signed-off-by Tal Shorer <[email protected]>--- package/Config.in | 1 + package/usbip/Config.in | 9 +++++++++ package/usbip/usbip.mk | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 package/usbip/Config.in create mode 100644 package/usbip/usbip.mkpackage/Config.in +0
-0package/Config.in endmenu · line 479
Quoted 4 lines
package/Config.in +1
-0@@ -476,6 +476,7 @@ endmenu source "package/usb_modeswitch/Config.in" source "package/usb_modeswitch_data/Config.in" source "package/usbmount/Config.in" + source "package/usbip/Config.in"Even though we use a separate package, I wonder if the option should still be within the linux-tools menu, to avoid people wondering what is so different with that one tool, compared to the others.
So, maybe keep the prompt in package/linux-tools/Config.in:
# Here only for the menuconfig; it's a real package config
BR2_PACKAGE_USBIP
bool "usbip"and in package/usbip/Config.in:
# Prompt in the linux-tools package
configBR2_PACKAGE_USBIPThomas?
package/usbip/Config.in line 3
Quoted 12 lines
package/Config.in +0
-0source "package/usbutils/Config.in" source "package/w_scan/Config.in" source "package/wf111/Config.in"package/usbip/Config.in new file +3
-0@@ -0,0 +1,9 @@ +config BR2_PACKAGE_USBIP + bool "usbip" + depends on !BR2_STATIC_LIBSWhy does it need to depend on !static ?
You forgot to depend on a kernel being enabled. But if you move the prompt to the linux-tools package, it is automatically done.
package/usbip/Config.in line 5
Quoted 2 lines
package/usbip/Config.in new file +2
-0+ help + epic stuffYou already spot this help text. ;-)
package/usbip/Config.in line 9
Quoted 3 lines
package/usbip/Config.in new file +3
-0+if BR2_PACKAGE_USBIP + +endifNo option, so no need for this if-block.
However, there is a need for it, see below.
package/usbip/usbip.mk line 7
Quoted 13 lines
package/usbip/usbip.mk new file +7
-0@@ -0,0 +1,18 @@ +################################################################################ +# +# usbib +# +################################################################################ + +USBIP_SITE = $(LINUX_DIR)/tools/usb/usbipThis location is only valid since linux-3.17. Before that, it was in
drivers/staging/usbip/userspace/so maybe you want to allow for the two cases.
In Config.in:
Quoted 4 lines
if BR2_PACKAGE_USBIP
config BR2_PACKAGE_USBIP_3_17_OR_LATER
bool "Linux kernel >= 3.17"I did this, but the other way around. The option is whether or not to use the _old_ path, so the unsuspecting user will get the new path for compatibility. Is this ok?
Quoted 5 lines
endif
And then in usb.mk:
Quoted 15 lines
ifeq ($(BR2_PACKAGE_USBIP_3_17_OR_LATER),y)
USBIP_BASE_DIR = tools/usb/usbip
else
USBIP_BASE_DIR = drivers/staging/usbip/userspace/
endif
USBIP_SITE = $(LINUX_DIR)/$(USBIP_BASE_DIR)
define USBIP_CHECK_SRC
if [ ! -d $(USBIP_SITE) ]; then \
echo "Your kernel does not have usbip in $(USBIP_BASE_DIR)" >&2; \
exit 1; \
fi
endef
USBIP_PRE_EXTRACT_HOOKS += USBIP_CHECK_SRCI tried getting this hook to run, but since the site method is "local", the package just gets rsynced. However, trying to add this to USBIP_PRE_RSYNC_HOOKS doesn't help either because the rsync rule checks for the source directory before calling the PRE_RSYNC_HOOKS. The error this outputs to the user is as follows:
tal at tal-N552VW-FY117T:~/Dev/lfs/buildroot|0 $ make usbip
Quoted 1 line
Quoted 1 line
Quoted 1 line
usbip Syncing from source dir
/home/tal/Dev/lfs/buildroot/output/build/linux-master/drivers/staging/usbip/userspace
ERROR: /home/tal/Dev/lfs/buildroot/output/build/linux-master/drivers/staging/usbip/userspace does not existpackage/pkg-generic.mk:163: recipe for target '/home/tal/Dev/lfs/buildroot/output/build/usbip/.stamp_rsynced' failed make[1]: *** [/home/tal/Dev/lfs/buildroot/output/build/usbip/.stamp_rsynced] Error 1
Makefile:76: recipe for target '_all' failed
make: *** [_all] Error 2
tal at tal-N552VW-FY117T:~/Dev/lfs/buildroot|2 $
which is not as nice as the one you suggested, but I find it somewhat acceptable. Your thoughts?
Quoted 42 lines
package/usbip/usbip.mk line 12
Quoted 5 lines
package/usbip/usbip.mk new file +5
-0+USBIP_SITE_METHOD = local +USBIP_LICENSE = GPLv2+ +USBIP_LICENSE_FILES = COPYING +USBIP_INSTALL_STAGING = YES +USBIP_DEPENDENCIES = linuxI think it is better to use
USBIP_PATCH_DEPENDENCIEShere, because we don't really need the kernel to be built; we just need it to be extracted and patched.For the records, we introduced the linux-tools package because we encountered a circular dependency chain, and we want to avoid that in the future; see 20b1446 (linux/tools: make it a real, separate package) for the explanations.
package/usbip/usbip.mk line 16
Quoted 4 lines
package/usbip/usbip.mk new file +4
-0+ +USBIP_AUTORECONF = YES + +USBIP_DEPENDENCIES += host-automake host-autoconf host-libtoolYou don't need those dependencies; they are brought in automatically by the autotools-package infrastructure.
Regards,
Yann E. MORIN.Quoted 8 lines
+$(eval $(autotools-package))
--
2.7.4_______________________________________________
buildroot mailing list
buildroot at 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 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |'------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
Tal, All,
On 2016-12-11 19:39 +0200, Tal Shorer spake thusly:
Quoted 3 lines · Yann E. MORIN
Quoted 2 lines
On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
Quoted 1 line
Add usbip tools (usbip, usbipd) from the running linux source
[--SNIP--]
Quoted 19 lines
Quoted 19 lines
package/usbip/usbip.mk line 7
Quoted 13 lines
package/usbip/usbip.mk new file +7
-0@@ -0,0 +1,18 @@ +################################################################################ +# +# usbib +# +################################################################################ + +USBIP_SITE = $(LINUX_DIR)/tools/usb/usbipThis location is only valid since linux-3.17. Before that, it was in
drivers/staging/usbip/userspace/so maybe you want to allow for the two cases.
In Config.in:
Quoted 7 lines
Quoted 4 lines
if BR2_PACKAGE_USBIP config BR2_PACKAGE_USBIP_3_17_OR_LATER bool "Linux kernel >= 3.17"I did this, but the other way around. The option is whether or not to use the _old_ path, so the unsuspecting user will get the new path for compatibility. Is this ok?
Yes, totally OK. One way or the other is equivalent to me. :-)
However, you mentioned compatibility. There's no compatibility to have here in Buildroot, because there were no release which would have defaulted one way or the other. So, whatever we choose, it is OK.
Quoted 19 lines
Quoted 4 lines
endif
And then in usb.mk:
Quoted 15 lines
ifeq ($(BR2_PACKAGE_USBIP_3_17_OR_LATER),y) USBIP_BASE_DIR = tools/usb/usbip else USBIP_BASE_DIR = drivers/staging/usbip/userspace/ endif USBIP_SITE = $(LINUX_DIR)/$(USBIP_BASE_DIR) define USBIP_CHECK_SRC if [ ! -d $(USBIP_SITE) ]; then \ echo "Your kernel does not have usbip in $(USBIP_BASE_DIR)" >&2; \ exit 1; \ fi endef USBIP_PRE_EXTRACT_HOOKS += USBIP_CHECK_SRC
Quoted 4 lines
I tried getting this hook to run, but since the site method is "local", the package just gets rsynced. However, trying to add this to
USBIP_PRE_RSYNC_HOOKSdoesn't help either because the rsync rule checks for the source directory before calling thePRE_RSYNC_HOOKS.
Damn it...
I think this is a shortcoming in the rsync method: the hooks should be called even before we try to run-or-test anything. I.e. the hooks should be called before we do the test.
Care to send a patch for that?
Regards,
Yann E. MORIN.
Quoted 59 lines
The error
this outputs to the user is as follows:tal at tal-N552VW-FY117T:~/Dev/lfs/buildroot|0 $ make usbip
Quoted 1 line
Quoted 1 line
Quoted 1 line
usbip Syncing from source dir
/home/tal/Dev/lfs/buildroot/output/build/linux-master/drivers/staging/usbip/userspaceERROR:
/home/tal/Dev/lfs/buildroot/output/build/linux-master/drivers/staging/usbip/userspacedoes not existpackage/pkg-generic.mk:163: recipe for target '/home/tal/Dev/lfs/buildroot/output/build/usbip/.stamp_rsynced' failed make[1]: *** [/home/tal/Dev/lfs/buildroot/output/build/usbip/.stamp_rsynced] Error 1
Makefile:76: recipe for target '_all' failed
make: *** [_all] Error 2
tal at tal-N552VW-FY117T:~/Dev/lfs/buildroot|2 $which is not as nice as the one you suggested, but I find it somewhat acceptable. Your thoughts?
Quoted 42 lines
package/usbip/usbip.mk line 12
Quoted 5 lines
package/usbip/usbip.mk new file +5
-0+USBIP_SITE_METHOD = local +USBIP_LICENSE = GPLv2+ +USBIP_LICENSE_FILES = COPYING +USBIP_INSTALL_STAGING = YES +USBIP_DEPENDENCIES = linuxI think it is better to use
USBIP_PATCH_DEPENDENCIEShere, because we don't really need the kernel to be built; we just need it to be extracted and patched.For the records, we introduced the linux-tools package because we encountered a circular dependency chain, and we want to avoid that in the future; see 20b1446 (linux/tools: make it a real, separate package) for the explanations.
package/usbip/usbip.mk line 16
Quoted 4 lines
package/usbip/usbip.mk new file +4
-0+ +USBIP_AUTORECONF = YES + +USBIP_DEPENDENCIES += host-automake host-autoconf host-libtoolYou don't need those dependencies; they are brought in automatically by the autotools-package infrastructure.
Regards,
Yann E. MORIN.Quoted 8 lines
+$(eval $(autotools-package))
--
2.7.4_______________________________________________
buildroot mailing list
buildroot at 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 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |'------------------------------^-------^------------------^--------------------'
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR)
This will allow packages to define their pre-rsync hooks which will be guaranteed to run even if the source is missing.
--- package/pkg-generic.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
package/pkg-generic.mk +1 -1
@@ -161,8 +161,8 @@ $(BUILD_DIR)/%/.stamp_extracted: # used. $(BUILD_DIR)/%/.stamp_rsynced: @$(call MESSAGE,"Syncing from source dir $(SRCDIR)") - @test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1) $(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep)) + @test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1) rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $(call qstrip,$(SRCDIR))/ $(@D) $(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep)) $(Q)touch $@
Signature · 1 line
2.7.4
PATCH 1/1 pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR)
Tal, All,
On 2016-12-11 20:12 +0200, Tal Shorer spake thusly:
Quoted 4 lines
This will allow packages to define their pre-rsync hooks which will be guaranteed to run even if the source is missing.
Signed-off-by Tal Shorer <[email protected]>
Before this goes in, we have to notice that packages (most probably, out-of-tree packages in a br2-external tree) that defined hooks that relied on the check to be done before the hooks are called, may now break with this change.
However, I still believe this change is good, because it ensures the hooks are called before the infra does anything, which is what we do in all the other similar commands; also, there are valid cases where we want the hooks to run first.
Regards,
Yann E. MORIN.
Quoted 25 lines
--- package/pkg-generic.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)package/pkg-generic.mk +1
-1@@ -161,8 +161,8 @@ $(BUILD_DIR)/%/.stamp_extracted: # used. $(BUILD_DIR)/%/.stamp_rsynced: @$(call MESSAGE,"Syncing from source dir $(SRCDIR)") - @test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1) $(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep)) + @test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1) rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $(call qstrip,$(SRCDIR))/ $(@D) $(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep)) $(Q)touch $@Signature · 6 lines
2.7.4 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 package/usbip: new package
In reply to Tal Shorerusbip is part of the linux tree, so it should be part of the linux-tools infra. However, it uses autotools, which are rather difficult to use in the linux-tools infra.
So we make it a proper, separate autotools package. We only rely on the kernel to be extracted and use that as the source.
---
Note: in order for the user-friendly error to appear when the source directory is wrong (depends on a config item, was moved in linux-3.17), it requires that the patch to fix the hook order in the rsync command (https://patchwork.ozlabs.org/patch/704859/) goes in first.
--- package/linux-tools/Config.in | 2 ++ package/usbip/Config.in | 14 ++++++++++++++ package/usbip/usbip.mk | 29 +++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 package/usbip/Config.in create mode 100644 package/usbip/usbip.mk
package/linux-tools/Config.in +2 -0
@@ -83,4 +83,6 @@ comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" depends on BR2_USE_MMU depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS +source package/usbip/Config.in + endmenu
package/usbip/Config.in new file +14 -0
@@ -0,0 +1,14 @@ +# Prompt in the linux-tools package +# Here only for the menuconfig; it's a real package +config BR2_PACKAGE_USBIP + bool"usbip" + help + usbip is a set of tools that allows machines to share their + usb devices over the network, to be driven by a remote client. + +if BR2_PACKAGE_USBIP + +config BR2_PACKAGE_USBIP_3_16_OR_EARLIER + bool "use old directory path for usbip (Linux kernel <= 3.16)" + +endif
package/usbip/usbip.mk new file +29 -0
@@ -0,0 +1,29 @@ +################################################################################ +# +# usbib +# +################################################################################ + +ifeq ($(BR2_PACKAGE_USBIP_3_16_OR_EARLIER),y) +USBIP_BASE_DIR = drivers/staging/usbip/userspace +else +USBIP_BASE_DIR = tools/usb/usbip +endif +USBIP_SITE = $(LINUX_DIR)/$(USBIP_BASE_DIR) +USBIP_SITE_METHOD = local +USBIP_LICENSE = GPLv2 +USBIP_LICENSE_FILES = COPYING +USBIP_INSTALL_STAGING = YES +USBIP_PATCH_DEPENDENCIES = linux + +USBIP_AUTORECONF = yes + +define USBIP_CHECK_SRC + @if [ ! -d $(USBIP_SITE) ]; then \ + echo "Your kernel does not have usbip in $(USBIP_BASE_DIR)" >&2; \ + exit 1; \ + fi +endef +USBIP_PRE_RSYNC_HOOKS += USBIP_CHECK_SRC + +$(eval $(autotools-package))
Signature · 1 line
2.7.4
Thomas Petazzoni
AppliedPATCH 1/1 pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR)
In reply to Tal ShorerHello,
Quoted 7 lines · Tal Shorer
This will allow packages to define their pre-rsync hooks which will be guaranteed to run even if the source is missing.
Signed-off-by Tal Shorer <[email protected]>--- package/pkg-generic.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to master, thanks.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/1 usbip: add a new package
In reply to Yann E. MORINOn Sun, Dec 11, 2016 at 7:45 PM, Yann E. MORIN <[email protected]> wrote:
[--SNIP--]
Quoted 7 lines
Quoted 4 lines
I tried getting this hook to run, but since the site method is "local", the package just gets rsynced. However, trying to add this to
USBIP_PRE_RSYNC_HOOKSdoesn't help either because the rsync rule checks for the source directory before calling thePRE_RSYNC_HOOKS.Damn it...
Quoted 9 lines
I think this is a shortcoming in the rsync method: the hooks should be called even before we try to run-or-test anything. I.e. the hooks should be called before we do the test.
Care to send a patch for that?
Regards,
Yann E. MORIN.
done. Should I wait for that patch to get noticed or should I just send what I have with PRE_RSYNC_HOOKS and expect that the error will "fix itself" when the other patch gets in?
PATCH 1/1 usbip: add a new package
Tal, All,
On 2016-12-11 20:31 +0200, Tal Shorer spake thusly:
Quoted 13 lines · Yann E. MORIN
[--SNIP--]
Quoted 11 lines
Quoted 4 lines
I tried getting this hook to run, but since the site method is "local", the package just gets rsynced. However, trying to add this to
USBIP_PRE_RSYNC_HOOKSdoesn't help either because the rsync rule checks for the source directory before calling thePRE_RSYNC_HOOKS.Damn it...
I think this is a shortcoming in the rsync method: the hooks should be called even before we try to run-or-test anything. I.e. the hooks should be called before we do the test.
Quoted 6 lines
Quoted 5 lines
Care to send a patch for that?
Regards,
Yann E. MORIN.done.
Thanks! :-)
Quoted 3 lines
Should I wait for that patch to get noticed or should I just send what I have with
PRE_RSYNC_HOOKSand expect that the error will "fix itself" when the other patch gets in?
You can just send the patch now, and add in a post-commit log that the other patch is required first, i.e. in your commit log, add a three-dash line and the message after it, like so:
package/usbip: new package
usbip is part of the linus tree, so it should be part of the linux-tools infra. However, it uses autotools, which are rather difficult to use in the linux-tools infra.
So we make it a proper, separate autotools package. We only rely omn the kernel to be extracted and use that as the source.
---
Note: this requires that the patch to fix the hook order in the rsync command (https://patchwork.ozlabs.org/patch/704859/) goes in first.
And cherry-on-the-cake, make it a reply to that patch, with --in-reply-to when sending the patch.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
In reply to Tal ShorerQuoted 4 lines · Tal Shorer
Quoted 3 lines · Yann E. MORIN
Tal, Thomas, All,
On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
[snip]
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
package/usbip/usbip.mk new file +0
-0
Quoted 8 lines
Quoted 8 lines
package/usbip/usbip.mk line 7
Quoted 8 lines
package/usbip/usbip.mk new file +7
-0@@ -0,0 +1,18 @@ +################################################################################ +# +# usbib +# +################################################################################ + +USBIP_SITE = $(LINUX_DIR)/tools/usb/usbip
There's one little problem with this approach: it breaks 'make source-check' on
a clean tree. 'make source-check' doesn't extract the source, so "test -d $$($(2)_OVERRIDE_SRCDIR)" will fail.
Quoted 6 lines
Quoted 6 lines
This location is only valid since linux-3.17. Before that, it was in
drivers/staging/usbip/userspace/so maybe you want to allow for the two cases.
In Config.in:
Quoted 26 lines
Quoted 4 lines
if BR2_PACKAGE_USBIP config BR2_PACKAGE_USBIP_3_17_OR_LATER bool "Linux kernel >= 3.17"I did this, but the other way around. The option is whether or not to use the _old_ path, so the unsuspecting user will get the new path for compatibility. Is this ok?
Quoted 19 lines
endif
And then in usb.mk:
ifeq ($(BR2_PACKAGE_USBIP_3_17_OR_LATER),y) USBIP_BASE_DIR = tools/usb/usbip else USBIP_BASE_DIR = drivers/staging/usbip/userspace/ endif USBIP_SITE = $(LINUX_DIR)/$(USBIP_BASE_DIR) define USBIP_CHECK_SRC if [ ! -d $(USBIP_SITE) ]; then \ echo "Your kernel does not have usbip in $(USBIP_BASE_DIR)" >&2; \ exit 1; \ fi endef USBIP_PRE_EXTRACT_HOOKS += USBIP_CHECK_SRC
I really don't like this approach where the user has to specify it is Linux >=
3.17. If we start introducing version symbols, we should do it for all possible versions and at the level of the BR2_KERNEL_LINUX symbol itself, similar like for the external toolchain kernel headers symbol.
However, doesn't something like this work? Or is it considered too much of a hack?
# Before v3.17 it was in staging.
# USBIP_SITE is only used inside rules, after linux has already been extracted.
USBIP_SITE = $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace)
Regards,
Arnout
Quoted 6 lines
Quoted 1 line
I tried getting this hook to run, but since the site method is "local", the package just gets rsynced. However, trying to add this to
USBIP_PRE_RSYNC_HOOKSdoesn't help either because the rsync rule checks for the source directory before calling thePRE_RSYNC_HOOKS. The error this outputs to the user is as follows:
[snip]
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 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: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/1 usbip: add a new package
Arnout, All,
On 2016-12-11 20:59 +0100, Arnout Vandecappelle spake thusly:
Quoted 15 lines · Tal Shorer
Quoted 4 lines · Yann E. MORIN
Quoted 3 lines
Tal, Thomas, All,
On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
[snip]
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
package/usbip/usbip.mk new file +3
-0@@ -0,0 +1,18 @@ +################################################################################ +# +# usbib
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
package/usbip/usbip.mk line 7
Quoted 4 lines
package/usbip/usbip.mk new file +4
-0+# +################################################################################ + +USBIP_SITE = $(LINUX_DIR)/tools/usb/usbipThere's one little problem with this approach: it breaks 'make source-check' on
a clean tree. 'make source-check' doesn't extract the source, so "test -d $$($(2)_OVERRIDE_SRCDIR)" will fail.
Hmm... Bad... :-(
But see below...
Quoted 32 lines
Quoted 5 lines
Quoted 5 lines
This location is only valid since linux-3.17. Before that, it was in
drivers/staging/usbip/userspace/so maybe you want to allow for the two cases.
In Config.in:Quoted 26 lines
Quoted 4 lines
if BR2_PACKAGE_USBIP config BR2_PACKAGE_USBIP_3_17_OR_LATER bool "Linux kernel >= 3.17"I did this, but the other way around. The option is whether or not to use the _old_ path, so the unsuspecting user will get the new path for compatibility. Is this ok?
Quoted 19 lines
endif
And then in usb.mk:
ifeq ($(BR2_PACKAGE_USBIP_3_17_OR_LATER),y) USBIP_BASE_DIR = tools/usb/usbip else USBIP_BASE_DIR = drivers/staging/usbip/userspace/ endif USBIP_SITE = $(LINUX_DIR)/$(USBIP_BASE_DIR) define USBIP_CHECK_SRC if [ ! -d $(USBIP_SITE) ]; then \ echo "Your kernel does not have usbip in $(USBIP_BASE_DIR)" >&2; \ exit 1; \ fi endef USBIP_PRE_EXTRACT_HOOKS += USBIP_CHECK_SRC
Quoted 4 lines
I really don't like this approach where the user has to specify it is Linux >=
3.17. If we start introducing version symbols, we should do it for all possible versions and at the level of theBR2_KERNEL_LINUXsymbol itself, similar like for the external toolchain kernel headers symbol.
That's quite a bit of an overhead...
Quoted 7 lines
However, doesn't something like this work? Or is it considered too much of a hack?
# Before v3.17 it was in staging.
#USBIP_SITEis only used inside rules, after linux has already been extracted.USBIP_SITE = $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace)
I like this approach as well; it is better than mine.
However, it does not really solve the source-check issue, does it?
Hmm... It probably does, because:
- in an empty build tree, the linux source are not extracted, so there would be nothing to $(wildcard), so USBIP_SITE would be empty, so the download infra would not try to download anything (fortunately, we do not enforce an empty _SITE_METHOD when _SITE is empty; we only enforce an empty _VERSION);
- in an already-extracted tree, $(wildcard would return an existign directory, so the source-check test would be happy.
So yes, this is a hack. A tricky one, for sure, but it looks like it would work.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
In reply to Arnout VandecappelleQuoted 64 lines · Arnout Vandecappelle
Quoted 4 lines · Tal Shorer
Quoted 3 lines · Yann E. MORIN
Tal, Thomas, All,
On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
[snip]
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
package/usbip/usbip.mk new file +0
-0Quoted 8 lines
Quoted 8 lines
package/usbip/usbip.mk line 7
Quoted 8 lines
package/usbip/usbip.mk new file +7
-0@@ -0,0 +1,18 @@ +################################################################################ +# +# usbib +# +################################################################################ + +USBIP_SITE = $(LINUX_DIR)/tools/usb/usbipThere's one little problem with this approach: it breaks 'make source-check' on
a clean tree. 'make source-check' doesn't extract the source, so "test -d $$($(2)_OVERRIDE_SRCDIR)" will fail.Quoted 6 lines
Quoted 6 lines
This location is only valid since linux-3.17. Before that, it was in
drivers/staging/usbip/userspace/so maybe you want to allow for the two cases.
In Config.in:Quoted 26 lines
Quoted 4 lines
if BR2_PACKAGE_USBIP config BR2_PACKAGE_USBIP_3_17_OR_LATER bool "Linux kernel >= 3.17"I did this, but the other way around. The option is whether or not to use the _old_ path, so the unsuspecting user will get the new path for compatibility. Is this ok?
Quoted 19 lines
endif
And then in usb.mk:
ifeq ($(BR2_PACKAGE_USBIP_3_17_OR_LATER),y) USBIP_BASE_DIR = tools/usb/usbip else USBIP_BASE_DIR = drivers/staging/usbip/userspace/ endif USBIP_SITE = $(LINUX_DIR)/$(USBIP_BASE_DIR) define USBIP_CHECK_SRC if [ ! -d $(USBIP_SITE) ]; then \ echo "Your kernel does not have usbip in $(USBIP_BASE_DIR)" >&2; \ exit 1; \ fi endef USBIP_PRE_EXTRACT_HOOKS += USBIP_CHECK_SRCI really don't like this approach where the user has to specify it is Linux >=
3.17. If we start introducing version symbols, we should do it for all possible versions and at the level of theBR2_KERNEL_LINUXsymbol itself, similar like for the external toolchain kernel headers symbol.
Quoted 7 lines
However, doesn't something like this work? Or is it considered too much of a hack?
# Before v3.17 it was in staging.
#USBIP_SITEis only used inside rules, after linux has already been extracted.USBIP_SITE = $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace)
It doesn't, but I fail to understand why exactly. consider a line like USBIP_SITE = $(wildcard $(LINUX_DIR)/tools/usb/usbip)
as opposed to a line likeUSBIP_SITE = $(LINUX_DIR)/tools/usb/usbip
the latter works while the former doesn't. It looks like the infra doesn't recognize that it needs to rsync (empty OVERRIDE_SRCDIR?), causing the build directory to be empty. I tried some shell commands and got the same results as the wildcard attempt. A possible "workaround" is to only use the new path, de-facto requiring linux >= 3.17, which is two years old now.
Or we could go back to being a linux-tool, because the wildcard hack works there. Maybe because we can't mess up OVERRIDE_SRCDIR this way.
Quoted 22 lines
Regards,
ArnoutQuoted 6 lines
Quoted 1 line
I tried getting this hook to run, but since the site method is "local", the package just gets rsynced. However, trying to add this to
USBIP_PRE_RSYNC_HOOKSdoesn't help either because the rsync rule checks for the source directory before calling thePRE_RSYNC_HOOKS. The error this outputs to the user is as follows:[snip]
--
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 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: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/1 usbip: add a new package
Tal, Arnout, All,
On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
Quoted 1 line · Arnout Vandecappelle
[--SNIP--]
Quoted 8 lines
Quoted 7 lines
However, doesn't something like this work? Or is it considered too much of a hack?
# Before v3.17 it was in staging.
#USBIP_SITEis only used inside rules, after linux has already been extracted.USBIP_SITE = $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace)It doesn't,
Weird, it does work here:
USBIP_SITE= $(wildcard \
$(LINUX_DIR)/tools/usb/usbip \
$(LINUX_DIR)/drivers/staging/usbip/userspace)
USBIP_SITE_METHOD = local
USBIP_PATCH_DEPENDENCIES = linux
define USBIP_CONFIGURE_CMDS
@echo 'USBIP_SITE="$(USBIP_SITE)"'
endef
$(eval $(generic-package))And then:
$ make usbip-configure
[--SNIP--]
USBIP_SITE="/home/ymorin/dev/buildroot/O/build/linux-4.8.13/tools/usb/usbip"Quid? ;-)
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
Quoted 28 lines · Yann E. MORIN
Tal, Arnout, All,
On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
Quoted 1 line · Arnout Vandecappelle
[--SNIP--]
Quoted 8 lines
Quoted 7 lines
However, doesn't something like this work? Or is it considered too much of a hack?
# Before v3.17 it was in staging.
#USBIP_SITEis only used inside rules, after linux has already been extracted.USBIP_SITE = $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace)It doesn't,
Weird, it does work here:
USBIP_SITE= $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace) USBIP_SITE_METHOD = local USBIP_PATCH_DEPENDENCIES = linux define USBIP_CONFIGURE_CMDS @echo 'USBIP_SITE="$(USBIP_SITE)"' endef $(eval $(generic-package))
Quoted 7 lines
And then:
$ make usbip-configure [--SNIP--] USBIP_SITE="/home/ymorin/dev/buildroot/O/build/linux-4.8.13/tools/usb/usbip"Quid? ;-)
Notice how it says "Extracting" in the "snip". That's a hint that it doesn't understand it needs to rsync. In the top Make, we can find:
########
include $(sort $(wildcard package/*/*.mk))
include boot/common.mk
include linux/linux.mk
include fs/common.mk
########
so pkg-autotools evaluates the wildcard before LINUX_DIR is defined
Quoted 11 lines
Regards,
Yann E. MORIN.--
.-----------------.--------------------.------------------.--------------------.| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |'------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
Tal, All,
On 2016-12-12 00:11 +0200, Tal Shorer spake thusly:
Quoted 31 lines · Yann E. MORIN
Quoted 30 lines
Tal, Arnout, All,
On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
Quoted 1 line · Arnout Vandecappelle
[--SNIP--]
Quoted 8 lines
Quoted 7 lines
However, doesn't something like this work? Or is it considered too much of a hack?
# Before v3.17 it was in staging.
#USBIP_SITEis only used inside rules, after linux has already been extracted.USBIP_SITE = $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace)It doesn't,
Weird, it does work here:
USBIP_SITE= $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace) USBIP_SITE_METHOD = local USBIP_PATCH_DEPENDENCIES = linux define USBIP_CONFIGURE_CMDS @echo 'USBIP_SITE="$(USBIP_SITE)"' endef $(eval $(generic-package))And then:
Quoted 7 lines
Quoted 5 lines
$ make usbip-configure [--SNIP--] USBIP_SITE="/home/ymorin/dev/buildroot/O/build/linux-4.8.13/tools/usb/usbip"Quid? ;-)
Notice how it says "Extracting" in the "snip". That's a hint that it doesn't understand it needs to rsync.
Oh dang, stupid me...
Quoted 9 lines
In the top Make, we can find:
########
include $(sort $(wildcard package/*/*.mk))include
boot/common.mk
includelinux/linux.mk
includefs/common.mk
########
so pkg-autotools evaluates the wildcard beforeLINUX_DIRis defined
And contrary to what Arnout said, _SITE *is* evaluated outside of rules, but soon-enough in pkg-generic.mk, before LINUX_DIR had a change to be defined.
Good catch, Tal.
So, back to squarte one. You have two options, then:
- adding a kconfig option, like your latest patch provides,
- not supporting kernels <= 3.16, like you also suggested.
I'm fine with either solution.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
Quoted 58 lines · Yann E. MORIN
Tal, All,
On 2016-12-12 00:11 +0200, Tal Shorer spake thusly:
Quoted 31 lines · Yann E. MORIN
Quoted 30 lines
Tal, Arnout, All,
On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
Quoted 1 line · Arnout Vandecappelle
[--SNIP--]
Quoted 8 lines
Quoted 7 lines
However, doesn't something like this work? Or is it considered too much of a hack?
# Before v3.17 it was in staging.
#USBIP_SITEis only used inside rules, after linux has already been extracted.USBIP_SITE = $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace)It doesn't,
Weird, it does work here:
USBIP_SITE= $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace) USBIP_SITE_METHOD = local USBIP_PATCH_DEPENDENCIES = linux define USBIP_CONFIGURE_CMDS @echo 'USBIP_SITE="$(USBIP_SITE)"' endef $(eval $(generic-package))And then:
Quoted 7 lines
Quoted 5 lines
$ make usbip-configure [--SNIP--] USBIP_SITE="/home/ymorin/dev/buildroot/O/build/linux-4.8.13/tools/usb/usbip"Quid? ;-)
Notice how it says "Extracting" in the "snip". That's a hint that it doesn't understand it needs to rsync.
Oh dang, stupid me...
Quoted 9 lines
In the top Make, we can find:
########
include $(sort $(wildcard package/*/*.mk))include
boot/common.mk
includelinux/linux.mk
includefs/common.mk
########
so pkg-autotools evaluates the wildcard beforeLINUX_DIRis definedAnd contrary to what Arnout said,
_SITE*is* evaluated outside of rules, but soon-enough in pkg-generic.mk, beforeLINUX_DIRhad a change to be defined.
Quoted 8 lines
Good catch, Tal.
So, back to squarte one. You have two options, then:
- adding a kconfig option, like your latest patch provides,
- not supporting kernels <= 3.16, like you also suggested.
Both don't deal with source-check, though. I think we should consider linux-tools again at this point, since it doesn't have any of these problems (it has its own problems, but I'm beginning to think I'd rather deal with those). If nobody objects to this approach I'll conjure something up tomorrow since it's getting pretty late in the GMT+2 part of the world :)
Quoted 12 lines
I'm fine with either solution.
Regards,
Yann E. MORIN.--
.-----------------.--------------------.------------------.--------------------.| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |'------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
Quoted 39 lines · Tal Shorer
Quoted 38 lines · Yann E. MORIN
Tal, All,
On 2016-12-12 00:11 +0200, Tal Shorer spake thusly:
Quoted 35 lines · Yann E. MORIN
Quoted 34 lines
Tal, Arnout, All,
On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
Quoted 1 line · Arnout Vandecappelle
[--SNIP--]
Quoted 8 lines
Quoted 7 lines
However, doesn't something like this work? Or is it considered too much of a hack?
# Before v3.17 it was in staging.
#USBIP_SITEis only used inside rules, after linux has already been extracted.USBIP_SITE = $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace)It doesn't,
Weird, it does work here:
USBIP_SITE= $(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/usbip/userspace) USBIP_SITE_METHOD = local USBIP_PATCH_DEPENDENCIES = linux define USBIP_CONFIGURE_CMDS @echo 'USBIP_SITE="$(USBIP_SITE)"' endef $(eval $(generic-package))And then:
$ make usbip-configure [--SNIP--] USBIP_SITE="/home/ymorin/dev/buildroot/O/build/linux-4.8.13/tools/usb/usbip"
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Quid? ;-)
Notice how it says "Extracting" in the "snip". That's a hint that it doesn't understand it needs to rsync.
Oh dang, stupid me...
No, stupid *me*. I saw this:
ifeq ($$($(2)_OVERRIDE_SRCDIR),)
and I figured that it wasn't empty because we assigned something to it. But of course it _is_ empty when you evaluate it.
Quoted 19 lines
Quoted 19 lines
Quoted 10 lines
In the top Make, we can find:
########
include $(sort $(wildcard package/*/*.mk))include
boot/common.mk
includelinux/linux.mk
includefs/common.mk
########
so pkg-autotools evaluates the wildcard beforeLINUX_DIRis definedAnd contrary to what Arnout said,
_SITE*is* evaluated outside of rules, but soon-enough in pkg-generic.mk, beforeLINUX_DIRhad a change to be defined.Good catch, Tal.
So, back to squarte one. You have two options, then:
Quoted 8 lines
Quoted 4 lines
- adding a kconfig option, like your latest patch provides,
- not supporting kernels <= 3.16, like you also suggested.
Both don't deal with source-check, though. I think we should consider linux-tools again at this point, since it doesn't have any of these problems (it has its own problems, but I'm beginning to think I'd rather deal with those). If nobody objects to this approach I'll
+1 to that approach.
Regards,
Arnout
Quoted 14 lines
conjure something up tomorrow since it's getting pretty late in the GMT+2 part of the world :)
Quoted 12 lines
I'm fine with either solution.
Regards,
Yann E. MORIN.--
.-----------------.--------------------.------------------.--------------------.| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |'------------------------------^-------^------------------^--------------------'
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 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: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/1 usbip: add a new package
Tal, Arnout, All,
On 2016-12-12 10:40 +0100, Arnout Vandecappelle spake thusly:
Quoted 7 lines · Tal Shorer
Quoted 4 lines
Both don't deal with source-check, though. I think we should consider linux-tools again at this point, since it doesn't have any of these problems (it has its own problems, but I'm beginning to think I'd rather deal with those). If nobody objects to this approach I'll
+1 to that approach.
Still, I don't like the custom autotools commands we'll have to have. What about (mock-up, just for the sake of the example):
###############
# usbip
###############
# No USBIP_SITE, no USB_VERSION, we vampirise the code from the
# linux kernel
USBIP_PATCH_DEPENDENCIES = linux
USBIP_SRC_DIR = $)(wildcard \
$(LINUX_DIR)/tools/usb/usbip \
$(LINUX_DIR)/drivers/staging/blabla)
define USBIP_EXTRACT_CMDS
if [ -z "$(USB_SRC_DIR)" ]; then \
echo "No usbip source in your kernel tree" 2>&1; \
exit 1; \
fi
rsync $(USB_SRC_DIR) $(@D)
endef
$(eval $(autotools-package))This way, we still have a proper autotools package, we don;t fail on source-check, and we re-use the kernel sources.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
Quoted 36 lines · Yann E. MORIN
Tal, Arnout, All,
On 2016-12-12 10:40 +0100, Arnout Vandecappelle spake thusly:
Quoted 7 lines · Tal Shorer
Quoted 4 lines
Both don't deal with source-check, though. I think we should consider linux-tools again at this point, since it doesn't have any of these problems (it has its own problems, but I'm beginning to think I'd rather deal with those). If nobody objects to this approach I'll
+1 to that approach.
Still, I don't like the custom autotools commands we'll have to have. What about (mock-up, just for the sake of the example):
############### # usbip ############### # No USBIP_SITE, no USB_VERSION, we vampirise the code from the # linux kernel USBIP_PATCH_DEPENDENCIES = linux USBIP_SRC_DIR = $)(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/blabla) define USBIP_EXTRACT_CMDS if [ -z "$(USB_SRC_DIR)" ]; then \ echo "No usbip source in your kernel tree" 2>&1; \ exit 1; \ fi rsync $(USB_SRC_DIR) $(@D) endef $(eval $(autotools-package))
Quoted 2 lines
This way, we still have a proper autotools package, we don;t fail on source-check, and we re-use the kernel sources.
This works after some modifications. We end up duplicating the rsync command from pkg-generic, but the argument that this is better than duplicating the configure command is easily made.
I'll send a new patch
Quoted 11 lines
Regards,
Yann E. MORIN.--
.-----------------.--------------------.------------------.--------------------.| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |'------------------------------^-------^------------------^--------------------'
PATCH v3 1/1 usbip: new package
add the usbip package from the kernel source, allowing users to share usb devices over a network connection
--- package/linux-tools/Config.in | 2 ++ package/usbip/Config.in | 5 +++++ package/usbip/usbip.mk | 21 +++++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 package/usbip/Config.in create mode 100644 package/usbip/usbip.mk
package/linux-tools/Config.in +2 -0
@@ -83,4 +83,6 @@ comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" depends on BR2_USE_MMU depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS +source package/usbip/Config.in + endmenu
package/usbip/Config.in new file +5 -0
@@ -0,0 +1,5 @@ +config BR2_PACKAGE_USBIP + bool"usbip" + help + usbip is a set of tools that allows machines to share their + usb devices over the network, to be driven by a remote client.
package/usbip/usbip.mk new file +21 -0
@@ -0,0 +1,21 @@ +############### +# usbip +############### + +# No USBIP_SITE, no USB_VERSION, we vampirise the code from the +# linux kernel +USBIP_PATCH_DEPENDENCIES = linux + +USBIP_SRC_DIR = $(wildcard \ + $(LINUX_DIR)/tools/usb/usbip \ + $(LINUX_DIR)/drivers/staging/usbip/userspace) + +define USBIP_EXTRACT_CMDS + if [[ -z "$(USBIP_SRC_DIR)" ]]; then \ + echo "No usbip source in your kernel tree" 2>&1; \ + exit 1; \ + fi + rsync -a $(USBIP_SRC_DIR)/ $(@D) +endef + +$(eval $(autotools-package))
Signature · 1 line
2.7.4
PATCH v3 1/1 usbip: new package
Tal, All,
On 2016-12-13 00:21 +0200, Tal Shorer spake thusly:
package/usbip/Config.in
Quoted 29 lines
add the usbip package from the kernel source, allowing users to share usb devices over a network connection
Signed-off-by Tal Shorer <[email protected]>--- package/linux-tools/Config.in | 2 ++ package/usbip/Config.in | 5 +++++ package/usbip/usbip.mk | 21 +++++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 package/usbip/Config.in create mode 100644 package/usbip/usbip.mkpackage/linux-tools/Config.in +2
-0@@ -83,4 +83,6 @@ comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" depends on BR2_USE_MMU depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS +source package/usbip/Config.in + endmenupackage/usbip/Config.in new file +0
-0@@ -0,0 +1,5 @@
Maybe add a small comment above the config option to state that this is included from the linux-tools infra, rather than from the usual package/Config.in, to avoid confusing developpers later on...
package/usbip/Config.in line 2
Quoted 2 lines
package/usbip/Config.in new file +2
-0+config BR2_PACKAGE_USBIP + bool"usbip"
So you dropped the dependency on !static. Does that mean you were able to test it with a static build?
It also required libudev, so you have to depend on BR2_PACKAGE_HAS_UDEV.
It needs usb.ids from usbutils, so you also need to select it:
# This file is included from linux-tools, not from the usual # package/Config.in location, because it is a linux tools, but uses # the autotool infra
config BR2_PACKAGE_USBIP
bool "usbip"
depends on BR2_PACKAGE_HAS_UDEV
depends on BR2_TOOLCHAIN_HAS_THREADS # usbutils <- libusb select BR2_PACKAGE_USBUTILS
comment "usbip needs udev /dev management and toolchain w/ threads"
depends on !BR2_PACKAGE_HAS_UDEV \
|| !BR2_TOOLCHAIN_HAS_THREADS
It also has optional support for libwrap, which we do not have in Buildroot. See below...
package/usbip/usbip.mk line 3
Quoted 12 lines
package/usbip/Config.in new file +3
-0+ help + usbip is a set of tools that allows machines to share their + usb devices over the network, to be driven by a remote client.package/usbip/usbip.mk new file +3
-0@@ -0,0 +1,21 @@ +############### +# usbip +###############
Incorrect header. ;-)
package/usbip/usbip.mk line 7
Quoted 3 lines
package/usbip/usbip.mk new file +3
-0+# No USBIP_SITE, no USB_VERSION, we vampirise the code from the +# linux kernel +USBIP_PATCH_DEPENDENCIES = linux
Since it uses udev, you also have to depend on it:
USBIP_PATCH_DEPENDENCIES = linux
USBIP_DEPENDENCIES = udev
(Just to show that you have both types of dependencies.)
Also, it can use libwrap, which we do not have in Buildroot, so you may want to forcibly disable it:
USBIP_CONF_OPTS = --without-tcp-wrappers
package/usbip/usbip.mk line 14
Quoted 6 lines
package/usbip/usbip.mk new file +6
-0+USBIP_SRC_DIR = $(wildcard \ + $(LINUX_DIR)/tools/usb/usbip \ + $(LINUX_DIR)/drivers/staging/usbip/userspace) + +define USBIP_EXTRACT_CMDS + if [[ -z "$(USBIP_SRC_DIR)" ]]; then \
No need to use [[...]], you can simply use [...] here.
package/usbip/usbip.mk line 18
Quoted 4 lines
package/usbip/usbip.mk new file +4
-0+ echo "No usbip source in your kernel tree" 2>&1; \ + exit 1; \ + fi + rsync -a $(USBIP_SRC_DIR)/ $(@D)
I think you will want to really mimick the rsync rule we use elsewhere:
rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $(USBIP_SRC_DIR)/ $(@D)
Cordialement,
Yann E. MORIN.
Quoted 10 lines
package/usbip/usbip.mk new file +3
-0+endef + +$(eval $(autotools-package))Signature · 6 lines
2.7.4 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v4 1/1 usbip: new package
add the usbip package from the kernel source, allowing users to share usb devices over a network connection
--- package/linux-tools/Config.in | 2 ++ package/usbip/Config.in | 14 ++++++++++++++ package/usbip/usbip.mk | 26 ++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 package/usbip/Config.in create mode 100644 package/usbip/usbip.mk
package/linux-tools/Config.in +2 -0
@@ -83,4 +83,6 @@ comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" depends on BR2_USE_MMU depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS +source package/usbip/Config.in + endmenu
package/usbip/Config.in new file +14 -0
@@ -0,0 +1,14 @@ +# sourced from package/linux-tools/Config.in rather than from package/Config.in + +config BR2_PACKAGE_USBIP + bool "usbip" + depends on BR2_PACKAGE_HAS_UDEV + depends on BR2_TOOLCHAIN_HAS_THREADS # usbutils <- libusb + depends on !BR2_STATIC_LIBS + select BR2_PACKAGE_USBUTILS + help + usbip is a set of tools that allows machines to share their + usb devices over the network, to be driven by a remote client. + +comment "usbip needs udev /dev management and toolchain w/ threads" + depends on !BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HAS_THREADS
package/usbip/usbip.mk new file +26 -0
@@ -0,0 +1,26 @@ +################################################################################ +# +# usbip +# +################################################################################ + +# No USBIP_SITE, no USB_VERSION, we vampirise the code from the +# linux kernel +USBIP_PATCH_DEPENDENCIES = linux +USBIP_DEPENDENCIES = udev + +USBIP_CONF_OPTS = --without-tcp-wrappers + +USBIP_SRC_DIR = $(wildcard \ + $(LINUX_DIR)/tools/usb/usbip \ + $(LINUX_DIR)/drivers/staging/usbip/userspace) + +define USBIP_EXTRACT_CMDS + if [ -z "$(USBIP_SRC_DIR)" ]; then \ + echo "No usbip source in your kernel tree" 2>&1; \ + exit 1; \ + fi + rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $(USBIP_SRC_DIR)/ $(@D) +endef + +$(eval $(autotools-package))
Signature · 1 line
2.7.4
PATCH v4 1/1 usbip: new package
Quoted 76 lines · Tal Shorer
add the usbip package from the kernel source, allowing users to share usb devices over a network connection
Signed-off-by Tal Shorer <[email protected]>--- package/linux-tools/Config.in | 2 ++ package/usbip/Config.in | 14 ++++++++++++++ package/usbip/usbip.mk | 26 ++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 package/usbip/Config.in create mode 100644 package/usbip/usbip.mkpackage/linux-tools/Config.in +2
-0@@ -83,4 +83,6 @@ comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" depends on BR2_USE_MMU depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS +source package/usbip/Config.in + endmenupackage/usbip/Config.in new file +14
-0@@ -0,0 +1,14 @@ +# sourced from package/linux-tools/Config.in rather than from package/Config.in + +config BR2_PACKAGE_USBIP + bool "usbip" + depends on BR2_PACKAGE_HAS_UDEV + depends on BR2_TOOLCHAIN_HAS_THREADS # usbutils <- libusb + depends on !BR2_STATIC_LIBS + select BR2_PACKAGE_USBUTILS + help + usbip is a set of tools that allows machines to share their + usb devices over the network, to be driven by a remote client. + +comment "usbip needs udev /dev management and toolchain w/ threads" + depends on !BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HAS_THREADSpackage/usbip/usbip.mk new file +26
-1@@ -0,0 +1,26 @@ +################################################################################ +# +# usbip +# +################################################################################ + +# No USBIP_SITE, no USB_VERSION, we vampirise the code from the +# linux kernel +USBIP_PATCH_DEPENDENCIES = linux +USBIP_DEPENDENCIES = udev + +USBIP_CONF_OPTS = --without-tcp-wrappers + +USBIP_SRC_DIR = $(wildcard \ + $(LINUX_DIR)/tools/usb/usbip \ + $(LINUX_DIR)/drivers/staging/usbip/userspace) + +define USBIP_EXTRACT_CMDS + if [ -z "$(USBIP_SRC_DIR)" ]; then \ + echo "No usbip source in your kernel tree" 2>&1; \ + exit 1; \ + fi + rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $(USBIP_SRC_DIR)/ $(@D) +endef + +$(eval $(autotools-package)) --
Quoted 2 lines
2.7.4
ping?
PATCH v4 1/1 usbip: new package
In reply to Tal ShorerTal, All,
On 2016-12-14 19:28 +0200, Tal Shorer spake thusly:
Quoted 4 lines
add the usbip package from the kernel source, allowing users to share usb devices over a network connection
Signed-off-by Tal Shorer <[email protected]>
[--SNIP--]
Quoted 6 lines
package/usbip/Config.in new file +0
-0
package/usbip/Config.in line 8
Quoted 8 lines
package/usbip/Config.in new file +8
-0@@ -0,0 +1,14 @@ +# sourced from package/linux-tools/Config.in rather than from package/Config.in + +config BR2_PACKAGE_USBIP + bool "usbip" + depends on BR2_PACKAGE_HAS_UDEV + depends on BR2_TOOLCHAIN_HAS_THREADS # usbutils <- libusb + depends on !BR2_STATIC_LIBS + select BR2_PACKAGE_USBUTILS
It is weird to see usbutils being selected here, but not appear in the dependnecy list in the .mk. But that's because it is only a runtime dependency. You need to state so, with a comment like:
select BR2_PACKAGE_USBUTILS # runtime
package/usbip/Config.in line 14
Quoted 6 lines
package/usbip/Config.in new file +6
-0+ help + usbip is a set of tools that allows machines to share their + usb devices over the network, to be driven by a remote client. + +comment "usbip needs udev /dev management and toolchain w/ threads" + depends on !BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HAS_THREADS
This comment should also mention the !static case. However, in a previous iteration, you did remove this restriction. Can you confirm that usbip does not build in a static-only build?
If not, then the comment should also account for that:
comment "usbip needs udev /dev management and a toolchain w/ threads, shared library"
depends on !BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HAS_THREADS \
|| BR2_STATIC_LIBSSee the manual:
https://buildroot.org/downloads/manual/manual.html#dependencies-target-toolchain-options
I'll try to give it a spin here in a moment...
Regards,
Yann E. MORIN.
Quoted 39 lines
package/usbip/usbip.mk new file +26
-0@@ -0,0 +1,26 @@ +################################################################################ +# +# usbip +# +################################################################################ + +# No USBIP_SITE, no USB_VERSION, we vampirise the code from the +# linux kernel +USBIP_PATCH_DEPENDENCIES = linux +USBIP_DEPENDENCIES = udev + +USBIP_CONF_OPTS = --without-tcp-wrappers + +USBIP_SRC_DIR = $(wildcard \ + $(LINUX_DIR)/tools/usb/usbip \ + $(LINUX_DIR)/drivers/staging/usbip/userspace) + +define USBIP_EXTRACT_CMDS + if [ -z "$(USBIP_SRC_DIR)" ]; then \ + echo "No usbip source in your kernel tree" 2>&1; \ + exit 1; \ + fi + rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $(USBIP_SRC_DIR)/ $(@D) +endef + +$(eval $(autotools-package))Signature · 6 lines
2.7.4 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v4 1/1 usbip: new package
In reply to Tal ShorerTal, All,
On 2016-12-14 19:28 +0200, Tal Shorer spake thusly:
Quoted 2 lines
add the usbip package from the kernel source, allowing users to share usb devices over a network connection
So, I finally had time to test this. I'm sad to report that it does not work... :-(
Here's my defconfig:
BR2_x86_i686=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y
BR2_LINUX_KERNEL=y
BR2_LINUX_KERNEL_USE_ARCH_DEFAULT_CONFIG=y
BR2_PACKAGE_USBIP=yThe first error is that the patch dependencies are only acted upon at the time the package is actually patched, not extracted. So the linux kernel sources are not extracted (and patched) at the time we try to rsync the usbip source tree.
This is easily fixable by making linux a "classic" dependency, but that is not really nice... :-/
Then, the source code for usbip only has configure.ac, not configure, so it needs USBIP_AUTORECONF = YES.
But then the build fails because it is missing linux/usbip.h. The toolchain I'm using is too old to have this header.
usbip.h has been installed starting with kernel 3.17, so you also have to depend on the correct kernel headers version:
depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_17
I've stopped here, I started another build with a more recent toolchain with more recent kernel headers. I'll further report when the build is finished...
Regards,
Yann E. MORIN.
Quoted 79 lines
Signed-off-by Tal Shorer <[email protected]>--- package/linux-tools/Config.in | 2 ++ package/usbip/Config.in | 14 ++++++++++++++ package/usbip/usbip.mk | 26 ++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 package/usbip/Config.in create mode 100644 package/usbip/usbip.mkpackage/linux-tools/Config.in +2
-0@@ -83,4 +83,6 @@ comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" depends on BR2_USE_MMU depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS +source package/usbip/Config.in + endmenupackage/usbip/Config.in new file +14
-0@@ -0,0 +1,14 @@ +# sourced from package/linux-tools/Config.in rather than from package/Config.in + +config BR2_PACKAGE_USBIP + bool "usbip" + depends on BR2_PACKAGE_HAS_UDEV + depends on BR2_TOOLCHAIN_HAS_THREADS # usbutils <- libusb + depends on !BR2_STATIC_LIBS + select BR2_PACKAGE_USBUTILS + help + usbip is a set of tools that allows machines to share their + usb devices over the network, to be driven by a remote client. + +comment "usbip needs udev /dev management and toolchain w/ threads" + depends on !BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HAS_THREADSpackage/usbip/usbip.mk new file +26
-0@@ -0,0 +1,26 @@ +################################################################################ +# +# usbip +# +################################################################################ + +# No USBIP_SITE, no USB_VERSION, we vampirise the code from the +# linux kernel +USBIP_PATCH_DEPENDENCIES = linux +USBIP_DEPENDENCIES = udev + +USBIP_CONF_OPTS = --without-tcp-wrappers + +USBIP_SRC_DIR = $(wildcard \ + $(LINUX_DIR)/tools/usb/usbip \ + $(LINUX_DIR)/drivers/staging/usbip/userspace) + +define USBIP_EXTRACT_CMDS + if [ -z "$(USBIP_SRC_DIR)" ]; then \ + echo "No usbip source in your kernel tree" 2>&1; \ + exit 1; \ + fi + rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $(USBIP_SRC_DIR)/ $(@D) +endef + +$(eval $(autotools-package))Signature · 6 lines
2.7.4 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v4 1/1 usbip: new package
Tal, All,
On 2016-12-23 19:24 +0100, Yann E. MORIN spake thusly:
Quoted 6 lines
On 2016-12-14 19:28 +0200, Tal Shorer spake thusly:
Quoted 2 lines
add the usbip package from the kernel source, allowing users to share usb devices over a network connection
So, I finally had time to test this. I'm sad to report that it does not work... :-(
[--SNIP--]
Quoted 3 lines
I've stopped here, I started another build with a more recent toolchain with more recent kernel headers. I'll further report when the build is finished...
OK, so I was able to eventually build it with those changes:
package/usbip/Config.in +4 -2
@@ -4,11 +4,13 @@ config BR2_PACKAGE_USBIP bool "usbip" depends on BR2_PACKAGE_HAS_UDEV depends on BR2_TOOLCHAIN_HAS_THREADS # usbutils <- libusb + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_ 17 depends on !BR2_STATIC_LIBS select BR2_PACKAGE_USBUTILS help usbip is a set of tools that allows machines to share their usb devices over the network, to be driven by a remote client. -comment "usbip needs udev /dev management and toolchain w/ threads" - depends on !BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HAS_THREADS +comment "usbip needs udev /dev management and a toolchain w/ threads, dynamic library, headers >= 3.17" + depends on !BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HAS_THREADS \ + || BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_17
package/usbip/usbip.mk +3 -3
@@ -4,11 +4,11 @@ # ################################################################################ -# No USBIP_SITE, no USB_VERSION, we vampirise the code from the +# No USBIP_SITE, no USBIP_VERSION, we vampirise the code from the # linux kernel -USBIP_PATCH_DEPENDENCIES = linux -USBIP_DEPENDENCIES = udev +USBIP_DEPENDENCIES = udev linux +USBIP_AUTORECONF = YES USBIP_CONF_OPTS = --without-tcp-wrappers USBIP_SRC_DIR = $(wildcard \
(not sure if the patch did not get mangled when I copy-pasted it.)
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 usbip: add a new package
In reply to Tal ShorerTal, All,
On 2016-12-13 00:17 +0200, Tal Shorer spake thusly:
Quoted 35 lines · Yann E. MORIN
Quoted 34 lines
Tal, Arnout, All,
On 2016-12-12 10:40 +0100, Arnout Vandecappelle spake thusly:
Quoted 7 lines · Tal Shorer
Quoted 4 lines
Both don't deal with source-check, though. I think we should consider linux-tools again at this point, since it doesn't have any of these problems (it has its own problems, but I'm beginning to think I'd rather deal with those). If nobody objects to this approach I'll
+1 to that approach.
Still, I don't like the custom autotools commands we'll have to have. What about (mock-up, just for the sake of the example):
############### # usbip ############### # No USBIP_SITE, no USB_VERSION, we vampirise the code from the # linux kernel USBIP_PATCH_DEPENDENCIES = linux USBIP_SRC_DIR = $)(wildcard \ $(LINUX_DIR)/tools/usb/usbip \ $(LINUX_DIR)/drivers/staging/blabla) define USBIP_EXTRACT_CMDS if [ -z "$(USB_SRC_DIR)" ]; then \ echo "No usbip source in your kernel tree" 2>&1; \ exit 1; \ fi rsync $(USB_SRC_DIR) $(@D) endef
Quoted 5 lines
Quoted 4 lines
$(eval $(autotools-package))This way, we still have a proper autotools package, we don;t fail on source-check, and we re-use the kernel sources.
This works after some modifications.
Yes, it was definitely buggy, it was just a mock-up...
Quoted 3 lines
We end up duplicating the rsync
command from pkg-generic, but the argument that this is better than duplicating the configure command is easily made.
Yes, the rsync command is duplicated, but that one is easier to maintain than the autotools commands (configure, build, target-install, and staging-isntall).
Quoted 1 line
I'll send a new patch
Thanks, I'll review it now.
Regards,
Yann E. MORIN.
Quoted 11 lines
Quoted 11 lines
Regards,
Yann E. MORIN.--
.-----------------.--------------------.------------------.--------------------.| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |'------------------------------^-------^------------------^--------------------'
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v2 1/2 package: linux-tools: allow tools to define configure hooks
In reply to Tal Shorerdefine LINUX_TOOLS_POST_CONFIGURE_HOOKS. Similiar to other hooks in this package, this hook loops over the configured tools and runs their $(TOOL)_CONFIGURE_CMDS hook
--- package/linux-tools/linux-tools.mk | 4 ++++ 1 file changed, 4 insertions(+)
package/linux-tools/linux-tools.mk +4 -0
@@ -31,6 +31,10 @@ LINUX_TOOLS_DEPENDENCIES += $(foreach tool,$(LINUX_TOOLS),\ $(if $(BR2_PACKAGE_LINUX_TOOLS_$(call UPPERCASE,$(tool))),\ $($(call UPPERCASE,$(tool))_DEPENDENCIES))) +LINUX_TOOLS_POST_CONFIGURE_HOOKS += $(foreach tool,$(LINUX_TOOLS),\ + $(if $(BR2_PACKAGE_LINUX_TOOLS_$(call UPPERCASE,$(tool))),\ + $(call UPPERCASE,$(tool))_CONFIGURE_CMDS)) + LINUX_TOOLS_POST_BUILD_HOOKS += $(foreach tool,$(LINUX_TOOLS),\ $(if $(BR2_PACKAGE_LINUX_TOOLS_$(call UPPERCASE,$(tool))),\ $(call UPPERCASE,$(tool))_BUILD_CMDS))
Signature · 1 line
2.7.4
PATCH v2 2/2 package: linux-tools: add usbip
add the usbip package from the kernel source, allowing users to share usb devices over a network connection
--- package/linux-tools/Config.in | 7 ++++++ package/linux-tools/linux-tool-usbip.mk | 42 +++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 package/linux-tools/linux-tool-usbip.mk
package/linux-tools/Config.in +7 -0
@@ -83,4 +83,11 @@ comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" depends on BR2_USE_MMU depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS +config BR2_PACKAGE_LINUX_TOOLS_USBIP + bool"usbip" + select BR2_PACKAGE_LINUX_TOOLS + help + usbip is a set of tools that allows machines to share their + usb devices over the network, to be driven by a remote client. + endmenu
package/linux-tools/linux-tool-usbip.mk new file +42 -0
42 lines not on this page · raw message
Signature · 1 line
2.7.4
PATCH v2 2/2 package: linux-tools: add usbip
Tal, All,
On 2016-12-12 19:09 +0200, Tal Shorer spake thusly:
Quoted 2 lines
add the usbip package from the kernel source, allowing users to share usb devices over a network connection
Sorry, but I still think a spearate package is better, because we re-use the autotools infra rather than rewrite it...
See my reply in the previos thread.
Regards,
Yann E. MORIN.
Quoted 78 lines
Signed-off-by Tal Shorer <[email protected]>--- package/linux-tools/Config.in | 7 ++++++ package/linux-tools/linux-tool-usbip.mk | 42 +++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 package/linux-tools/linux-tool-usbip.mkpackage/linux-tools/Config.in +7
-0@@ -83,4 +83,11 @@ comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" depends on BR2_USE_MMU depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS +config BR2_PACKAGE_LINUX_TOOLS_USBIP + bool"usbip" + select BR2_PACKAGE_LINUX_TOOLS + help + usbip is a set of tools that allows machines to share their + usb devices over the network, to be driven by a remote client. + endmenupackage/linux-tools/linux-tool-usbip.mk new file +42
-0@@ -0,0 +1,42 @@ +################################################################################ +# +# usbip +# +################################################################################ + +LINUX_TOOLS += usbip + +USBIP_MAKE_OPTS = $(LINUX_MAKE_FLAGS) + +USBIP_DIR = $(wildcard \ + $(LINUX_DIR)/tools/usb/usbip \ + $(LINUX_DIR)/drivers/staging/usbip/userspace) + +define USBIP_CONFIGURE_CMDS + $(Q)if ! [[ -d $(USBIP_DIR) ]] >/dev/null 2>&1 ; then \ + echo "Your kernel version is too old and does not have usbip." ; \ + echo "At least kernel 3.0 must be used." ; \ + exit 1 ; \ + fi + cd $(USBIP_DIR) && $(AUTORECONF) && \ + $(TARGET_CONFIGURE_OPTS) \ + $(TARGET_CONFIGURE_ARGS) \ + ./configure \ + --target=$(GNU_TARGET_NAME) \ + --host=$(GNU_TARGET_NAME) \ + --build=$(GNU_HOST_NAME) \ + --prefix=$(TARGET_DIR)/usr \ + $(SHARED_STATIC_LIBS_OPTS) + +endef + +define USBIP_BUILD_CMDS + $(TARGET_MAKE_ENV) $(MAKE) -C $(USBIP_DIR) $(USBIP_MAKE_OPTS) +endef + +define USBIP_INSTALL_TARGET_CMDS + $(TARGET_MAKE_ENV) $(MAKE) -C $(USBIP_DIR) \ + $(USBIP_MAKE_OPTS) \ + INSTALL_ROOT=$(TARGET_DIR) \ + install +endefSignature · 6 lines
2.7.4 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'