All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
@ 2014-07-31 20:40 Jean-Christophe DUBOIS
  2014-08-02 20:55 ` Jean-Christophe DUBOIS
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Jean-Christophe DUBOIS @ 2014-07-31 20:40 UTC (permalink / raw)
  To: buildroot

This patch adds the bcache-tools package to buildroot.

This is the bcache tools, required to setup the linux bcache
feature of the Linux kernel.

Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>
---
Changes v1 to v2
 * remove 'support' word from comment statement in Config.in.
 * use github helper to get the project site instead of hardcoded URL.
---
 package/Config.in                    |  1 +
 package/bcache-tools/Config.in       | 11 +++++++++++
 package/bcache-tools/bcache-tools.mk | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+)
 create mode 100644 package/bcache-tools/Config.in
 create mode 100644 package/bcache-tools/bcache-tools.mk

diff --git a/package/Config.in b/package/Config.in
index 29b8e6b..7468b28 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -279,6 +279,7 @@ endmenu
 	source "package/a10disp/Config.in"
 	source "package/acpid/Config.in"
 	source "package/avrdude/Config.in"
+	source "package/bcache-tools/Config.in"
 	source "package/cdrkit/Config.in"
 	source "package/cryptsetup/Config.in"
 	source "package/cwiid/Config.in"
diff --git a/package/bcache-tools/Config.in b/package/bcache-tools/Config.in
new file mode 100644
index 0000000..fbe4158
--- /dev/null
+++ b/package/bcache-tools/Config.in
@@ -0,0 +1,11 @@
+config BR2_PACKAGE_BCACHE_TOOLS
+	bool "bcache tools"
+	depends on BR2_PACKAGE_HAS_UDEV
+	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
+	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
+	help
+	  This is the bcache tools, required to setup the linux bcache
+	  feature of the Linux kernel.
+
+comment "bcache-tools needs udev /dev management"
+        depends on !BR2_PACKAGE_HAS_UDEV
diff --git a/package/bcache-tools/bcache-tools.mk b/package/bcache-tools/bcache-tools.mk
new file mode 100644
index 0000000..8b0b061
--- /dev/null
+++ b/package/bcache-tools/bcache-tools.mk
@@ -0,0 +1,34 @@
+################################################################################
+#
+# bcache-tools
+#
+################################################################################
+
+BCACHE_TOOLS_VERSION = 1.0.7
+BCACHE_TOOLS_SITE = $(call github,g2p,bcache-tools,v$(BCACHE_TOOLS_VERSION))
+BCACHE_TOOLS_SOURCE = v$(BCACHE_TOOLS_VERSION).zip
+BCACHE_TOOLS_LICENSE = GPLv2
+BCACHE_TOOLS_LICENSE_FILES = COPYING
+BCACHE_TOOLS_DEPENDENCIES = host-pkgconf util-linux eudev
+BCACHE_TOOLS_MAKE_ENV = $(TARGET_MAKE_ENV) CC="$(TARGET_CC)"
+
+define BCACHE_TOOLS_EXTRACT_CMDS
+	unzip -d $(BUILD_DIR) $(DL_DIR)/$(BCACHE_TOOLS_SOURCE)
+endef
+
+define BCACHE_TOOLS_FIX_PKGCONFIG
+	$(SED) 's^`pkg-config^`$(PKG_CONFIG_HOST_BINARY)^' \
+		$(BCACHE_TOOLS_DIR)/Makefile
+endef
+
+define BCACHE_TOOLS_BUILD_CMDS
+	$(BCACHE_TOOLS_FIX_PKGCONFIG)
+	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D)
+endef
+
+define BCACHE_TOOLS_INSTALL_TARGET_CMDS
+	$(INSTALL) -m 0755 -d $(TARGET_DIR)/usr/share/man/man8
+	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
+endef
+
+$(eval $(generic-package))
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
  2014-07-31 20:40 [Buildroot] [PATCH v2] bcache-tools: Adding package Jean-Christophe DUBOIS
@ 2014-08-02 20:55 ` Jean-Christophe DUBOIS
  2014-08-03  8:39   ` Yann E. MORIN
  2014-08-03 18:53 ` Yann E. MORIN
  2014-08-04  4:51 ` Thomas De Schampheleire
  2 siblings, 1 reply; 10+ messages in thread
From: Jean-Christophe DUBOIS @ 2014-08-02 20:55 UTC (permalink / raw)
  To: buildroot

Sorry to bother you guys, but how do I know the patch is accepted (or not) ?

Thanks.

JC

Le 07/31/2014 10:40 PM, Jean-Christophe DUBOIS a ?crit :
> This patch adds the bcache-tools package to buildroot.
>
> This is the bcache tools, required to setup the linux bcache
> feature of the Linux kernel.
>
> Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>
> ---
> Changes v1 to v2
>   * remove 'support' word from comment statement in Config.in.
>   * use github helper to get the project site instead of hardcoded URL.
> ---
>   package/Config.in                    |  1 +
>   package/bcache-tools/Config.in       | 11 +++++++++++
>   package/bcache-tools/bcache-tools.mk | 34 ++++++++++++++++++++++++++++++++++
>   3 files changed, 46 insertions(+)
>   create mode 100644 package/bcache-tools/Config.in
>   create mode 100644 package/bcache-tools/bcache-tools.mk
>
> diff --git a/package/Config.in b/package/Config.in
> index 29b8e6b..7468b28 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -279,6 +279,7 @@ endmenu
>   	source "package/a10disp/Config.in"
>   	source "package/acpid/Config.in"
>   	source "package/avrdude/Config.in"
> +	source "package/bcache-tools/Config.in"
>   	source "package/cdrkit/Config.in"
>   	source "package/cryptsetup/Config.in"
>   	source "package/cwiid/Config.in"
> diff --git a/package/bcache-tools/Config.in b/package/bcache-tools/Config.in
> new file mode 100644
> index 0000000..fbe4158
> --- /dev/null
> +++ b/package/bcache-tools/Config.in
> @@ -0,0 +1,11 @@
> +config BR2_PACKAGE_BCACHE_TOOLS
> +	bool "bcache tools"
> +	depends on BR2_PACKAGE_HAS_UDEV
> +	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
> +	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
> +	help
> +	  This is the bcache tools, required to setup the linux bcache
> +	  feature of the Linux kernel.
> +
> +comment "bcache-tools needs udev /dev management"
> +        depends on !BR2_PACKAGE_HAS_UDEV
> diff --git a/package/bcache-tools/bcache-tools.mk b/package/bcache-tools/bcache-tools.mk
> new file mode 100644
> index 0000000..8b0b061
> --- /dev/null
> +++ b/package/bcache-tools/bcache-tools.mk
> @@ -0,0 +1,34 @@
> +################################################################################
> +#
> +# bcache-tools
> +#
> +################################################################################
> +
> +BCACHE_TOOLS_VERSION = 1.0.7
> +BCACHE_TOOLS_SITE = $(call github,g2p,bcache-tools,v$(BCACHE_TOOLS_VERSION))
> +BCACHE_TOOLS_SOURCE = v$(BCACHE_TOOLS_VERSION).zip
> +BCACHE_TOOLS_LICENSE = GPLv2
> +BCACHE_TOOLS_LICENSE_FILES = COPYING
> +BCACHE_TOOLS_DEPENDENCIES = host-pkgconf util-linux eudev
> +BCACHE_TOOLS_MAKE_ENV = $(TARGET_MAKE_ENV) CC="$(TARGET_CC)"
> +
> +define BCACHE_TOOLS_EXTRACT_CMDS
> +	unzip -d $(BUILD_DIR) $(DL_DIR)/$(BCACHE_TOOLS_SOURCE)
> +endef
> +
> +define BCACHE_TOOLS_FIX_PKGCONFIG
> +	$(SED) 's^`pkg-config^`$(PKG_CONFIG_HOST_BINARY)^' \
> +		$(BCACHE_TOOLS_DIR)/Makefile
> +endef
> +
> +define BCACHE_TOOLS_BUILD_CMDS
> +	$(BCACHE_TOOLS_FIX_PKGCONFIG)
> +	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D)
> +endef
> +
> +define BCACHE_TOOLS_INSTALL_TARGET_CMDS
> +	$(INSTALL) -m 0755 -d $(TARGET_DIR)/usr/share/man/man8
> +	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
> +endef
> +
> +$(eval $(generic-package))

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
  2014-08-02 20:55 ` Jean-Christophe DUBOIS
