All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] usbip: add a new package
@ 2016-12-09  8:37 Tal Shorer
  2016-12-10 13:31 ` Thomas Petazzoni
  2016-12-11 13:56 ` Yann E. MORIN
  0 siblings, 2 replies; 34+ messages in thread
From: Tal Shorer @ 2016-12-09  8:37 UTC (permalink / raw)
  To: buildroot

Add usbip tools (usbip, usbipd) from the running linux source

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 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

diff --git a/package/Config.in b/package/Config.in
index 9ed296f..8d8f410 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -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"
diff --git a/package/usbip/Config.in b/package/usbip/Config.in
new file mode 100644
index 0000000..580c917
--- /dev/null
+++ b/package/usbip/Config.in
@@ -0,0 +1,9 @@
+config BR2_PACKAGE_USBIP
+	bool "usbip"
+	depends on !BR2_STATIC_LIBS
+	help
+	  epic stuff
+
+if BR2_PACKAGE_USBIP
+
+endif
diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
new file mode 100644
index 0000000..e6bd7f7
--- /dev/null
+++ b/package/usbip/usbip.mk
@@ -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))
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-09  8:37 [Buildroot] [PATCH] usbip: add a new package Tal Shorer
@ 2016-12-10 13:31 ` Thomas Petazzoni
  2016-12-10 18:39   ` Tal Shorer
  2016-12-11 13:56 ` Yann E. MORIN
  1 sibling, 1 reply; 34+ messages in thread
From: Thomas Petazzoni @ 2016-12-10 13:31 UTC (permalink / raw)
  To: buildroot

Hello,

On Fri,  9 Dec 2016 10:37:51 +0200, Tal Shorer wrote:
> Add usbip tools (usbip, usbipd) from the running linux source
> 
> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-10 13:31 ` Thomas Petazzoni
@ 2016-12-10 18:39   ` Tal Shorer
  2016-12-11 10:06     ` Yann E. MORIN
  0 siblings, 1 reply; 34+ messages in thread
From: Tal Shorer @ 2016-12-10 18:39 UTC (permalink / raw)
  To: buildroot

On Sat, Dec 10, 2016 at 3:31 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Fri,  9 Dec 2016 10:37:51 +0200, Tal Shorer wrote:
>> Add usbip tools (usbip, usbipd) from the running linux source
>>
>> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
>
> 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?
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 :)

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-10 18:39   ` Tal Shorer
@ 2016-12-11 10:06     ` Yann E. MORIN
  0 siblings, 0 replies; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-11 10:06 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-10 20:39 +0200, Tal Shorer spake thusly:
> On Sat, Dec 10, 2016 at 3:31 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
> > Hello,
> >
> > On Fri,  9 Dec 2016 10:37:51 +0200, Tal Shorer wrote:
> >> Add usbip tools (usbip, usbipd) from the running linux source
> >>
> >> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
> >
> > 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?
> 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.

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.

