All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/3] mender-artifact support
@ 2018-08-26 21:41 Mirza Krak
  2018-08-26 21:41 ` [Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target Mirza Krak
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Mirza Krak @ 2018-08-26 21:41 UTC (permalink / raw)
  To: buildroot

First patch adds "host" support in pkg-golang.mk.

Second patch adds a new package, mender-artifact. This is a host tool
to generate update images in Mender Artifact format compatible with
Mender client and the Mender server. More information about the Mender
artifact format can be found here:

    https://github.com/mendersoftware/mender-artifact/blob/master/Documentation/artifact-format.md

Third patch adds the mender-artifact package to Mirza Krak in DEVELOPERS.

Patch 2-3 depend on:

    package/pkg-golang: add support for host target

Mirza Krak (3):
  package/pkg-golang: add support for host target
  package/mender-artifact: initial support
  DEVELOPERS: add mender-artifact to Mirza Krak

 DEVELOPERS                                   |  1 +
 package/Config.in.host                       |  1 +
 package/mender-artifact/Config.in.host       |  7 +++++
 package/mender-artifact/mender-artifact.hash | 29 +++++++++++++++++++++
 package/mender-artifact/mender-artifact.mk   | 21 +++++++++++++++
 package/pkg-golang.mk                        | 38 ++++++++++++++++++++++++----
 6 files changed, 92 insertions(+), 5 deletions(-)
 create mode 100644 package/mender-artifact/Config.in.host
 create mode 100644 package/mender-artifact/mender-artifact.hash
 create mode 100644 package/mender-artifact/mender-artifact.mk

--
2.11.0

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target
  2018-08-26 21:41 [Buildroot] [PATCH 0/3] mender-artifact support Mirza Krak
@ 2018-08-26 21:41 ` Mirza Krak
  2018-08-27  9:02   ` Thomas Petazzoni
  2018-08-26 21:41 ` [Buildroot] [PATCH 2/3] package/mender-artifact: initial support Mirza Krak
  2018-08-26 21:41 ` [Buildroot] [PATCH 3/3] DEVELOPERS: add mender-artifact to Mirza Krak Mirza Krak
  2 siblings, 1 reply; 11+ messages in thread
From: Mirza Krak @ 2018-08-26 21:41 UTC (permalink / raw)
  To: buildroot

With this you can add:

    $(eval $(host-golang-package))

to a package .mk file to build for host.

Signed-off-by: Mirza Krak <mirza.krak@northern.tech>
---
 package/pkg-golang.mk | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
index 6eacd14180..7d9956378f 100644
--- a/package/pkg-golang.mk
+++ b/package/pkg-golang.mk
@@ -23,10 +23,13 @@
 
 GO_BIN = $(HOST_DIR)/bin/go
 
-# We pass an empty GOBIN, otherwise "go install: cannot install
-# cross-compiled binaries when GOBIN is set"
 GO_TARGET_ENV = \
 	$(HOST_GO_TARGET_ENV) \
+	$(GO_HOST_ENV)
+
+# We pass an empty GOBIN, otherwise "go install: cannot install
+# cross-compiled binaries when GOBIN is set"
+GO_HOST_ENV = \
 	PATH=$(BR_PATH) \
 	GOBIN= \
 	CGO_ENABLED=$(HOST_GO_CGO_ENABLED)
@@ -44,7 +47,6 @@ GO_TARGET_ENV = \
 #             packages
 #  argument 4 is the type (target or host)
 #
-# NOTE Only type target is supported at the moment
 ################################################################################
 
 define inner-golang-package
@@ -96,6 +98,9 @@ endif
 # Build step. Only define it if not already defined by the package .mk
 # file.
 ifndef $(2)_BUILD_CMDS
+ifeq ($(4),target)
+
+# Build package for target
 define $(2)_BUILD_CMDS
 	$$(foreach d,$$($(2)_BUILD_TARGETS),\
 		cd $$($(2)_SRC_PATH); \
@@ -107,10 +112,23 @@ define $(2)_BUILD_CMDS
 			./$$(d)
 	)
 endef
+else
+	# Build package for host
+define $(2)_BUILD_CMDS
+	$$(foreach d,$$($(2)_BUILD_TARGETS),\
+		cd $$($(2)_SRC_PATH); \
+		$$(GO_HOST_ENV) \
+			GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
+			$$($(2)_GO_ENV) \
+			$$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
+			-o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
+			./$$(d)
+	)
+endef
+endif
 endif
 
-# Target installation step. Only define it if not already defined by the
-# package .mk file.
+# Target installation step
 ifndef $(2)_INSTALL_TARGET_CMDS
 define $(2)_INSTALL_TARGET_CMDS
 	$$(foreach d,$$($(2)_INSTALL_BINS),\
@@ -119,6 +137,15 @@ define $(2)_INSTALL_TARGET_CMDS
 endef
 endif
 
+# Host installation step
+ifndef $(2)_INSTALL_CMDS
+define $(2)_INSTALL_CMDS
+	$$(foreach d,$$($(2)_INSTALL_BINS),\
+		$(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $(HOST_DIR)/usr/bin/$$(d)
+	)
+endef
+endif
+
 # Call the generic package infrastructure to generate the necessary make
 # targets
 $(call inner-generic-package,$(1),$(2),$(3),$(4))
@@ -130,3 +157,4 @@ endef # inner-golang-package
 ################################################################################
 
 golang-package = $(call inner-golang-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
+host-golang-package = $(call inner-golang-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 2/3] package/mender-artifact: initial support
  2018-08-26 21:41 [Buildroot] [PATCH 0/3] mender-artifact support Mirza Krak
  2018-08-26 21:41 ` [Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target Mirza Krak
@ 2018-08-26 21:41 ` Mirza Krak
  2018-08-27  9:14   ` Thomas Petazzoni
  2018-08-26 21:41 ` [Buildroot] [PATCH 3/3] DEVELOPERS: add mender-artifact to Mirza Krak Mirza Krak
  2 siblings, 1 reply; 11+ messages in thread
From: Mirza Krak @ 2018-08-26 21:41 UTC (permalink / raw)
  To: buildroot

mender-artifact is a host tool to generate update images
in the Mender artifact file format.

See https://github.com/mendersoftware/mender-artifact for more
information about the format and the tool.

Signed-off-by: Mirza Krak <mirza.krak@northern.tech>
---
 package/Config.in.host                       |  1 +
 package/mender-artifact/Config.in.host       |  7 +++++++
 package/mender-artifact/mender-artifact.hash | 29 ++++++++++++++++++++++++++++
 package/mender-artifact/mender-artifact.mk   | 21 ++++++++++++++++++++
 4 files changed, 58 insertions(+)
 create mode 100644 package/mender-artifact/Config.in.host
 create mode 100644 package/mender-artifact/mender-artifact.hash
 create mode 100644 package/mender-artifact/mender-artifact.mk

diff --git a/package/Config.in.host b/package/Config.in.host
index 7838ffc219..2b85d2b189 100644
--- a/package/Config.in.host
+++ b/package/Config.in.host
@@ -30,6 +30,7 @@ menu "Host utilities"
 	source "package/jsmin/Config.in.host"
 	source "package/lpc3250loader/Config.in.host"
 	source "package/lttng-babeltrace/Config.in.host"
+	source "package/mender-artifact/Config.in.host"
 	source "package/mfgtools/Config.in.host"
 	source "package/mkpasswd/Config.in.host"
 	source "package/mtd/Config.in.host"
diff --git a/package/mender-artifact/Config.in.host b/package/mender-artifact/Config.in.host
new file mode 100644
index 0000000000..bb03e96018
--- /dev/null
+++ b/package/mender-artifact/Config.in.host
@@ -0,0 +1,7 @@
+config BR2_PACKAGE_HOST_MENDER_ARTIFACT
+    depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
+    depends on BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS
+    depends on BR2_TOOLCHAIN_HAS_THREADS
+    bool "host mender-artifact"
+    help
+      Mender image artifact tool
diff --git a/package/mender-artifact/mender-artifact.hash b/package/mender-artifact/mender-artifact.hash
new file mode 100644
index 0000000000..af003fe7de
--- /dev/null
+++ b/package/mender-artifact/mender-artifact.hash
@@ -0,0 +1,29 @@
+# Locally computed:
+sha256 2a0322d8707c8ea7cada12c8f96144382264c898cadc35e8058bb9ea6bf8b041 mender-artifact-2.2.0.tar.gz
+
+# License, locally computed
+
+
+# Vendor licenses
+# Generated with sed '/^[A-Za-z0-9_]/s/^/sha256  /' LIC_FILES_CHKSUM.sha256
+
+# Apache-2.0 license.
+sha256 b40930bbcf80744c86c46a12bc9da056641d722716c378f5659b9e555ef833e1 LICENSE
+sha256 ceb1b36ff073bd13d9806d4615b931707768ca9023805620acc32dd1cfc2f680 vendor/github.com/mendersoftware/mendertesting/LICENSE
+
+# BSD 2 Clause licenses.
+sha256 8d427fd87bc9579ea368fde3d49f9ca22eac857f91a9dec7e3004bdfab7dee86 vendor/github.com/pkg/errors/LICENSE
+
+# BSD 3 Clause licenses.
+sha256 2eb550be6801c1ea434feba53bf6d12e7c71c90253e0a9de4a4f46cf88b56477 vendor/github.com/pmezard/go-difflib/LICENSE
+
+# ISC licenses.
+sha256 3525392c6db3b804af76980b2c560ee9ec1abdadd907d76a26091df7c78f3a25 vendor/github.com/davecgh/go-spew/LICENSE
+
+# MIT licenses.
+sha256 402f39eed8a1851385d0703999aa9f23d067c2ea3e15c63c074e389cbf8f8f8f vendor/github.com/stretchr/testify/LICENSE
+sha256 402f39eed8a1851385d0703999aa9f23d067c2ea3e15c63c074e389cbf8f8f8f vendor/github.com/stretchr/testify/LICENCE.txt
+sha256 da277af11b85227490377fbcac6afccc68be560c4fff36ac05ca62de55345fd7 vendor/github.com/urfave/cli/LICENSE
+
+# sha256 of all the vendor licenses combined
+sha256 a9d3dea05c34f2bf7eb3fa677dafe09e9a80edd024cb31da750616c10f2bbcbb LIC_FILES_CHKSUM.sha256
diff --git a/package/mender-artifact/mender-artifact.mk b/package/mender-artifact/mender-artifact.mk
new file mode 100644
index 0000000000..590895de8c
--- /dev/null
+++ b/package/mender-artifact/mender-artifact.mk
@@ -0,0 +1,21 @@
+################################################################################
+#
+# mender-artifact
+#
+################################################################################
+
+MENDER_ARTIFACT_VERSION = 2.2.0
+MENDER_ARTIFACT_SITE = $(call github,mendersoftware,mender-artifact,$(MENDER_ARTIFACT_VERSION))
+MENDER_ARTIFACT_LICENSE = Apache-2.0, BSD-2-Clause, BSD-3-Clause, ISC, MIT
+MENDER_ARTIFACT_LICENSE_FILES = \
+	LICENSE \
+	LIC_FILES_CHKSUM.sha256 \
+	vendor/github.com/mendersoftware/mendertesting/LICENSE \
+	vendor/github.com/pkg/errors/LICENSE \
+	vendor/github.com/pmezard/go-difflib/LICENSE \
+	vendor/github.com/davecgh/go-spew/LICENSE \
+	vendor/github.com/stretchr/testify/LICENSE \
+	vendor/github.com/stretchr/testify/LICENCE.txt \
+	vendor/github.com/urfave/cli/LICENSE
+
+$(eval $(host-golang-package))
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 3/3] DEVELOPERS: add mender-artifact to Mirza Krak
  2018-08-26 21:41 [Buildroot] [PATCH 0/3] mender-artifact support Mirza Krak
  2018-08-26 21:41 ` [Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target Mirza Krak
  2018-08-26 21:41 ` [Buildroot] [PATCH 2/3] package/mender-artifact: initial support Mirza Krak
@ 2018-08-26 21:41 ` Mirza Krak
  2018-08-27  9:14   ` Thomas Petazzoni
  2 siblings, 1 reply; 11+ messages in thread
From: Mirza Krak @ 2018-08-26 21:41 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Mirza Krak <mirza.krak@northern.tech>
---
 DEVELOPERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/DEVELOPERS b/DEVELOPERS
index a6d76dd9c4..b69a75fdfd 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -1439,6 +1439,7 @@ F:	package/shadowsocks-libev/
 
 N:	Mirza Krak <mirza.krak@northern.tech>
 F:	package/mender/
+F:  package/mender-artifact/
 
 N:	Morgan Delestre <m.delestre@sinters.fr>
 F:	package/monkey/
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target
  2018-08-26 21:41 ` [Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target Mirza Krak
@ 2018-08-27  9:02   ` Thomas Petazzoni
  2018-09-05 13:27     ` Angelo Compagnucci
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2018-08-27  9:02 UTC (permalink / raw)
  To: buildroot

Hello Mirza,

On Sun, 26 Aug 2018 23:41:44 +0200, Mirza Krak wrote:
> With this you can add:
> 
>     $(eval $(host-golang-package))
> 
> to a package .mk file to build for host.
> 
> Signed-off-by: Mirza Krak <mirza.krak@northern.tech>

Thanks for this contribution! I believe the title should be improved,
because "host target" is a bit confusion. Perhaps:

package/pkg-golang: add support for building host packages

This commit also needs a small update to the Buildroot manual, to add
something like "The ability to build host packages is also available,
with the host-golang-package macro."

> ---
>  package/pkg-golang.mk | 38 +++++++++++++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
> index 6eacd14180..7d9956378f 100644
> --- a/package/pkg-golang.mk
> +++ b/package/pkg-golang.mk
> @@ -23,10 +23,13 @@
>  
>  GO_BIN = $(HOST_DIR)/bin/go
>  
> -# We pass an empty GOBIN, otherwise "go install: cannot install
> -# cross-compiled binaries when GOBIN is set"
>  GO_TARGET_ENV = \
>  	$(HOST_GO_TARGET_ENV) \
> +	$(GO_HOST_ENV)

I think it's a bit weird and dangerous to re-use GO_HOST_ENV in
GO_TARGET_ENV. Indeed, there is the risk that we add something
host-specific, and it will be re-used for both target and host.

So either your duplicate the definitions (which is reasonable if they
aren't a lot of them), or you add something like GO_COMMON_ENV, re-used
between target and host.

>  define inner-golang-package
> @@ -96,6 +98,9 @@ endif
>  # Build step. Only define it if not already defined by the package .mk
>  # file.
>  ifndef $(2)_BUILD_CMDS
> +ifeq ($(4),target)
> +

Drop this empty line.

> +# Build package for target
>  define $(2)_BUILD_CMDS
>  	$$(foreach d,$$($(2)_BUILD_TARGETS),\
>  		cd $$($(2)_SRC_PATH); \
> @@ -107,10 +112,23 @@ define $(2)_BUILD_CMDS
>  			./$$(d)
>  	)
>  endef
> +else
> +	# Build package for host

Don't indent this comment.

> +define $(2)_BUILD_CMDS
> +	$$(foreach d,$$($(2)_BUILD_TARGETS),\
> +		cd $$($(2)_SRC_PATH); \
> +		$$(GO_HOST_ENV) \
> +			GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
> +			$$($(2)_GO_ENV) \
> +			$$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
> +			-o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
> +			./$$(d)
> +	)
> +endef
> +endif
>  endif
>  
> -# Target installation step. Only define it if not already defined by the
> -# package .mk file.

Why are you changing this comment ?

> +# Target installation step
>  ifndef $(2)_INSTALL_TARGET_CMDS
>  define $(2)_INSTALL_TARGET_CMDS
>  	$$(foreach d,$$($(2)_INSTALL_BINS),\
> @@ -119,6 +137,15 @@ define $(2)_INSTALL_TARGET_CMDS
>  endef
>  endif
>  
> +# Host installation step
> +ifndef $(2)_INSTALL_CMDS
> +define $(2)_INSTALL_CMDS
> +	$$(foreach d,$$($(2)_INSTALL_BINS),\
> +		$(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $(HOST_DIR)/usr/bin/$$(d)


Install in $(HOST_DIR)/bin, not $(HOST_DIR)/usr/bin. We no longer use
$(HOST_DIR)/usr/bin in fact, and $(HOST_DIR)/usr is a symlink to
$(HOST_DIR).

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 2/3] package/mender-artifact: initial support
  2018-08-26 21:41 ` [Buildroot] [PATCH 2/3] package/mender-artifact: initial support Mirza Krak
@ 2018-08-27  9:14   ` Thomas Petazzoni
  2018-08-27  9:28     ` Mirza Krak
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2018-08-27  9:14 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 26 Aug 2018 23:41:45 +0200, Mirza Krak wrote:
> mender-artifact is a host tool to generate update images
> in the Mender artifact file format.
> 
> See https://github.com/mendersoftware/mender-artifact for more
> information about the format and the tool.
> 
> Signed-off-by: Mirza Krak <mirza.krak@northern.tech>

Great to see this being packaged!

> diff --git a/package/mender-artifact/Config.in.host b/package/mender-artifact/Config.in.host
> new file mode 100644
> index 0000000000..bb03e96018
> --- /dev/null
> +++ b/package/mender-artifact/Config.in.host
> @@ -0,0 +1,7 @@
> +config BR2_PACKAGE_HOST_MENDER_ARTIFACT
> +    depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
> +    depends on BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS
> +    depends on BR2_TOOLCHAIN_HAS_THREADS
> +    bool "host mender-artifact"
> +    help
> +      Mender image artifact tool

Please run ./utils/check-package on this new package. You will notice
that the indentation is not good, and that the "depends on" lines
should go after the "bool" line.

Please add an upstream URL in the Config.in help text (see other
packages).

> diff --git a/package/mender-artifact/mender-artifact.hash b/package/mender-artifact/mender-artifact.hash
> new file mode 100644
> index 0000000000..af003fe7de
> --- /dev/null
> +++ b/package/mender-artifact/mender-artifact.hash
> @@ -0,0 +1,29 @@
> +# Locally computed:
> +sha256 2a0322d8707c8ea7cada12c8f96144382264c898cadc35e8058bb9ea6bf8b041 mender-artifact-2.2.0.tar.gz
> +
> +# License, locally computed
> +

This comment feels a bit lonely, no ? Is it here to say that among the
below license hashes, the hash of "LICENSE" was locally computed, while
the hashes for the licenses in vendor/ were extracted from
LIC_FILES_CHKSUM.sha256 ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 3/3] DEVELOPERS: add mender-artifact to Mirza Krak
  2018-08-26 21:41 ` [Buildroot] [PATCH 3/3] DEVELOPERS: add mender-artifact to Mirza Krak Mirza Krak
@ 2018-08-27  9:14   ` Thomas Petazzoni
  2018-08-27  9:25     ` Mirza Krak
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2018-08-27  9:14 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 26 Aug 2018 23:41:46 +0200, Mirza Krak wrote:
> Signed-off-by: Mirza Krak <mirza.krak@northern.tech>

This should be merged into PATCH 2/3.

> ---
>  DEVELOPERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/DEVELOPERS b/DEVELOPERS
> index a6d76dd9c4..b69a75fdfd 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -1439,6 +1439,7 @@ F:	package/shadowsocks-libev/
>  
>  N:	Mirza Krak <mirza.krak@northern.tech>
>  F:	package/mender/
> +F:  package/mender-artifact/

Please respect the indentation of the file.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 3/3] DEVELOPERS: add mender-artifact to Mirza Krak
  2018-08-27  9:14   ` Thomas Petazzoni
@ 2018-08-27  9:25     ` Mirza Krak
  0 siblings, 0 replies; 11+ messages in thread
From: Mirza Krak @ 2018-08-27  9:25 UTC (permalink / raw)
  To: buildroot

On Mon, Aug 27, 2018 at 11:14 AM, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> Hello,
>
> On Sun, 26 Aug 2018 23:41:46 +0200, Mirza Krak wrote:
>> Signed-off-by: Mirza Krak <mirza.krak@northern.tech>
>
> This should be merged into PATCH 2/3.

Will do. Thanks!

>> ---
>>  DEVELOPERS | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/DEVELOPERS b/DEVELOPERS
>> index a6d76dd9c4..b69a75fdfd 100644
>> --- a/DEVELOPERS
>> +++ b/DEVELOPERS
>> @@ -1439,6 +1439,7 @@ F:      package/shadowsocks-libev/
>>
>>  N:   Mirza Krak <mirza.krak@northern.tech>
>>  F:   package/mender/
>> +F:  package/mender-artifact/
>
> Please respect the indentation of the file.

Doh! Will fix. Thanks!.

-- 
Mirza Krak | Embedded Solutions Architect | https://mender.io

 Northern.tech AS | @northerntechHQ

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 2/3] package/mender-artifact: initial support
  2018-08-27  9:14   ` Thomas Petazzoni
@ 2018-08-27  9:28     ` Mirza Krak
  0 siblings, 0 replies; 11+ messages in thread
From: Mirza Krak @ 2018-08-27  9:28 UTC (permalink / raw)
  To: buildroot

On Mon, Aug 27, 2018 at 11:14 AM, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> Hello,
>
> On Sun, 26 Aug 2018 23:41:45 +0200, Mirza Krak wrote:
>> mender-artifact is a host tool to generate update images
>> in the Mender artifact file format.
>>
>> See https://github.com/mendersoftware/mender-artifact for more
>> information about the format and the tool.
>>
>> Signed-off-by: Mirza Krak <mirza.krak@northern.tech>
>
> Great to see this being packaged!
>
>> diff --git a/package/mender-artifact/Config.in.host b/package/mender-artifact/Config.in.host
>> new file mode 100644
>> index 0000000000..bb03e96018
>> --- /dev/null
>> +++ b/package/mender-artifact/Config.in.host
>> @@ -0,0 +1,7 @@
>> +config BR2_PACKAGE_HOST_MENDER_ARTIFACT
>> +    depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
>> +    depends on BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS
>> +    depends on BR2_TOOLCHAIN_HAS_THREADS
>> +    bool "host mender-artifact"
>> +    help
>> +      Mender image artifact tool
>
> Please run ./utils/check-package on this new package. You will notice
> that the indentation is not good, and that the "depends on" lines
> should go after the "bool" line.
>
> Please add an upstream URL in the Config.in help text (see other
> packages).

Will do. Thanks.

>
>> diff --git a/package/mender-artifact/mender-artifact.hash b/package/mender-artifact/mender-artifact.hash
>> new file mode 100644
>> index 0000000000..af003fe7de
>> --- /dev/null
>> +++ b/package/mender-artifact/mender-artifact.hash
>> @@ -0,0 +1,29 @@
>> +# Locally computed:
>> +sha256 2a0322d8707c8ea7cada12c8f96144382264c898cadc35e8058bb9ea6bf8b041 mender-artifact-2.2.0.tar.gz
>> +
>> +# License, locally computed
>> +
>
> This comment feels a bit lonely, no ? Is it here to say that among the
> below license hashes, the hash of "LICENSE" was locally computed, while
> the hashes for the licenses in vendor/ were extracted from
> LIC_FILES_CHKSUM.sha256 ?

Actually the "LICENSE" sha256 can also be found in
LIC_FILES_CHKSUM.sha256. So no license sums are locally computed. Will
update comment to better reflect this.

Thanks.

-- 
Mirza Krak | Embedded Solutions Architect | https://mender.io

 Northern.tech AS | @northerntechHQ

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target
  2018-08-27  9:02   ` Thomas Petazzoni
@ 2018-09-05 13:27     ` Angelo Compagnucci
  2018-09-05 13:30       ` Angelo Compagnucci
  0 siblings, 1 reply; 11+ messages in thread
From: Angelo Compagnucci @ 2018-09-05 13:27 UTC (permalink / raw)
  To: buildroot

Hi Mirza,

On Mon, Aug 27, 2018 at 11:02 AM, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> Hello Mirza,
>
> On Sun, 26 Aug 2018 23:41:44 +0200, Mirza Krak wrote:
>> With this you can add:
>>
>>     $(eval $(host-golang-package))
>>
>> to a package .mk file to build for host.
>>
>> Signed-off-by: Mirza Krak <mirza.krak@northern.tech>
>
> Thanks for this contribution! I believe the title should be improved,
> because "host target" is a bit confusion. Perhaps:
>
> package/pkg-golang: add support for building host packages
>
> This commit also needs a small update to the Buildroot manual, to add
> something like "The ability to build host packages is also available,
> with the host-golang-package macro."

Could you repost this patch? I found it very useful and I think you
could post it sooner than the rest of the series.

Thanks!

>
>> ---
>>  package/pkg-golang.mk | 38 +++++++++++++++++++++++++++++++++-----
>>  1 file changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
>> index 6eacd14180..7d9956378f 100644
>> --- a/package/pkg-golang.mk
>> +++ b/package/pkg-golang.mk
>> @@ -23,10 +23,13 @@
>>
>>  GO_BIN = $(HOST_DIR)/bin/go
>>
>> -# We pass an empty GOBIN, otherwise "go install: cannot install
>> -# cross-compiled binaries when GOBIN is set"
>>  GO_TARGET_ENV = \
>>       $(HOST_GO_TARGET_ENV) \
>> +     $(GO_HOST_ENV)
>
> I think it's a bit weird and dangerous to re-use GO_HOST_ENV in
> GO_TARGET_ENV. Indeed, there is the risk that we add something
> host-specific, and it will be re-used for both target and host.
>
> So either your duplicate the definitions (which is reasonable if they
> aren't a lot of them), or you add something like GO_COMMON_ENV, re-used
> between target and host.
>
>>  define inner-golang-package
>> @@ -96,6 +98,9 @@ endif
>>  # Build step. Only define it if not already defined by the package .mk
>>  # file.
>>  ifndef $(2)_BUILD_CMDS
>> +ifeq ($(4),target)
>> +
>
> Drop this empty line.
>
>> +# Build package for target
>>  define $(2)_BUILD_CMDS
>>       $$(foreach d,$$($(2)_BUILD_TARGETS),\
>>               cd $$($(2)_SRC_PATH); \
>> @@ -107,10 +112,23 @@ define $(2)_BUILD_CMDS
>>                       ./$$(d)
>>       )
>>  endef
>> +else
>> +     # Build package for host
>
> Don't indent this comment.
>
>> +define $(2)_BUILD_CMDS
>> +     $$(foreach d,$$($(2)_BUILD_TARGETS),\
>> +             cd $$($(2)_SRC_PATH); \
>> +             $$(GO_HOST_ENV) \
>> +                     GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
>> +                     $$($(2)_GO_ENV) \
>> +                     $$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
>> +                     -o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
>> +                     ./$$(d)
>> +     )
>> +endef
>> +endif
>>  endif
>>
>> -# Target installation step. Only define it if not already defined by the
>> -# package .mk file.
>
> Why are you changing this comment ?
>
>> +# Target installation step
>>  ifndef $(2)_INSTALL_TARGET_CMDS
>>  define $(2)_INSTALL_TARGET_CMDS
>>       $$(foreach d,$$($(2)_INSTALL_BINS),\
>> @@ -119,6 +137,15 @@ define $(2)_INSTALL_TARGET_CMDS
>>  endef
>>  endif
>>
>> +# Host installation step
>> +ifndef $(2)_INSTALL_CMDS
>> +define $(2)_INSTALL_CMDS
>> +     $$(foreach d,$$($(2)_INSTALL_BINS),\
>> +             $(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $(HOST_DIR)/usr/bin/$$(d)
>
>
> Install in $(HOST_DIR)/bin, not $(HOST_DIR)/usr/bin. We no longer use
> $(HOST_DIR)/usr/bin in fact, and $(HOST_DIR)/usr is a symlink to
> $(HOST_DIR).
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target
  2018-09-05 13:27     ` Angelo Compagnucci
@ 2018-09-05 13:30       ` Angelo Compagnucci
  0 siblings, 0 replies; 11+ messages in thread
From: Angelo Compagnucci @ 2018-09-05 13:30 UTC (permalink / raw)
  To: buildroot

Hi Mirza,

On Wed, Sep 5, 2018 at 3:27 PM, Angelo Compagnucci
<angelo@amarulasolutions.com> wrote:
> Hi Mirza,
>
> On Mon, Aug 27, 2018 at 11:02 AM, Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
>> Hello Mirza,
>>
>> On Sun, 26 Aug 2018 23:41:44 +0200, Mirza Krak wrote:
>>> With this you can add:
>>>
>>>     $(eval $(host-golang-package))
>>>
>>> to a package .mk file to build for host.
>>>
>>> Signed-off-by: Mirza Krak <mirza.krak@northern.tech>
>>
>> Thanks for this contribution! I believe the title should be improved,
>> because "host target" is a bit confusion. Perhaps:
>>
>> package/pkg-golang: add support for building host packages
>>
>> This commit also needs a small update to the Buildroot manual, to add
>> something like "The ability to build host packages is also available,
>> with the host-golang-package macro."
>
> Could you repost this patch? I found it very useful and I think you
> could post it sooner than the rest of the series.

Just found your patch repost, sorry for the noise!

Thanks!

>
> Thanks!
>
>>
>>> ---
>>>  package/pkg-golang.mk | 38 +++++++++++++++++++++++++++++++++-----
>>>  1 file changed, 33 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
>>> index 6eacd14180..7d9956378f 100644
>>> --- a/package/pkg-golang.mk
>>> +++ b/package/pkg-golang.mk
>>> @@ -23,10 +23,13 @@
>>>
>>>  GO_BIN = $(HOST_DIR)/bin/go
>>>
>>> -# We pass an empty GOBIN, otherwise "go install: cannot install
>>> -# cross-compiled binaries when GOBIN is set"
>>>  GO_TARGET_ENV = \
>>>       $(HOST_GO_TARGET_ENV) \
>>> +     $(GO_HOST_ENV)
>>
>> I think it's a bit weird and dangerous to re-use GO_HOST_ENV in
>> GO_TARGET_ENV. Indeed, there is the risk that we add something
>> host-specific, and it will be re-used for both target and host.
>>
>> So either your duplicate the definitions (which is reasonable if they
>> aren't a lot of them), or you add something like GO_COMMON_ENV, re-used
>> between target and host.
>>
>>>  define inner-golang-package
>>> @@ -96,6 +98,9 @@ endif
>>>  # Build step. Only define it if not already defined by the package .mk
>>>  # file.
>>>  ifndef $(2)_BUILD_CMDS
>>> +ifeq ($(4),target)
>>> +
>>
>> Drop this empty line.
>>
>>> +# Build package for target
>>>  define $(2)_BUILD_CMDS
>>>       $$(foreach d,$$($(2)_BUILD_TARGETS),\
>>>               cd $$($(2)_SRC_PATH); \
>>> @@ -107,10 +112,23 @@ define $(2)_BUILD_CMDS
>>>                       ./$$(d)
>>>       )
>>>  endef
>>> +else
>>> +     # Build package for host
>>
>> Don't indent this comment.
>>
>>> +define $(2)_BUILD_CMDS
>>> +     $$(foreach d,$$($(2)_BUILD_TARGETS),\
>>> +             cd $$($(2)_SRC_PATH); \
>>> +             $$(GO_HOST_ENV) \
>>> +                     GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
>>> +                     $$($(2)_GO_ENV) \
>>> +                     $$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
>>> +                     -o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
>>> +                     ./$$(d)
>>> +     )
>>> +endef
>>> +endif
>>>  endif
>>>
>>> -# Target installation step. Only define it if not already defined by the
>>> -# package .mk file.
>>
>> Why are you changing this comment ?
>>
>>> +# Target installation step
>>>  ifndef $(2)_INSTALL_TARGET_CMDS
>>>  define $(2)_INSTALL_TARGET_CMDS
>>>       $$(foreach d,$$($(2)_INSTALL_BINS),\
>>> @@ -119,6 +137,15 @@ define $(2)_INSTALL_TARGET_CMDS
>>>  endef
>>>  endif
>>>
>>> +# Host installation step
>>> +ifndef $(2)_INSTALL_CMDS
>>> +define $(2)_INSTALL_CMDS
>>> +     $$(foreach d,$$($(2)_INSTALL_BINS),\
>>> +             $(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $(HOST_DIR)/usr/bin/$$(d)
>>
>>
>> Install in $(HOST_DIR)/bin, not $(HOST_DIR)/usr/bin. We no longer use
>> $(HOST_DIR)/usr/bin in fact, and $(HOST_DIR)/usr is a symlink to
>> $(HOST_DIR).
>>
>> Thanks!
>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-09-05 13:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-26 21:41 [Buildroot] [PATCH 0/3] mender-artifact support Mirza Krak
2018-08-26 21:41 ` [Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target Mirza Krak
2018-08-27  9:02   ` Thomas Petazzoni
2018-09-05 13:27     ` Angelo Compagnucci
2018-09-05 13:30       ` Angelo Compagnucci
2018-08-26 21:41 ` [Buildroot] [PATCH 2/3] package/mender-artifact: initial support Mirza Krak
2018-08-27  9:14   ` Thomas Petazzoni
2018-08-27  9:28     ` Mirza Krak
2018-08-26 21:41 ` [Buildroot] [PATCH 3/3] DEVELOPERS: add mender-artifact to Mirza Krak Mirza Krak
2018-08-27  9:14   ` Thomas Petazzoni
2018-08-27  9:25     ` Mirza Krak

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.