@ 2014-08-03  8:39   ` Yann E. MORIN
  0 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2014-08-03  8:39 UTC (permalink / raw)
  To: buildroot

Jean-Christophe, All,

On 2014-08-02 22:55 +0200, Jean-Christophe DUBOIS spake thusly:
> Sorry to bother you guys, but how do I know the patch is accepted (or not) ?

You'll get a mail that it is applied. It can take a few days, or even a
week before someone gets to review a patch.

I'm interested in bcache-tools, so I've queued it for review.

Don't worry, your patch is not forgotten. Sorry it takes long, but
reviewing is not a simple task...

In the meantime, we have a lot of patches to review and test:
    http://patchwork.ozlabs.org/project/buildroot/list/

A bit of help in testing them is always welcome! ;-)

Regards,
Yann E. MORIN.

> Le 07/31/2014 10:40 PM, Jean-Christophe DUBOIS a ?crit :
> >This patch adds the bcache-tools package to buildroot.
> >
> >This is the bcache tools, required to setup the linux bcache
> >feature of the Linux kernel.
> >
> >Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>
> >---
> >Changes v1 to v2
> >  * remove 'support' word from comment statement in Config.in.
> >  * use github helper to get the project site instead of hardcoded URL.
> >---
> >  package/Config.in                    |  1 +
> >  package/bcache-tools/Config.in       | 11 +++++++++++
> >  package/bcache-tools/bcache-tools.mk | 34 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 46 insertions(+)
> >  create mode 100644 package/bcache-tools/Config.in
> >  create mode 100644 package/bcache-tools/bcache-tools.mk
> >
> >diff --git a/package/Config.in b/package/Config.in
> >index 29b8e6b..7468b28 100644
> >--- a/package/Config.in
> >+++ b/package/Config.in
> >@@ -279,6 +279,7 @@ endmenu
> >  	source "package/a10disp/Config.in"
> >  	source "package/acpid/Config.in"
> >  	source "package/avrdude/Config.in"
> >+	source "package/bcache-tools/Config.in"
> >  	source "package/cdrkit/Config.in"
> >  	source "package/cryptsetup/Config.in"
> >  	source "package/cwiid/Config.in"
> >diff --git a/package/bcache-tools/Config.in b/package/bcache-tools/Config.in
> >new file mode 100644
> >index 0000000..fbe4158
> >--- /dev/null
> >+++ b/package/bcache-tools/Config.in
> >@@ -0,0 +1,11 @@
> >+config BR2_PACKAGE_BCACHE_TOOLS
> >+	bool "bcache tools"
> >+	depends on BR2_PACKAGE_HAS_UDEV
> >+	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
> >+	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
> >+	help
> >+	  This is the bcache tools, required to setup the linux bcache
> >+	  feature of the Linux kernel.
> >+
> >+comment "bcache-tools needs udev /dev management"
> >+        depends on !BR2_PACKAGE_HAS_UDEV
> >diff --git a/package/bcache-tools/bcache-tools.mk b/package/bcache-tools/bcache-tools.mk
> >new file mode 100644
> >index 0000000..8b0b061
> >--- /dev/null
> >+++ b/package/bcache-tools/bcache-tools.mk
> >@@ -0,0 +1,34 @@
> >+################################################################################
> >+#
> >+# bcache-tools
> >+#
> >+################################################################################
> >+
> >+BCACHE_TOOLS_VERSION = 1.0.7
> >+BCACHE_TOOLS_SITE = $(call github,g2p,bcache-tools,v$(BCACHE_TOOLS_VERSION))
> >+BCACHE_TOOLS_SOURCE = v$(BCACHE_TOOLS_VERSION).zip
> >+BCACHE_TOOLS_LICENSE = GPLv2
> >+BCACHE_TOOLS_LICENSE_FILES = COPYING
> >+BCACHE_TOOLS_DEPENDENCIES = host-pkgconf util-linux eudev
> >+BCACHE_TOOLS_MAKE_ENV = $(TARGET_MAKE_ENV) CC="$(TARGET_CC)"
> >+
> >+define BCACHE_TOOLS_EXTRACT_CMDS
> >+	unzip -d $(BUILD_DIR) $(DL_DIR)/$(BCACHE_TOOLS_SOURCE)
> >+endef
> >+
> >+define BCACHE_TOOLS_FIX_PKGCONFIG
> >+	$(SED) 's^`pkg-config^`$(PKG_CONFIG_HOST_BINARY)^' \
> >+		$(BCACHE_TOOLS_DIR)/Makefile
> >+endef
> >+
> >+define BCACHE_TOOLS_BUILD_CMDS
> >+	$(BCACHE_TOOLS_FIX_PKGCONFIG)
> >+	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D)
> >+endef
> >+
> >+define BCACHE_TOOLS_INSTALL_TARGET_CMDS
> >+	$(INSTALL) -m 0755 -d $(TARGET_DIR)/usr/share/man/man8
> >+	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
> >+endef
> >+
> >+$(eval $(generic-package))
> 
> _______________________________________________
> 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] 10+ messages in thread
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
  2014-07-31 20:40 [Buildroot] [PATCH v2] bcache-tools: Adding package Jean-Christophe DUBOIS
  2014-08-02 20:55 ` Jean-Christophe DUBOIS