> 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

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-09  8:37 [Buildroot] [PATCH] usbip: add a new package Tal Shorer
  2016-12-10 13:31 ` Thomas Petazzoni
@ 2016-12-11 13:56 ` Yann E. MORIN
  2016-12-11 17:39   ` Tal Shorer
  1 sibling, 1 reply; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-11 13:56 UTC (permalink / raw)
  To: buildroot

Tal, Thomas, All,

On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
> 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....

> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
> ---
>  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
> 
> diff --git a/package/Config.in b/package/Config.in
> index 9ed296f..8d8f410 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -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?

>  	source "package/usbutils/Config.in"
>  	source "package/w_scan/Config.in"
>  	source "package/wf111/Config.in"
> diff --git a/package/usbip/Config.in b/package/usbip/Config.in
> new file mode 100644
> index 0000000..580c917
> --- /dev/null
> +++ b/package/usbip/Config.in
> @@ -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.

> +	help
> +	  epic stuff

You already spot this help text. ;-)

> +if BR2_PACKAGE_USBIP
> +
> +endif

No option, so no need for this if-block.

However, there is a need for it, see below.

> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
> new file mode 100644
> index 0000000..e6bd7f7
> --- /dev/null
> +++ b/package/usbip/usbip.mk
> @@ -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"

    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

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

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

> +$(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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 13:56 ` Yann E. MORIN
@ 2016-12-11 17:39   ` Tal Shorer
  2016-12-11 17:45     ` Yann E. MORIN
  2016-12-11 19:59     ` Arnout Vandecappelle
  0 siblings, 2 replies; 34+ messages in thread
From: Tal Shorer @ 2016-12-11 17:39 UTC (permalink / raw)
  To: buildroot

On Sun, Dec 11, 2016 at 3:56 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Tal, Thomas, All,
>
> On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
>> 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....
>
>> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
>> ---
>>  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
>>
>> diff --git a/package/Config.in b/package/Config.in
>> index 9ed296f..8d8f410 100644
>> --- a/package/Config.in
>> +++ b/package/Config.in
>> @@ -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?
>
>>       source "package/usbutils/Config.in"
>>       source "package/w_scan/Config.in"
>>       source "package/wf111/Config.in"
>> diff --git a/package/usbip/Config.in b/package/usbip/Config.in
>> new file mode 100644
>> index 0000000..580c917
>> --- /dev/null
>> +++ b/package/usbip/Config.in
>> @@ -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.
>
>> +     help
>> +       epic stuff
>
> You already spot this help text. ;-)
>
>> +if BR2_PACKAGE_USBIP
>> +
>> +endif
>
> No option, so no need for this if-block.
>
> However, there is a need for it, see below.
>
>> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
>> new file mode 100644
>> index 0000000..e6bd7f7
>> --- /dev/null
>> +++ b/package/usbip/usbip.mk
>> @@ -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"
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?
>
>     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 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
>>> 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 exist
package/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?
>> +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.
>
>> +
>> +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.
>
>> +$(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.  |
> '------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 17:39   ` Tal Shorer
@ 2016-12-11 17:45     ` Yann E. MORIN
  2016-12-11 18:12       ` [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR) Tal Shorer
  2016-12-11 18:31       ` [Buildroot] [PATCH] usbip: add a new package Tal Shorer
  2016-12-11 19:59     ` Arnout Vandecappelle
  1 sibling, 2 replies; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-11 17:45 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-11 19:39 +0200, Tal Shorer spake thusly:
> On Sun, Dec 11, 2016 at 3:56 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
> >> Add usbip tools (usbip, usbipd) from the running linux source
[--SNIP--]
> >> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
> >> new file mode 100644
> >> index 0000000..e6bd7f7
> >> --- /dev/null
> >> +++ b/package/usbip/usbip.mk
> >> @@ -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"
> 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.

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

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.

> The error
> this outputs to the user is as follows:
> 
> tal at tal-N552VW-FY117T:~/Dev/lfs/buildroot|0 $ make usbip
> >>> 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 exist
> package/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?
> >> +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.
> >
> >> +
> >> +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.
> >
> >> +$(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.  |
> > '------------------------------^-------^------------------^--------------------'

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR)
  2016-12-11 17:45     ` Yann E. MORIN
@ 2016-12-11 18:12       ` Tal Shorer
  2016-12-11 19:19         ` Yann E. MORIN
                           ` (2 more replies)
  2016-12-11 18:31       ` [Buildroot] [PATCH] usbip: add a new package Tal Shorer
  1 sibling, 3 replies; 34+ messages in thread
From: Tal Shorer @ 2016-12-11 18:12 UTC (permalink / raw)
  To: buildroot

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 <tal.shorer@gmail.com>
---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 827de62..3ca71b0 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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 $@
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 17:45     ` Yann E. MORIN
  2016-12-11 18:12       ` [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR) Tal Shorer
@ 2016-12-11 18:31       ` Tal Shorer
  2016-12-11 19:24         ` Yann E. MORIN
  1 sibling, 1 reply; 34+ messages in thread
From: Tal Shorer @ 2016-12-11 18:31 UTC (permalink / raw)
  To: buildroot

On Sun, Dec 11, 2016 at 7:45 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
[--SNIP--]
>> 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_HOOKS doesn't help either because the rsync rule checks
>> for the source directory before calling the PRE_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.
>
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?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR)
  2016-12-11 18:12       ` [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR) Tal Shorer