@ 2014-08-03 18:53 ` Yann E. MORIN
  2014-08-04 15:28   ` Jean-Christophe DUBOIS
  2014-08-04 19:53   ` Jean-Christophe DUBOIS
  2014-08-04  4:51 ` Thomas De Schampheleire
  2 siblings, 2 replies; 10+ messages in thread
From: Yann E. MORIN @ 2014-08-03 18:53 UTC (permalink / raw)
  To: buildroot

Jean-Christophe, All,

On 2014-07-31 22:40 +0200, Jean-Christophe DUBOIS spake thusly:
> This patch adds the bcache-tools package to buildroot.

This line above is not needed: it is obvious this is adding the package
to Buildroot! ;-)

> This is the bcache tools, required to setup the linux bcache
> feature of the Linux kernel.
> 
> Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>
> ---
> diff --git a/package/bcache-tools/Config.in b/package/bcache-tools/Config.in
> new file mode 100644
> index 0000000..fbe4158
> --- /dev/null
> +++ b/package/bcache-tools/Config.in
> @@ -0,0 +1,11 @@
> +config BR2_PACKAGE_BCACHE_TOOLS
> +	bool "bcache tools"
> +	depends on BR2_PACKAGE_HAS_UDEV
> +	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
> +	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID

Since you select sub-options of util-linux, you must also select the
package itself:
    select BR2_PACKAGE_UTIL_LINUX

But you must also inherit its dependencies:
    depends on BR2_LARGEFILE # util-linux
    depends on BR2_USE_WCHAR # util-linux

But you must also inherit from the sub-options' dependencies, too:
    depends on BR2_USE_MMU # util-linux (libblkid)

> +	help
> +	  This is the bcache tools, required to setup the linux bcache
> +	  feature of the Linux kernel.
> +
> +comment "bcache-tools needs udev /dev management"
> +        depends on !BR2_PACKAGE_HAS_UDEV

And you must adapt the comment to the dependencies above, like:
    bcache needs udev /dev management and a toolchain w/ largefile, wchar

> diff --git a/package/bcache-tools/bcache-tools.mk b/package/bcache-tools/bcache-tools.mk
> new file mode 100644
> index 0000000..8b0b061
> --- /dev/null
> +++ b/package/bcache-tools/bcache-tools.mk
> @@ -0,0 +1,34 @@
> +################################################################################
> +#
> +# bcache-tools
> +#
> +################################################################################
> +
> +BCACHE_TOOLS_VERSION = 1.0.7
> +BCACHE_TOOLS_SITE = $(call github,g2p,bcache-tools,v$(BCACHE_TOOLS_VERSION))
> +BCACHE_TOOLS_SOURCE = v$(BCACHE_TOOLS_VERSION).zip
> +BCACHE_TOOLS_LICENSE = GPLv2
> +BCACHE_TOOLS_LICENSE_FILES = COPYING
> +BCACHE_TOOLS_DEPENDENCIES = host-pkgconf util-linux eudev
> +BCACHE_TOOLS_MAKE_ENV = $(TARGET_MAKE_ENV) CC="$(TARGET_CC)"
> +
> +define BCACHE_TOOLS_EXTRACT_CMDS
> +	unzip -d $(BUILD_DIR) $(DL_DIR)/$(BCACHE_TOOLS_SOURCE)
> +endef
> +
> +define BCACHE_TOOLS_FIX_PKGCONFIG
> +	$(SED) 's^`pkg-config^`$(PKG_CONFIG_HOST_BINARY)^' \
> +		$(BCACHE_TOOLS_DIR)/Makefile
> +endef

This should probably be a post-extract hook. See:
    http://buildroot.net/downloads/manual/manual.html#hooks

> +define BCACHE_TOOLS_BUILD_CMDS
> +	$(BCACHE_TOOLS_FIX_PKGCONFIG)
> +	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D)

This should probably be:
    $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS)

TARGET_CONFIGURE_OPTS contains the CC= assignment.

Thus, the BCACHE_TOOLS_MAKE_ENV no longer makes sense, so you can remove
it.

> +endef
> +
> +define BCACHE_TOOLS_INSTALL_TARGET_CMDS
> +	$(INSTALL) -m 0755 -d $(TARGET_DIR)/usr/share/man/man8
> +	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install

Ditto:
    $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) \
        DESTDIR=$(TARGET_DIR) install

Regards,
Yann E. MORIN.

> +endef
> +
> +$(eval $(generic-package))
> -- 
> 1.9.1
> 
> _______________________________________________
> 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] 10+ messages in thread
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
  2014-07-31 20:40 [Buildroot] [PATCH v2] bcache-tools: Adding package Jean-Christophe DUBOIS
  2014-08-02 20:55 ` Jean-Christophe DUBOIS
  2014-08-03 18:53 ` Yann E. MORIN