@ 2016-12-11 19:19         ` Yann E. MORIN
  2016-12-11 19:44         ` [Buildroot] [PATCH] package/usbip: new package Tal Shorer
  2016-12-17 15:00         ` [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR) Thomas Petazzoni
  2 siblings, 0 replies; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-11 19:19 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-11 20:12 +0200, Tal Shorer spake thusly:
> 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 <tal.shorer@gmail.com>

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.

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 827de62..3ca71b0 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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 $@
> -- 
> 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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 18:31       ` [Buildroot] [PATCH] usbip: add a new package Tal Shorer
@ 2016-12-11 19:24         ` Yann E. MORIN
  0 siblings, 0 replies; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-11 19:24 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-11 20:31 +0200, Tal Shorer spake thusly:
> On Sun, Dec 11, 2016 at 7:45 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> [--SNIP--]
> >> 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_HOOKS doesn't help either because the rsync rule checks
> >> for the source directory before calling the PRE_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.
> >
> done.

Thanks! :-)

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

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.

    Signed-off-by: You

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

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] package/usbip: new package
  2016-12-11 18:12       ` [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR) Tal Shorer
  2016-12-11 19:19         ` Yann E. MORIN
@ 2016-12-11 19:44         ` Tal Shorer
  2016-12-17 15:00         ` [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR) Thomas Petazzoni
  2 siblings, 0 replies; 34+ messages in thread
From: Tal Shorer @ 2016-12-11 19:44 UTC (permalink / raw)
  To: buildroot

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

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>

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

diff --git a/package/linux-tools/Config.in b/package/linux-tools/Config.in
index db9ed9f..7fceca7 100644
--- a/package/linux-tools/Config.in
+++ b/package/linux-tools/Config.in
@@ -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
diff --git a/package/usbip/Config.in b/package/usbip/Config.in
new file mode 100644
index 0000000..7cd6c29
--- /dev/null
+++ b/package/usbip/Config.in
@@ -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
diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
new file mode 100644
index 0000000..51c0480
--- /dev/null
+++ b/package/usbip/usbip.mk
@@ -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))
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 17:39   ` Tal Shorer
  2016-12-11 17:45     ` Yann E. MORIN
@ 2016-12-11 19:59     ` Arnout Vandecappelle
  2016-12-11 21:40       ` Yann E. MORIN
  2016-12-11 21:46       ` Tal Shorer
  1 sibling, 2 replies; 34+ messages in thread
From: Arnout Vandecappelle @ 2016-12-11 19:59 UTC (permalink / raw)
  To: buildroot



On 11-12-16 18:39, Tal Shorer wrote:
> On Sun, Dec 11, 2016 at 3:56 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> Tal, Thomas, All,
>>
>> On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
[snip]
>>> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
>>> new file mode 100644
>>> index 0000000..e6bd7f7
>>> --- /dev/null
>>> +++ b/package/usbip/usbip.mk
>>> @@ -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.

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