@ 2014-08-04  4:51 ` Thomas De Schampheleire
  2 siblings, 0 replies; 10+ messages in thread
From: Thomas De Schampheleire @ 2014-08-04  4:51 UTC (permalink / raw)
  To: buildroot

Jean-Christophe DUBOIS <jcd@tribudubois.net> schreef:
>This patch adds the bcache-tools package to buildroot.
>
>This is the bcache tools, required to setup the linux bcache
>feature of the Linux kernel.
>
>Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>
>---
>Changes v1 to v2
> * remove 'support' word from comment statement in Config.in.
> * use github helper to get the project site instead of hardcoded URL.
>---
> package/Config.in                    |  1 +
> package/bcache-tools/Config.in       | 11 +++++++++++
> package/bcache-tools/bcache-tools.mk | 34 ++++++++++++++++++++++++++++++++++
> 3 files changed, 46 insertions(+)
> create mode 100644 package/bcache-tools/Config.in
> create mode 100644 package/bcache-tools/bcache-tools.mk
>
>diff --git a/package/Config.in b/package/Config.in
>index 29b8e6b..7468b28 100644
>--- a/package/Config.in
>+++ b/package/Config.in
>@@ -279,6 +279,7 @@ endmenu
> 	source "package/a10disp/Config.in"
> 	source "package/acpid/Config.in"
> 	source "package/avrdude/Config.in"
>+	source "package/bcache-tools/Config.in"
> 	source "package/cdrkit/Config.in"
> 	source "package/cryptsetup/Config.in"
> 	source "package/cwiid/Config.in"
>diff --git a/package/bcache-tools/Config.in b/package/bcache-tools/Config.in
>new file mode 100644
>index 0000000..fbe4158
>--- /dev/null
>+++ b/package/bcache-tools/Config.in
>@@ -0,0 +1,11 @@
>+config BR2_PACKAGE_BCACHE_TOOLS
>+	bool "bcache tools"
>+	depends on BR2_PACKAGE_HAS_UDEV
>+	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
>+	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
>+	help
>+	  This is the bcache tools, required to setup the linux bcache
>+	  feature of the Linux kernel.

I think a brief explanation of what this bcache feature is could be helpful for users that use the Buildroot menuconfig to detect useful packages.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
  2014-08-03 18:53 ` Yann E. MORIN
@ 2014-08-04 15:28   ` Jean-Christophe DUBOIS
  2014-08-04 20:49     ` Yann E. MORIN
  2014-08-04 19:53   ` Jean-Christophe DUBOIS
  1 sibling, 1 reply; 10+ messages in thread
From: Jean-Christophe DUBOIS @ 2014-08-04 15:28 UTC (permalink / raw)
  To: buildroot

Hello Yann,

Thanks for the feedback/review.

Few questions below.

JC

Le 08/03/2014 08:53 PM, Yann E. MORIN a ?crit :
> Jean-Christophe, All,
>
> On 2014-07-31 22:40 +0200, Jean-Christophe DUBOIS spake thusly:
>> This patch adds the bcache-tools package to buildroot.
> This line above is not needed: it is obvious this is adding the package
> to Buildroot! ;-)
OK
>> This is the bcache tools, required to setup the linux bcache
>> feature of the Linux kernel.
>>
>> Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>
>> ---
>> diff --git a/package/bcache-tools/Config.in b/package/bcache-tools/Config.in
>> new file mode 100644
>> index 0000000..fbe4158
>> --- /dev/null
>> +++ b/package/bcache-tools/Config.in
>> @@ -0,0 +1,11 @@
>> +config BR2_PACKAGE_BCACHE_TOOLS
>> +	bool "bcache tools"
>> +	depends on BR2_PACKAGE_HAS_UDEV
>> +	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
>> +	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
> Since you select sub-options of util-linux, you must also select the
> package itself:
>      select BR2_PACKAGE_UTIL_LINUX
>
> But you must also inherit its dependencies:
>      depends on BR2_LARGEFILE # util-linux
>      depends on BR2_USE_WCHAR # util-linux
>
> But you must also inherit from the sub-options' dependencies, too:
>      depends on BR2_USE_MMU # util-linux (libblkid)
Sorry I am new to buildroot, but wouldn't be expected that selecting 
LIBUUID and LIBBLKID would automatically select the LIBUUID and LIBBLKID 
dependencies and therefore it does not have to be explicitly selected by 
all packages depending on these features ?

If one day the util-linux dependency is changing, will it be expected to 
change the dependency of all package using linux-util?
>> +	help
>> +	  This is the bcache tools, required to setup the linux bcache
>> +	  feature of the Linux kernel.
>> +
>> +comment "bcache-tools needs udev /dev management"
>> +        depends on !BR2_PACKAGE_HAS_UDEV
> And you must adapt the comment to the dependencies above, like:
>      bcache needs udev /dev management and a toolchain w/ largefile, wchar
>
>> diff --git a/package/bcache-tools/bcache-tools.mk b/package/bcache-tools/bcache-tools.mk
>> new file mode 100644
>> index 0000000..8b0b061
>> --- /dev/null
>> +++ b/package/bcache-tools/bcache-tools.mk
>> @@ -0,0 +1,34 @@
>> +################################################################################
>> +#
>> +# bcache-tools
>> +#
>> +################################################################################
>> +
>> +BCACHE_TOOLS_VERSION = 1.0.7
>> +BCACHE_TOOLS_SITE = $(call github,g2p,bcache-tools,v$(BCACHE_TOOLS_VERSION))
>> +BCACHE_TOOLS_SOURCE = v$(BCACHE_TOOLS_VERSION).zip
>> +BCACHE_TOOLS_LICENSE = GPLv2
>> +BCACHE_TOOLS_LICENSE_FILES = COPYING
>> +BCACHE_TOOLS_DEPENDENCIES = host-pkgconf util-linux eudev
>> +BCACHE_TOOLS_MAKE_ENV = $(TARGET_MAKE_ENV) CC="$(TARGET_CC)"
>> +
>> +define BCACHE_TOOLS_EXTRACT_CMDS
>> +	unzip -d $(BUILD_DIR) $(DL_DIR)/$(BCACHE_TOOLS_SOURCE)
>> +endef
>> +
>> +define BCACHE_TOOLS_FIX_PKGCONFIG
>> +	$(SED) 's^`pkg-config^`$(PKG_CONFIG_HOST_BINARY)^' \
>> +		$(BCACHE_TOOLS_DIR)/Makefile
>> +endef
> This should probably be a post-extract hook. See:
>      http://buildroot.net/downloads/manual/manual.html#hooks
I'll look into this. I copied this from the dnsmasq package.
>> +define BCACHE_TOOLS_BUILD_CMDS
>> +	$(BCACHE_TOOLS_FIX_PKGCONFIG)
>> +	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D)
> This should probably be:
>      $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS)
>
> TARGET_CONFIGURE_OPTS contains the CC= assignment.
>
> Thus, the BCACHE_TOOLS_MAKE_ENV no longer makes sense, so you can remove
> it.
OK, this is also coming from dnsmasq package.
>
>> +endef
>> +
>> +define BCACHE_TOOLS_INSTALL_TARGET_CMDS
>> +	$(INSTALL) -m 0755 -d $(TARGET_DIR)/usr/share/man/man8
>> +	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
> Ditto:
>      $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) \
>          DESTDIR=$(TARGET_DIR) install
>
> Regards,
> Yann E. MORIN.
>
>> +endef
>> +
>> +$(eval $(generic-package))
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
  2014-08-03 18:53 ` Yann E. MORIN
  2014-08-04 15:28   ` Jean-Christophe DUBOIS
@ 2014-08-04 19:53   ` Jean-Christophe DUBOIS
  2014-08-04 21:00     ` Yann E. MORIN
  1 sibling, 1 reply; 10+ messages in thread
From: Jean-Christophe DUBOIS @ 2014-08-04 19:53 UTC (permalink / raw)
  To: buildroot

A little question:

Le 08/03/2014 08:53 PM, Yann E. MORIN a ?crit :
> Jean-Christophe, All,
>
> On 2014-07-31 22:40 +0200, Jean-Christophe DUBOIS spake thusly:
>> +define BCACHE_TOOLS_BUILD_CMDS
>> +	$(BCACHE_TOOLS_FIX_PKGCONFIG)
>> +	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D)
> This should probably be:
>      $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS)
>
> TARGET_CONFIGURE_OPTS contains the CC= assignment.
>
> Thus, the BCACHE_TOOLS_MAKE_ENV no longer makes sense, so you can remove
> it.

When I do as you propose then various compilation variables 
$(TARGET_CONFIGURE_OPTS) are passed as $(MAKE) arguments.

Therefore these variables cannot be overridden/changed in the Makefile 
(which might the the desired behavior) unless explicitly allowed with 
the "override" directive.

Unfortunately the bcache-tools Makefile needs (for now) to be able to 
override the CFLAGS variable.

So I can certainly "patch" the bcache-tools Makefile (in the post 
extract hook for example) to allow "overriding" on the CFLAGS variable.

Another solution is to pass $(TARGET_CONFIGURE_OPTS) as environment 
variables to $(MAKE) in which case the various variables can still be 
modified by the Makefile.

Which of these 2 solutions is the preferred solution?

Is there another (more correct?) solution that would allow to change the 
CFLAGS variable contained in $(TARGET_CONFIGURE_OPTS)?

Thanks for your help.

JC

>
>> +endef
>> +
>> +define BCACHE_TOOLS_INSTALL_TARGET_CMDS
>> +	$(INSTALL) -m 0755 -d $(TARGET_DIR)/usr/share/man/man8
>> +	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
> Ditto:
>      $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) \
>          DESTDIR=$(TARGET_DIR) install
>
> Regards,
> Yann E. MORIN.
>
>> +endef
>> +
>> +$(eval $(generic-package))
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
  2014-08-04 15:28   ` Jean-Christophe DUBOIS