>>
> 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_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:
[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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 19:59     ` Arnout Vandecappelle
@ 2016-12-11 21:40       ` Yann E. MORIN
  2016-12-11 21:46       ` Tal Shorer
  1 sibling, 0 replies; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-11 21:40 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2016-12-11 20:59 +0100, Arnout Vandecappelle spake thusly:
> On 11-12-16 18:39, Tal Shorer wrote:
> > On Sun, Dec 11, 2016 at 3:56 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> >> Tal, Thomas, All,
> >>
> >> On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
> [snip]
> >>> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
> >>> new file mode 100644
> >>> index 0000000..e6bd7f7
> >>> --- /dev/null
> >>> +++ b/package/usbip/usbip.mk
> >>> @@ -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.

Hmm... Bad... :-(

But see below...

> >> 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"
> > 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?
> >>
> >>     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.

That's quite a bit of an overhead...

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

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 19:59     ` Arnout Vandecappelle
  2016-12-11 21:40       ` Yann E. MORIN
@ 2016-12-11 21:46       ` Tal Shorer
  2016-12-11 22:05         ` Yann E. MORIN
  1 sibling, 1 reply; 34+ messages in thread
From: Tal Shorer @ 2016-12-11 21:46 UTC (permalink / raw)
  To: buildroot

On Sun, Dec 11, 2016 at 9:59 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
> On 11-12-16 18:39, Tal Shorer wrote:
>> On Sun, Dec 11, 2016 at 3:56 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>> Tal, Thomas, All,
>>>
>>> On 2016-12-09 10:37 +0200, Tal Shorer spake thusly:
> [snip]
>>>> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
>>>> new file mode 100644
>>>> index 0000000..e6bd7f7
>>>> --- /dev/null
>>>> +++ b/package/usbip/usbip.mk
>>>> @@ -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.
>
>>>
>>> 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"
>> 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?
>>>
>>>     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)
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 like
USBIP_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.
>
>
>
>  Regards,
>  Arnout
>
>>>
>> 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_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:
> [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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 21:46       ` Tal Shorer
@ 2016-12-11 22:05         ` Yann E. MORIN
  2016-12-11 22:11           ` Tal Shorer
  0 siblings, 1 reply; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-11 22:05 UTC (permalink / raw)
  To: buildroot

Tal, Arnout, All,

On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
> On Sun, Dec 11, 2016 at 9:59 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
[--SNIP--]
> >  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)
> 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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 22:05         ` Yann E. MORIN
@ 2016-12-11 22:11           ` Tal Shorer
  2016-12-11 22:20             ` Yann E. MORIN
  0 siblings, 1 reply; 34+ messages in thread
From: Tal Shorer @ 2016-12-11 22:11 UTC (permalink / raw)
  To: buildroot

On Mon, Dec 12, 2016 at 12:05 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Tal, Arnout, All,
>
> On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
>> On Sun, Dec 11, 2016 at 9:59 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> [--SNIP--]
>> >  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)
>> 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? ;-)
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
>
> 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.  |
> '------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 22:11           ` Tal Shorer
@ 2016-12-11 22:20             ` Yann E. MORIN
  2016-12-11 22:30               ` Tal Shorer
  0 siblings, 1 reply; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-11 22:20 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-12 00:11 +0200, Tal Shorer spake thusly:
> On Mon, Dec 12, 2016 at 12:05 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > Tal, Arnout, All,
> >
> > On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
> >> On Sun, Dec 11, 2016 at 9:59 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> > [--SNIP--]
> >> >  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)
> >> 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? ;-)
> 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...

> 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

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 22:20             ` Yann E. MORIN
@ 2016-12-11 22:30               ` Tal Shorer
  2016-12-12  9:40                 ` Arnout Vandecappelle
  2016-12-12 17:09                 ` [Buildroot] [PATCH v2 1/2] package: linux-tools: allow tools to define configure hooks Tal Shorer
  0 siblings, 2 replies; 34+ messages in thread
From: Tal Shorer @ 2016-12-11 22:30 UTC (permalink / raw)
  To: buildroot

On Mon, Dec 12, 2016 at 12:20 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Tal, All,
>
> On 2016-12-12 00:11 +0200, Tal Shorer spake thusly:
>> On Mon, Dec 12, 2016 at 12:05 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> > Tal, Arnout, All,
>> >
>> > On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
>> >> On Sun, Dec 11, 2016 at 9:59 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> > [--SNIP--]
>> >> >  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)
>> >> 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? ;-)
>> 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...
>
>> 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
>
> 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.
>
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 :)
> 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.  |
> '------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-11 22:30               ` Tal Shorer
@ 2016-12-12  9:40                 ` Arnout Vandecappelle
  2016-12-12 17:25                   ` Yann E. MORIN
  2016-12-12 17:09                 ` [Buildroot] [PATCH v2 1/2] package: linux-tools: allow tools to define configure hooks Tal Shorer
  1 sibling, 1 reply; 34+ messages in thread
From: Arnout Vandecappelle @ 2016-12-12  9:40 UTC (permalink / raw)
  To: buildroot



On 11-12-16 23:30, Tal Shorer wrote:
> On Mon, Dec 12, 2016 at 12:20 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> Tal, All,
>>
>> On 2016-12-12 00:11 +0200, Tal Shorer spake thusly:
>>> On Mon, Dec 12, 2016 at 12:05 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>> Tal, Arnout, All,
>>>>
>>>> On 2016-12-11 23:46 +0200, Tal Shorer spake thusly:
>>>>> On Sun, Dec 11, 2016 at 9:59 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>>> [--SNIP--]
>>>>>>  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)
>>>>> 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? ;-)
>>> 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.

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

> conjure something up tomorrow since it's getting pretty late in the
> GMT+2 part of the world :)
>> 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.  |
>> '------------------------------^-------^------------------^--------------------'

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v2 1/2] package: linux-tools: allow tools to define configure hooks
  2016-12-11 22:30               ` Tal Shorer
  2016-12-12  9:40                 ` Arnout Vandecappelle
@ 2016-12-12 17:09                 ` Tal Shorer
  2016-12-12 17:09                   ` [Buildroot] [PATCH v2 2/2] package: linux-tools: add usbip Tal Shorer
  1 sibling, 1 reply; 34+ messages in thread
From: Tal Shorer @ 2016-12-12 17:09 UTC (permalink / raw)
  To: buildroot

define 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

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 package/linux-tools/linux-tools.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/linux-tools/linux-tools.mk b/package/linux-tools/linux-tools.mk
index 7fa8d19..1f752f3 100644
--- a/package/linux-tools/linux-tools.mk
+++ b/package/linux-tools/linux-tools.mk
@@ -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))
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v2 2/2] package: linux-tools: add usbip
  2016-12-12 17:09                 ` [Buildroot] [PATCH v2 1/2] package: linux-tools: allow tools to define configure hooks Tal Shorer
@ 2016-12-12 17:09                   ` Tal Shorer
  2016-12-12 17:26                     ` Yann E. MORIN
  0 siblings, 1 reply; 34+ messages in thread
From: Tal Shorer @ 2016-12-12 17:09 UTC (permalink / raw)
  To: buildroot

add the usbip package from the kernel source, allowing users to share
usb devices over a network connection

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 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

diff --git a/package/linux-tools/Config.in b/package/linux-tools/Config.in
index db9ed9f..83b5ee0 100644
--- a/package/linux-tools/Config.in
+++ b/package/linux-tools/Config.in
@@ -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
diff --git a/package/linux-tools/linux-tool-usbip.mk b/package/linux-tools/linux-tool-usbip.mk
new file mode 100644
index 0000000..1ec01b5
--- /dev/null
+++ b/package/linux-tools/linux-tool-usbip.mk
@@ -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
+endef
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-12  9:40                 ` Arnout Vandecappelle
@ 2016-12-12 17:25                   ` Yann E. MORIN
  2016-12-12 22:17                     ` Tal Shorer
  0 siblings, 1 reply; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-12 17:25 UTC (permalink / raw)
  To: buildroot

Tal, Arnout, All,

On 2016-12-12 10:40 +0100, Arnout Vandecappelle spake thusly:
> On 11-12-16 23:30, Tal Shorer wrote:
> > 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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v2 2/2] package: linux-tools: add usbip
  2016-12-12 17:09                   ` [Buildroot] [PATCH v2 2/2] package: linux-tools: add usbip Tal Shorer
@ 2016-12-12 17:26                     ` Yann E. MORIN
  0 siblings, 0 replies; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-12 17:26 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-12 19:09 +0200, Tal Shorer spake thusly:
> 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.

> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
> ---
>  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
> 
> diff --git a/package/linux-tools/Config.in b/package/linux-tools/Config.in
> index db9ed9f..83b5ee0 100644
> --- a/package/linux-tools/Config.in
> +++ b/package/linux-tools/Config.in
> @@ -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
> diff --git a/package/linux-tools/linux-tool-usbip.mk b/package/linux-tools/linux-tool-usbip.mk
> new file mode 100644
> index 0000000..1ec01b5
> --- /dev/null
> +++ b/package/linux-tools/linux-tool-usbip.mk
> @@ -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
> +endef
> -- 
> 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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-12 17:25                   ` Yann E. MORIN
@ 2016-12-12 22:17                     ` Tal Shorer
  2016-12-12 22:21                       ` [Buildroot] [PATCH v3] usbip: " Tal Shorer
  2016-12-13 17:35                       ` [Buildroot] [PATCH] usbip: add a " Yann E. MORIN
  0 siblings, 2 replies; 34+ messages in thread
From: Tal Shorer @ 2016-12-12 22:17 UTC (permalink / raw)
  To: buildroot

On Mon, Dec 12, 2016 at 7:25 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Tal, Arnout, All,
>
> On 2016-12-12 10:40 +0100, Arnout Vandecappelle spake thusly:
>> On 11-12-16 23:30, Tal Shorer wrote:
>> > 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.
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
>
> 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.  |
> '------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v3] usbip: new package
  2016-12-12 22:17                     ` Tal Shorer