@ 2014-08-04 20:49     ` Yann E. MORIN
  0 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2014-08-04 20:49 UTC (permalink / raw)
  To: buildroot

Jean-Christophe, All,

On 2014-08-04 17:28 +0200, Jean-Christophe DUBOIS spake thusly:
> Le 08/03/2014 08:53 PM, Yann E. MORIN a ?crit :
> >On 2014-07-31 22:40 +0200, Jean-Christophe DUBOIS spake thusly:
> >>diff --git a/package/bcache-tools/Config.in b/package/bcache-tools/Config.in
> >>new file mode 100644
> >>index 0000000..fbe4158
> >>--- /dev/null
> >>+++ b/package/bcache-tools/Config.in
> >>@@ -0,0 +1,11 @@
> >>+config BR2_PACKAGE_BCACHE_TOOLS
> >>+	bool "bcache tools"
> >>+	depends on BR2_PACKAGE_HAS_UDEV
> >>+	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
> >>+	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
> >Since you select sub-options of util-linux, you must also select the
> >package itself:
> >     select BR2_PACKAGE_UTIL_LINUX
> >
> >But you must also inherit its dependencies:
> >     depends on BR2_LARGEFILE # util-linux
> >     depends on BR2_USE_WCHAR # util-linux
> >
> >But you must also inherit from the sub-options' dependencies, too:
> >     depends on BR2_USE_MMU # util-linux (libblkid)

> Sorry I am new to buildroot, but wouldn't be expected that selecting LIBUUID
> and LIBBLKID would automatically select the LIBUUID and LIBBLKID
> dependencies and therefore it does not have to be explicitly selected by all
> packages depending on these features ?

No. That's a limitation of the kconfig language. 'select' does not
propagate the 'depends on'. See:
    http://buildroot.net/downloads/manual/manual.html#_literal_config_in_literal_file

    ---8<---
    Note. The current problem with the kconfig language is that these
    two dependency semantics are not internally linked. Therefore, it
    may be possible to select a package, whom one of its dependencies/
    requirement is not met.
    ---8<---

> If one day the util-linux dependency is changing, will it be expected to
> change the dependency of all package using linux-util?

Yes, but that would be the responsibility to the user bumping util-linux
to propagate the new dependencies or remove the old ones.

> >>+define BCACHE_TOOLS_FIX_PKGCONFIG
> >>+	$(SED) 's^`pkg-config^`$(PKG_CONFIG_HOST_BINARY)^' \
> >>+		$(BCACHE_TOOLS_DIR)/Makefile
> >>+endef
> >This should probably be a post-extract hook. See:
> >     http://buildroot.net/downloads/manual/manual.html#hooks

> I'll look into this. I copied this from the dnsmasq package.

Hmmm... Indeed. But that part was added 2.5 years ago, and we have
better infrastructure now. dnsmasq would be in need of a bit of love, it
looks like! ;-)

> >>+define BCACHE_TOOLS_BUILD_CMDS
> >>+	$(BCACHE_TOOLS_FIX_PKGCONFIG)
> >>+	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D)
> >This should probably be:
> >     $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS)
> >
> >TARGET_CONFIGURE_OPTS contains the CC= assignment.
> >
> >Thus, the BCACHE_TOOLS_MAKE_ENV no longer makes sense, so you can remove
> >it.
> OK, this is also coming from dnsmasq package.

OK, I saw you further replied on this matter. I'll read your other mail
and reply there.

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] 10+ messages in thread
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
  2014-08-04 19:53   ` Jean-Christophe DUBOIS
@ 2014-08-04 21:00     ` Yann E. MORIN
  2014-08-05  6:40       ` Jean-Christophe DUBOIS
  0 siblings, 1 reply; 10+ messages in thread
From: Yann E. MORIN @ 2014-08-04 21:00 UTC (permalink / raw)
  To: buildroot

Jean-Christophe, All,

On 2014-08-04 21:53 +0200, Jean-Christophe DUBOIS spake thusly:
> Le 08/03/2014 08:53 PM, Yann E. MORIN a ?crit :
> >Jean-Christophe, All,
> >
> >On 2014-07-31 22:40 +0200, Jean-Christophe DUBOIS spake thusly:
> >>+define BCACHE_TOOLS_BUILD_CMDS
> >>+	$(BCACHE_TOOLS_FIX_PKGCONFIG)
> >>+	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D)
> >This should probably be:
> >     $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS)
> >
> >TARGET_CONFIGURE_OPTS contains the CC= assignment.
> >
> >Thus, the BCACHE_TOOLS_MAKE_ENV no longer makes sense, so you can remove
> >it.
> 
> When I do as you propose then various compilation variables
> $(TARGET_CONFIGURE_OPTS) are passed as $(MAKE) arguments.
> 
> Therefore these variables cannot be overridden/changed in the Makefile
> (which might the the desired behavior) unless explicitly allowed with the
> "override" directive.
> 
> Unfortunately the bcache-tools Makefile needs (for now) to be able to
> override the CFLAGS variable.
> 
> So I can certainly "patch" the bcache-tools Makefile (in the post extract
> hook for example) to allow "overriding" on the CFLAGS variable.
> 
> Another solution is to pass $(TARGET_CONFIGURE_OPTS) as environment
> variables to $(MAKE) in which case the various variables can still be
> modified by the Makefile.
> 
> Which of these 2 solutions is the preferred solution?