@ 2016-12-12 22:21                       ` Tal Shorer
  2016-12-13 17:53                         ` Yann E. MORIN
  2016-12-13 17:35                       ` [Buildroot] [PATCH] usbip: add a " Yann E. MORIN
  1 sibling, 1 reply; 34+ messages in thread
From: Tal Shorer @ 2016-12-12 22:21 UTC (permalink / raw)
  To: buildroot

add the usbip package from the kernel source, allowing users to share
usb devices over a network connection

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 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

diff --git a/package/linux-tools/Config.in b/package/linux-tools/Config.in
index db9ed9f..7fceca7 100644
--- a/package/linux-tools/Config.in
+++ b/package/linux-tools/Config.in
@@ -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
diff --git a/package/usbip/Config.in b/package/usbip/Config.in
new file mode 100644
index 0000000..becc437
--- /dev/null
+++ b/package/usbip/Config.in
@@ -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.
diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
new file mode 100644
index 0000000..72c5c58
--- /dev/null
+++ b/package/usbip/usbip.mk
@@ -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))
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] usbip: add a new package
  2016-12-12 22:17                     ` Tal Shorer
  2016-12-12 22:21                       ` [Buildroot] [PATCH v3] usbip: " Tal Shorer
@ 2016-12-13 17:35                       ` Yann E. MORIN
  1 sibling, 0 replies; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-13 17:35 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-13 00:17 +0200, Tal Shorer spake thusly:
> On Mon, Dec 12, 2016 at 7:25 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > Tal, Arnout, All,
> >
> > On 2016-12-12 10:40 +0100, Arnout Vandecappelle spake thusly:
> >> On 11-12-16 23:30, Tal Shorer wrote:
> >> > 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.
> This works after some modifications.

Yes, it was definitely buggy, it was just a mock-up...

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

> I'll send a new patch

Thanks, I'll review it now.

Regards,
Yann E. MORIN.

> >
> > 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.  |
> > '------------------------------^-------^------------------^--------------------'

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v3] usbip: new package
  2016-12-12 22:21                       ` [Buildroot] [PATCH v3] usbip: " Tal Shorer
@ 2016-12-13 17:53                         ` Yann E. MORIN
  2016-12-14 17:28                           ` [Buildroot] [PATCH v4] " Tal Shorer
  0 siblings, 1 reply; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-13 17:53 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-13 00:21 +0200, Tal Shorer spake thusly:
> add the usbip package from the kernel source, allowing users to share
> usb devices over a network connection
> 
> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
> ---
>  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
> 
> diff --git a/package/linux-tools/Config.in b/package/linux-tools/Config.in
> index db9ed9f..7fceca7 100644
> --- a/package/linux-tools/Config.in
> +++ b/package/linux-tools/Config.in
> @@ -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
> diff --git a/package/usbip/Config.in b/package/usbip/Config.in
> new file mode 100644
> index 0000000..becc437
> --- /dev/null
> +++ b/package/usbip/Config.in
> @@ -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...

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

> +	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.
> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
> new file mode 100644
> index 0000000..72c5c58
> --- /dev/null
> +++ b/package/usbip/usbip.mk
> @@ -0,0 +1,21 @@
> +###############
> +# usbip
> +###############

Incorrect header. ;-)

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

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

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

> +endef
> +
> +$(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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v4] usbip: new package
  2016-12-13 17:53                         ` Yann E. MORIN
@ 2016-12-14 17:28                           ` Tal Shorer
  2016-12-21 22:17                             ` Tal Shorer
                                               ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Tal Shorer @ 2016-12-14 17:28 UTC (permalink / raw)
  To: buildroot

add the usbip package from the kernel source, allowing users to share
usb devices over a network connection

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 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

diff --git a/package/linux-tools/Config.in b/package/linux-tools/Config.in
index db9ed9f..7fceca7 100644
--- a/package/linux-tools/Config.in
+++ b/package/linux-tools/Config.in
@@ -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
diff --git a/package/usbip/Config.in b/package/usbip/Config.in
new file mode 100644
index 0000000..bb92a05
--- /dev/null
+++ b/package/usbip/Config.in
@@ -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
diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
new file mode 100644
index 0000000..7c8e244
--- /dev/null
+++ b/package/usbip/usbip.mk
@@ -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))
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR)
  2016-12-11 18:12       ` [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR) Tal Shorer
  2016-12-11 19:19         ` Yann E. MORIN
  2016-12-11 19:44         ` [Buildroot] [PATCH] package/usbip: new package Tal Shorer
@ 2016-12-17 15:00         ` Thomas Petazzoni
  2 siblings, 0 replies; 34+ messages in thread
From: Thomas Petazzoni @ 2016-12-17 15:00 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 11 Dec 2016 20:12:45 +0200, Tal Shorer wrote:
> 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 <tal.shorer@gmail.com>
> ---
>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v4] usbip: new package
  2016-12-14 17:28                           ` [Buildroot] [PATCH v4] " Tal Shorer
@ 2016-12-21 22:17                             ` Tal Shorer
  2016-12-22 20:45                             ` Yann E. MORIN
  2016-12-23 18:24                             ` Yann E. MORIN
  2 siblings, 0 replies; 34+ messages in thread
From: Tal Shorer @ 2016-12-21 22:17 UTC (permalink / raw)
  To: buildroot

On Wed, Dec 14, 2016 at 7:28 PM, Tal Shorer <tal.shorer@gmail.com> wrote:
> add the usbip package from the kernel source, allowing users to share
> usb devices over a network connection
>
> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
> ---
>  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
>
> diff --git a/package/linux-tools/Config.in b/package/linux-tools/Config.in
> index db9ed9f..7fceca7 100644
> --- a/package/linux-tools/Config.in
> +++ b/package/linux-tools/Config.in
> @@ -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
> diff --git a/package/usbip/Config.in b/package/usbip/Config.in
> new file mode 100644
> index 0000000..bb92a05
> --- /dev/null
> +++ b/package/usbip/Config.in
> @@ -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
> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
> new file mode 100644
> index 0000000..7c8e244
> --- /dev/null
> +++ b/package/usbip/usbip.mk
> @@ -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))
> --
> 2.7.4
>
ping?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v4] usbip: new package
  2016-12-14 17:28                           ` [Buildroot] [PATCH v4] " Tal Shorer
  2016-12-21 22:17                             ` Tal Shorer
@ 2016-12-22 20:45                             ` Yann E. MORIN
  2016-12-23 18:24                             ` Yann E. MORIN
  2 siblings, 0 replies; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-22 20:45 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-14 19:28 +0200, Tal Shorer spake thusly:
> add the usbip package from the kernel source, allowing users to share
> usb devices over a network connection
> 
> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
[--SNIP--]
> diff --git a/package/usbip/Config.in b/package/usbip/Config.in
> new file mode 100644
> index 0000000..bb92a05
> --- /dev/null
> +++ b/package/usbip/Config.in
> @@ -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

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

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

> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
> new file mode 100644
> index 0000000..7c8e244
> --- /dev/null
> +++ b/package/usbip/usbip.mk
> @@ -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))
> -- 
> 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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v4] usbip: new package
  2016-12-14 17:28                           ` [Buildroot] [PATCH v4] " Tal Shorer
  2016-12-21 22:17                             ` Tal Shorer
  2016-12-22 20:45                             ` Yann E. MORIN
@ 2016-12-23 18:24                             ` Yann E. MORIN
  2016-12-23 19:10                               ` Yann E. MORIN
  2 siblings, 1 reply; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-23 18:24 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-14 19:28 +0200, Tal Shorer spake thusly:
> 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=y

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

> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
> ---
>  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
> 
> diff --git a/package/linux-tools/Config.in b/package/linux-tools/Config.in
> index db9ed9f..7fceca7 100644
> --- a/package/linux-tools/Config.in
> +++ b/package/linux-tools/Config.in
> @@ -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
> diff --git a/package/usbip/Config.in b/package/usbip/Config.in
> new file mode 100644
> index 0000000..bb92a05
> --- /dev/null
> +++ b/package/usbip/Config.in
> @@ -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
> diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
> new file mode 100644
> index 0000000..7c8e244
> --- /dev/null
> +++ b/package/usbip/usbip.mk
> @@ -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))
> -- 
> 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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Buildroot] [PATCH v4] usbip: new package
  2016-12-23 18:24                             ` Yann E. MORIN