Hmmm. I'd say to pass them in the env (your second solution.) But I
can't vouch for the other developpers. Thomas P.?

Oh, BTW, I forgot one more point in my earlier review: please add the
URL to the homepage for the package:
    http://bcache.evilpiepirate.org/

And as requested by Thomas DS, change the help text to just replicate
the headlines of the homepage:

    help 
      Bcache is a Linux kernel block layer cache. It allows one or more
      fast disk drives such as flash-based solid state drives (SSDs) to
      act as a cache for one or more slower hard disk drives.

      http://bcache.evilpiepirate.org/

> Is there another (more correct?) solution that would allow to change the
> CFLAGS variable contained in $(TARGET_CONFIGURE_OPTS)?

Make it a proper autotools package and submit it upstream?
Just kidding ;-)

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] 10+ messages in thread
* [Buildroot] [PATCH v2] bcache-tools: Adding package.
  2014-08-04 21:00     ` Yann E. MORIN
@ 2014-08-05  6:40       ` Jean-Christophe DUBOIS
  0 siblings, 0 replies; 10+ messages in thread
From: Jean-Christophe DUBOIS @ 2014-08-05  6:40 UTC (permalink / raw)
  To: buildroot

Le 08/04/2014 11:00 PM, Yann E. MORIN a ?crit :
> Jean-Christophe, All,
>
> On 2014-08-04 21:53 +0200, Jean-Christophe DUBOIS spake thusly:
>> Le 08/03/2014 08:53 PM, Yann E. MORIN a ?crit :
>>> Jean-Christophe, All,
>>>
>>> On 2014-07-31 22:40 +0200, Jean-Christophe DUBOIS spake thusly:
>>>> +define BCACHE_TOOLS_BUILD_CMDS
>>>> +	$(BCACHE_TOOLS_FIX_PKGCONFIG)
>>>> +	$(BCACHE_TOOLS_MAKE_ENV) $(MAKE) -C $(@D)
>>> This should probably be:
>>>      $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS)
>>>
>>> TARGET_CONFIGURE_OPTS contains the CC= assignment.
>>>
>>> Thus, the BCACHE_TOOLS_MAKE_ENV no longer makes sense, so you can remove
>>> it.
>> When I do as you propose then various compilation variables
>> $(TARGET_CONFIGURE_OPTS) are passed as $(MAKE) arguments.
>>
>> Therefore these variables cannot be overridden/changed in the Makefile
>> (which might the the desired behavior) unless explicitly allowed with the
>> "override" directive.
>>
>> Unfortunately the bcache-tools Makefile needs (for now) to be able to
>> override the CFLAGS variable.
>>
>> So I can certainly "patch" the bcache-tools Makefile (in the post extract
>> hook for example) to allow "overriding" on the CFLAGS variable.
>>
>> Another solution is to pass $(TARGET_CONFIGURE_OPTS) as environment
>> variables to $(MAKE) in which case the various variables can still be
>> modified by the Makefile.
>>
>> Which of these 2 solutions is the preferred solution?
> Hmmm. I'd say to pass them in the env (your second solution.) But I
> can't vouch for the other developpers. Thomas P.?
OK, I'll use env variables.
>
> Oh, BTW, I forgot one more point in my earlier review: please add the
> URL to the homepage for the package:
>      http://bcache.evilpiepirate.org/
OK, I'll do.
>
> And as requested by Thomas DS, change the help text to just replicate
> the headlines of the homepage:
>
>      help
>        Bcache is a Linux kernel block layer cache. It allows one or more
>        fast disk drives such as flash-based solid state drives (SSDs) to
>        act as a cache for one or more slower hard disk drives.
>
>        http://bcache.evilpiepirate.org/
OK, I'll do.
>
>> Is there another (more correct?) solution that would allow to change the
>> CFLAGS variable contained in $(TARGET_CONFIGURE_OPTS)?
> Make it a proper autotools package and submit it upstream?
> Just kidding ;-)
I was expecting there was a way to alter the CFLAGS variable in 
$(TARGET_CONFIGURE_OPTS) on a package base.

For now, I'll go with the env variable solution.
>
> Regards,
> Yann E. MORIN.
>

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-08-05  6:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-31 20:40 [Buildroot] [PATCH v2] bcache-tools: Adding package Jean-Christophe DUBOIS
2014-08-02 20:55 ` Jean-Christophe DUBOIS
2014-08-03  8:39   ` Yann E. MORIN
2014-08-03 18:53 ` Yann E. MORIN
2014-08-04 15:28   ` Jean-Christophe DUBOIS
2014-08-04 20:49     ` Yann E. MORIN
2014-08-04 19:53   ` Jean-Christophe DUBOIS
2014-08-04 21:00     ` Yann E. MORIN
2014-08-05  6:40       ` Jean-Christophe DUBOIS
2014-08-04  4:51 ` Thomas De Schampheleire

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.