@ 2016-12-23 19:10                               ` Yann E. MORIN
  0 siblings, 0 replies; 34+ messages in thread
From: Yann E. MORIN @ 2016-12-23 19:10 UTC (permalink / raw)
  To: buildroot

Tal, All,

On 2016-12-23 19:24 +0100, Yann E. MORIN spake thusly:
> On 2016-12-14 19:28 +0200, Tal Shorer spake thusly:
> > 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--]
> 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:

diff --git a/package/usbip/Config.in b/package/usbip/Config.in
index bb92a05..d12839a 100644
--- a/package/usbip/Config.in
+++ b/package/usbip/Config.in
@@ -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
diff --git a/package/usbip/usbip.mk b/package/usbip/usbip.mk
index 7c8e244..43d0466 100644
--- a/package/usbip/usbip.mk
+++ b/package/usbip/usbip.mk
@@ -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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply related	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2016-12-23 19:10 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-09  8:37 [Buildroot] [PATCH] usbip: add a new package Tal Shorer
2016-12-10 13:31 ` Thomas Petazzoni
2016-12-10 18:39   ` Tal Shorer
2016-12-11 10:06     ` Yann E. MORIN
2016-12-11 13:56 ` Yann E. MORIN
2016-12-11 17:39   ` Tal Shorer
2016-12-11 17:45     ` Yann E. MORIN
2016-12-11 18:12       ` [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR) Tal Shorer
2016-12-11 19:19         ` Yann E. MORIN
2016-12-11 19:44         ` [Buildroot] [PATCH] package/usbip: new package Tal Shorer
2016-12-17 15:00         ` [Buildroot] [PATCH] pkg-generic: run $(PKG)_PRE_RSYNC_HOOKS before checking for the existence of $(SRCDIR) Thomas Petazzoni
2016-12-11 18:31       ` [Buildroot] [PATCH] usbip: add a new package Tal Shorer
2016-12-11 19:24         ` Yann E. MORIN
2016-12-11 19:59     ` Arnout Vandecappelle
2016-12-11 21:40       ` Yann E. MORIN
2016-12-11 21:46       ` Tal Shorer
2016-12-11 22:05         ` Yann E. MORIN
2016-12-11 22:11           ` Tal Shorer
2016-12-11 22:20             ` Yann E. MORIN
2016-12-11 22:30               ` Tal Shorer
2016-12-12  9:40                 ` Arnout Vandecappelle
2016-12-12 17:25                   ` Yann E. MORIN
2016-12-12 22:17                     ` Tal Shorer
2016-12-12 22:21                       ` [Buildroot] [PATCH v3] usbip: " Tal Shorer
2016-12-13 17:53                         ` Yann E. MORIN
2016-12-14 17:28                           ` [Buildroot] [PATCH v4] " Tal Shorer
2016-12-21 22:17                             ` Tal Shorer
2016-12-22 20:45                             ` Yann E. MORIN
2016-12-23 18:24                             ` Yann E. MORIN
2016-12-23 19:10                               ` Yann E. MORIN
2016-12-13 17:35                       ` [Buildroot] [PATCH] usbip: add a " Yann E. MORIN
2016-12-12 17:09                 ` [Buildroot] [PATCH v2 1/2] package: linux-tools: allow tools to define configure hooks Tal Shorer
2016-12-12 17:09                   ` [Buildroot] [PATCH v2 2/2] package: linux-tools: add usbip Tal Shorer
2016-12-12 17:26                     ` Yann E. MORIN

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.