All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [v2] fs/aci: Add App Container Image (ACI) support to filesystems
@ 2016-05-23 15:07 Brian 'redbeard' Harrington
  2016-05-24 13:14 ` Thomas Petazzoni
  2016-06-07 20:28 ` Thomas Petazzoni
  0 siblings, 2 replies; 3+ messages in thread
From: Brian 'redbeard' Harrington @ 2016-05-23 15:07 UTC (permalink / raw)
  To: buildroot

App Container Images (ACI) are a Linux containerization image format
defined as a part of the AppC specification as defined by CoreOS,
Red Hat, Google, and community members. These images consist of a Linux
userland and a JSON metadata file describing how to execute the actual
runtime.

This filesystem package utilizes Buildroot for the generation of
the userland and provides the user with a mechanism for configuring the
contents of the manifest file.

---
Changes v1 -> v2
  - Remove accidental 'default' of the fs/aci
  - Remove dependency on host Jq
  - Remove numerous "depends" lines in Kconfig, now in if block
  - Set default name of localhost/buildroot as well as error check
  - Allowed user to specify external manifest
  - Enabled setting of timestamp annotation via a (non default) config
    option
  - Set default version via sha256 of .config file
  - Added dependency on specific architectures
  - Moved logic for architecture checks into Kconfig
  - Use qstrip to clean up variables
  - Export variables to scripts in package to avoid numerous
    re-declarations
  - Removed setting of xattrs in tar command
  - Removed accidental re-use of TAR_OPTS
  - Decoupled initial creation of tarball and adding of additional files
    to simplify logic
  - Removed stray debugging "echo" lines used during development
  - Removed extra blank lines
  - Moved logic for generation of json manifest into a shell script

Signed-off-by: Brian 'redbeard' Harrington <redbeard@coreos.com>
---
 fs/Config.in       |   1 +
 fs/aci/Config.in   | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/aci/aci.json.sh |  51 +++++++++++++++++++++++
 fs/aci/aci.mk      |  80 ++++++++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+)
 create mode 100644 fs/aci/Config.in
 create mode 100755 fs/aci/aci.json.sh
 create mode 100644 fs/aci/aci.mk

diff --git a/fs/Config.in b/fs/Config.in
index 51ccf28..1c468c5 100644
--- a/fs/Config.in
+++ b/fs/Config.in
@@ -1,5 +1,6 @@
 menu "Filesystem images"
 
+source "fs/aci/Config.in"
 source "fs/axfs/Config.in"
 source "fs/cloop/Config.in"
 source "fs/cpio/Config.in"
diff --git a/fs/aci/Config.in b/fs/aci/Config.in
new file mode 100644
index 0000000..4df4791
--- /dev/null
+++ b/fs/aci/Config.in
@@ -0,0 +1,117 @@
+config BR2_TARGET_ROOTFS_ACI
+	bool "app container image (aci)"
+	default n
+        depends on BR2_arm || BR2_armeb || BR2_aarch64_eb || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le
+        depends on !BR2_ARM_CPU_ARMV4 
+	help
+	  Build an App Container Image for use with a Linux
+	  containerization engine like rkt, Kurma, 3ofCoins, nosecone,
+	  etc.
+
+if BR2_TARGET_ROOTFS_ACI
+
+config BR2_TARGET_ROOTFS_ACI_ARCH
+	string
+	default "i386"       if BR2_i386
+	default "arm64"      if BR2_x86_64
+	default "aarch64"    if BR2_aarch64
+	default "aarch64_be" if BR2_aarch64_be
+	default "ppc64"      if BR2_powerpc64
+	default "armv6l"     if BR2_arm && BR2_ARM_CPU_ARMV6
+	default "armv7l"     if BR2_arm && BR2_ARM_CPU_ARMV7A
+	default "armv7l"     if BR2_arm && BR2_ARM_CPU_ARMV7M
+	default "armv7b"     if BR2_armeb && BR2_ARM_CPU_ARMV7A
+	default "armv7b"     if BR2_armeb && BR2_ARM_CPU_ARMV7M
+
+choice
+	prompt "compression method"
+	default BR2_TARGET_ROOTFS_ACI_NONE
+	help
+	  Select compressor for application container image.
+
+config BR2_TARGET_ROOTFS_ACI_NONE
+	bool "no compression"
+	help
+	  Do not compress the ACI.
+
+config BR2_TARGET_ROOTFS_ACI_GZIP
+	bool "gzip"
+	help
+	  Do compress the ACI with gzip.
+
+config BR2_TARGET_ROOTFS_ACI_BZIP2
+	bool "bzip2"
+	help
+	  Do compress the ACI with bzip2.
+
+config BR2_TARGET_ROOTFS_ACI_LZMA
+	bool "lzma"
+	help
+	  Do compress the ACI with lzma.
+
+config BR2_TARGET_ROOTFS_ACI_LZO
+	bool "lzo"
+	help
+	  Do compress the ACI with lzop.
+
+config BR2_TARGET_ROOTFS_ACI_XZ
+	bool "xz"
+	help
+	  Do compress the ACI with xz.
+
+endchoice
+
+config BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+        bool "custom aci manifest"
+        help
+          Use custom aci manifest.                                            
+
+if BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+config BR2_TARGET_ROOTFS_ACI_MANIFEST_PATH
+        string "custom path"                                     
+        default ""                                                
+	depends on BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+        help
+          Path to custom ACI manifest.
+endif # BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+
+if !BR2_TARGET_ROOTFS_ACI_OPTIONS_MANIFEST_CUSTOM
+config BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME
+	string "name"
+	default "localhost/buildroot"
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+	help
+	  A human-readable name for this App Container Image (e.g. 
+	  example.com/my_app).
+
+comment "aci images must have a name"
+	depends on BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME = "localhost/buildroot"
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+
+config BR2_TARGET_ROOTFS_ACI_OPTIONS_EXEC
+	string "exec command"
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+	help
+	  Executable to launch on container instantiation and any relevant flags
+	  (e.g. /bin/sh).
+
+config BR2_TARGET_ROOTFS_ACI_OPTIONS_VERSION
+	string "version"
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+	help
+	  When combined with "name", this SHOULD be unique for every 
+	  build of an app (on a given "os"/"arch" combination - e.g. v1.3.2).
+
+comment "aci images should be supplied with a version"
+	depends on BR2_TARGET_ROOTFS_ACI_OPTIONS_VERSION = ""
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+
+config BR2_TARGET_ROOTFS_ACI_TIMESTAMP
+	bool "include created timestamp"
+	default n
+	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
+	help
+	  Include ACI annotation of timestamp when make was run.
+
+endif # !BR2_TARGET_ROOTFS_ACI_OPTIONS_MANIFEST_CUSTOM
+endif # BR2_TARGET_ROOTFS_ACI
diff --git a/fs/aci/aci.json.sh b/fs/aci/aci.json.sh
new file mode 100755
index 0000000..560e062
--- /dev/null
+++ b/fs/aci/aci.json.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+if [ "${ROOTFS_ACI_EXEC}n" == "n" ]; then
+	aciuser='"user": "0",'
+	acigroup='"group": "0"'
+else
+	aciuser='"user": '"\"${ROOTFS_ACI_EXEC}\","
+	acigroup='"group": '"\"${ROOTFS_ACI_EXEC}\""
+fi
+
+if [ "${ROOTFS_ACI_TIMESTAMP}n" != "n" ]; then
+createdat=",
+	{
+		\"name\": \"created\",
+		\"value\": \"${ROOTFS_ACI_TIMESTAMP}\"
+	}"
+fi
+
+
+cat <<EOF
+{
+	"acKind":"ImageManifest",
+	"acVersion":"0.7.4",
+	"name": "${ROOTFS_ACI_NAME}",
+	"labels":
+		[{
+			"name":"os",
+			"value":"linux"
+		},{
+			"name":"arch",
+			"value": "${ROOTFS_ACI_ARCH}"
+		},{ 
+			"name":"version",
+			"value": "${ROOTFS_ACI_VERSION}"
+		}],
+	"app":
+	{
+		"exec":
+			[
+			"${ROOTFS_ACI_EXEC}"
+			],
+		${aciuser}
+		${acigroup}
+	},
+	"annotations":
+	[{
+		"name": "buildroot.org/aci",
+		"value": "Generated by Buildroot"
+	}${createdat}
+	]
+}
+EOF
diff --git a/fs/aci/aci.mk b/fs/aci/aci.mk
new file mode 100644
index 0000000..abec378
--- /dev/null
+++ b/fs/aci/aci.mk
@@ -0,0 +1,80 @@
+################################################################################
+#
+# Generate App Container Image (ACI)
+# https://github.com/appc/spec/blob/master/spec/aci.md
+#
+################################################################################
+
+ROOTFS_ACI_NAME = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME))
+ROOTFS_ACI_EXEC = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_OPTIONS_EXEC))
+ROOTFS_ACI_ARCH = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_ARCH))
+ROOTFS_ACI_VERSION = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_OPTIONS_VERSION))
+ROOTFS_ACI_MANIFEST = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_MANIFEST_PATH))
+
+# Passing the generated variables into the running environment for make targets
+# leads to a cleaner implementation of the resulting shell scripts. In this way
+# nothing getting passed to tar should need quotes
+export ROOTFS_ACI_NAME
+export ROOTFS_ACI_EXEC 
+export ROOTFS_ACI_ARCH
+export ROOTFS_ACI_VERSION 
+export ROOTFS_ACI_MANIFEST 
+
+# If the user does not supply a version number, generate one which should play
+# nicely with reproducible builds, ideally signalling that for a given pairing
+# of buildroot version and config generates a consistent output.
+ifndef $(ROOTFS_ACI_VERSION)
+ROOTFS_ACI_VERSION = $(shell cat .config | sha256sum | cut -f1 -d' ')
+endif
+
+# In continuing support of deterministic builds, allow the user to annotate a
+# manifest with a timestamp but do not make this the default behavior.  In this
+# case it will be the timestamp of when "make" is run.
+ifeq ($(BR2_TARGET_ROOTFS_ACI_TIMESTAMP),y)
+ROOTFS_ACI_TIMESTAMP = $(shell date --rfc-3339=ns | tr " " "T")
+export ROOTFS_ACI_TIMESTAMP
+endif
+
+# For the generation of our ACI we pass the variables from the main
+# configuration into a scrip to generate a JSON manifest unless the user
+# provided their own manifest
+ifneq ($(ROOTFS_ACI_MANIFEST),)
+define ROOTFS_ACI_COPY_MANIFEST
+	cp '$(ROOTFS_ACI_MANIFEST)' $(BINARIES_DIR)/manifest
+endef
+ROOTFS_ACI_PRE_GEN_HOOKS += ROOTFS_ACI_COPY_MANIFEST
+else
+define ROOTFS_ACI_GENERATE_MANIFEST
+	fs/aci/aci.json.sh > $(BINARIES_DIR)/manifest
+endef
+ROOTFS_ACI_PRE_GEN_HOOKS += ROOTFS_ACI_GENERATE_MANIFEST
+endif
+
+# We then create the TAR archive of all components, making sure all paths are
+# represented correctly. Unfortunately as per fs/common.mk:8 the only a single
+# command is allowed.  The spec requires storing any relevant xattrs but as of 
+# 2016-05-22 buildroot does not generate these, though tooling inside the
+# target system can be added via the package BR2_PACKAGE_ATTR
+define ROOTFS_ACI_CMD
+	tar -cf $@ -C $(BINARIES_DIR) --owner=0 --group=0 manifest && \
+	tar -rf $@ -C $(TARGET_DIR) --numeric-owner --transform 's,^\./,rootfs/,' .
+endef
+
+# ACI images are required to end in ".aci" regardless of compression
+ifneq ($(BR2_TARGET_ROOTFS_ACI_NONE),y)
+define ROOTFS_ACI_MOVE
+	mv -f $(BINARIES_DIR)/rootfs.aci$(ROOTFS_ACI_COMPRESS_EXT) \
+		$(BINARIES_DIR)/rootfs.aci
+endef
+
+ROOTFS_ACI_POST_GEN_HOOKS += ROOTFS_ACI_MOVE
+endif
+
+ifeq ($(ROOTFS_ACI_NAME),)
+ifeq ($(call qstrip,$(ROOTFS_ACI_MANIFEST)),)
+$(error You supplied neither BR2_TARGET_ROOTFS_ACI_MANIFEST_PATH, \
+ROOTFS_ACI_NAME, nor BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME)
+endif
+endif
+
+$(eval $(call ROOTFS_TARGET,aci))
-- 
2.4.11

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

* [Buildroot] [v2] fs/aci: Add App Container Image (ACI) support to filesystems
  2016-05-23 15:07 [Buildroot] [v2] fs/aci: Add App Container Image (ACI) support to filesystems Brian 'redbeard' Harrington
@ 2016-05-24 13:14 ` Thomas Petazzoni
  2016-06-07 20:28 ` Thomas Petazzoni
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Petazzoni @ 2016-05-24 13:14 UTC (permalink / raw)
  To: buildroot

Hello,

Thanks for following up on this!

On Mon, 23 May 2016 08:07:30 -0700, Brian 'redbeard' Harrington wrote:
> App Container Images (ACI) are a Linux containerization image format
> defined as a part of the AppC specification as defined by CoreOS,
> Red Hat, Google, and community members. These images consist of a Linux
> userland and a JSON metadata file describing how to execute the actual
> runtime.
> 
> This filesystem package utilizes Buildroot for the generation of
> the userland and provides the user with a mechanism for configuring the
> contents of the manifest file.
> 

We need your Signed-off-by here (and not after the --- line like you're
doing today).

> ---
> Changes v1 -> v2
>   - Remove accidental 'default' of the fs/aci
>   - Remove dependency on host Jq
>   - Remove numerous "depends" lines in Kconfig, now in if block
>   - Set default name of localhost/buildroot as well as error check
>   - Allowed user to specify external manifest
>   - Enabled setting of timestamp annotation via a (non default) config
>     option
>   - Set default version via sha256 of .config file
>   - Added dependency on specific architectures
>   - Moved logic for architecture checks into Kconfig
>   - Use qstrip to clean up variables
>   - Export variables to scripts in package to avoid numerous
>     re-declarations
>   - Removed setting of xattrs in tar command
>   - Removed accidental re-use of TAR_OPTS
>   - Decoupled initial creation of tarball and adding of additional files
>     to simplify logic
>   - Removed stray debugging "echo" lines used during development
>   - Removed extra blank lines
>   - Moved logic for generation of json manifest into a shell script
> 
> Signed-off-by: Brian 'redbeard' Harrington <redbeard@coreos.com>
> ---
>  fs/Config.in       |   1 +
>  fs/aci/Config.in   | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/aci/aci.json.sh |  51 +++++++++++++++++++++++
>  fs/aci/aci.mk      |  80 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 249 insertions(+)
>  create mode 100644 fs/aci/Config.in
>  create mode 100755 fs/aci/aci.json.sh
>  create mode 100644 fs/aci/aci.mk
> 
> diff --git a/fs/Config.in b/fs/Config.in
> index 51ccf28..1c468c5 100644
> --- a/fs/Config.in
> +++ b/fs/Config.in
> @@ -1,5 +1,6 @@
>  menu "Filesystem images"
>  
> +source "fs/aci/Config.in"
>  source "fs/axfs/Config.in"
>  source "fs/cloop/Config.in"
>  source "fs/cpio/Config.in"
> diff --git a/fs/aci/Config.in b/fs/aci/Config.in
> new file mode 100644
> index 0000000..4df4791
> --- /dev/null
> +++ b/fs/aci/Config.in
> @@ -0,0 +1,117 @@
> +config BR2_TARGET_ROOTFS_ACI
> +	bool "app container image (aci)"
> +	default n

This line is not needed, as being disabled is the default state for a
bool option.

> +        depends on BR2_arm || BR2_armeb || BR2_aarch64_eb || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le
> +        depends on !BR2_ARM_CPU_ARMV4 

Wrong indentation for those two lines, it should be indented with a tab.

> +	help
> +	  Build an App Container Image for use with a Linux
> +	  containerization engine like rkt, Kurma, 3ofCoins, nosecone,
> +	  etc.

Maybe you could include some easy way to test such image, at least in
the commit log?

> +
> +if BR2_TARGET_ROOTFS_ACI
> +
> +config BR2_TARGET_ROOTFS_ACI_ARCH
> +	string
> +	default "i386"       if BR2_i386
> +	default "arm64"      if BR2_x86_64
> +	default "aarch64"    if BR2_aarch64
> +	default "aarch64_be" if BR2_aarch64_be
> +	default "ppc64"      if BR2_powerpc64
> +	default "armv6l"     if BR2_arm && BR2_ARM_CPU_ARMV6
> +	default "armv7l"     if BR2_arm && BR2_ARM_CPU_ARMV7A
> +	default "armv7l"     if BR2_arm && BR2_ARM_CPU_ARMV7M
> +	default "armv7b"     if BR2_armeb && BR2_ARM_CPU_ARMV7A
> +	default "armv7b"     if BR2_armeb && BR2_ARM_CPU_ARMV7M

I don't think supporting BR2_ARM_CPU_ARMV7M makes sense. I hardly see
an ARM noMMU system being used in this context, and I actually doubt it
works.

> +config BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
> +        bool "custom aci manifest"
> +        help
> +          Use custom aci manifest.                                

Trailing spaces.
            
> +
> +if BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
> +config BR2_TARGET_ROOTFS_ACI_MANIFEST_PATH
> +        string "custom path"                

Trailing spaces (please fix globally, there are more occurrences of
this).
 
> +        default ""                                                

Not needed, being empty is the default for a string.

> +	depends on BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST

It is useless to have both a "depends on" *and* enclose the option in
an "if...endif" block for the same condition. My preference is that
when there is a single option that should be conditionally visible, use
"depends on". When there's more, use a "if ... endif" block.

Also indentation is wrong.

> +        help
> +          Path to custom ACI manifest.
> +endif # BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
> +
> +if !BR2_TARGET_ROOTFS_ACI_OPTIONS_MANIFEST_CUSTOM

Please add an empty linew here.

> +config BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME
> +	string "name"
> +	default "localhost/buildroot"
> +	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
> +	help
> +	  A human-readable name for this App Container Image (e.g. 
> +	  example.com/my_app).
> +
> +comment "aci images must have a name"
> +	depends on BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME = "localhost/buildroot"

Why? Isn't this default name correct?

> +	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST

Not needed, you are already in a if ... endif block.

> +
> +config BR2_TARGET_ROOTFS_ACI_OPTIONS_EXEC
> +	string "exec command"
> +	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
> +	help
> +	  Executable to launch on container instantiation and any relevant flags
> +	  (e.g. /bin/sh).
> +
> +config BR2_TARGET_ROOTFS_ACI_OPTIONS_VERSION
> +	string "version"
> +	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST
> +	help
> +	  When combined with "name", this SHOULD be unique for every 
> +	  build of an app (on a given "os"/"arch" combination - e.g. v1.3.2).
> +
> +comment "aci images should be supplied with a version"
> +	depends on BR2_TARGET_ROOTFS_ACI_OPTIONS_VERSION = ""
> +	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST

Not needed, you're already in a if .. endif block for the same
condition.

> +
> +config BR2_TARGET_ROOTFS_ACI_TIMESTAMP
> +	bool "include created timestamp"
> +	default n

Not needed.

> +	depends on !BR2_TARGET_ROOTFS_ACI_USE_CUSTOM_MANIFEST

Ditto.

> +	help
> +	  Include ACI annotation of timestamp when make was run.

I am not sure this option is needed. Just always include a timestamp
for now. We can revisit than when/if we implement reproducible builds.

> +
> +endif # !BR2_TARGET_ROOTFS_ACI_OPTIONS_MANIFEST_CUSTOM
> +endif # BR2_TARGET_ROOTFS_ACI
> diff --git a/fs/aci/aci.json.sh b/fs/aci/aci.json.sh
> new file mode 100755
> index 0000000..560e062
> --- /dev/null
> +++ b/fs/aci/aci.json.sh
> @@ -0,0 +1,51 @@
> +#!/bin/sh
> +if [ "${ROOTFS_ACI_EXEC}n" == "n" ]; then

You can test if a string is empty rather than doing this.

if test -z "${ROOTFS_ACI_EXEC}" ; then
	...

> +	aciuser='"user": "0",'
> +	acigroup='"group": "0"'
> +else
> +	aciuser='"user": '"\"${ROOTFS_ACI_EXEC}\","
> +	acigroup='"group": '"\"${ROOTFS_ACI_EXEC}\""
> +fi
> +
> +if [ "${ROOTFS_ACI_TIMESTAMP}n" != "n" ]; then

Ditto.

> +createdat=",
> +	{
> +		\"name\": \"created\",
> +		\"value\": \"${ROOTFS_ACI_TIMESTAMP}\"
> +	}"
> +fi
> +
> +
> +cat <<EOF
> +{
> +	"acKind":"ImageManifest",
> +	"acVersion":"0.7.4",
> +	"name": "${ROOTFS_ACI_NAME}",
> +	"labels":
> +		[{
> +			"name":"os",
> +			"value":"linux"
> +		},{
> +			"name":"arch",
> +			"value": "${ROOTFS_ACI_ARCH}"
> +		},{ 
> +			"name":"version",
> +			"value": "${ROOTFS_ACI_VERSION}"
> +		}],
> +	"app":
> +	{
> +		"exec":
> +			[
> +			"${ROOTFS_ACI_EXEC}"
> +			],
> +		${aciuser}
> +		${acigroup}
> +	},
> +	"annotations":
> +	[{
> +		"name": "buildroot.org/aci",
> +		"value": "Generated by Buildroot"
> +	}${createdat}
> +	]
> +}
> +EOF

I must say I would probably prefer a template json file, and have
the .mk file simply SED some values in the template.

> diff --git a/fs/aci/aci.mk b/fs/aci/aci.mk
> new file mode 100644
> index 0000000..abec378
> --- /dev/null
> +++ b/fs/aci/aci.mk
> @@ -0,0 +1,80 @@
> +################################################################################
> +#
> +# Generate App Container Image (ACI)
> +# https://github.com/appc/spec/blob/master/spec/aci.md
> +#
> +################################################################################
> +
> +ROOTFS_ACI_NAME = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME))
> +ROOTFS_ACI_EXEC = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_OPTIONS_EXEC))
> +ROOTFS_ACI_ARCH = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_ARCH))
> +ROOTFS_ACI_VERSION = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_OPTIONS_VERSION))
> +ROOTFS_ACI_MANIFEST = $(call qstrip,$(BR2_TARGET_ROOTFS_ACI_MANIFEST_PATH))
> +
> +# Passing the generated variables into the running environment for make targets
> +# leads to a cleaner implementation of the resulting shell scripts. In this way
> +# nothing getting passed to tar should need quotes
> +export ROOTFS_ACI_NAME
> +export ROOTFS_ACI_EXEC 
> +export ROOTFS_ACI_ARCH
> +export ROOTFS_ACI_VERSION 
> +export ROOTFS_ACI_MANIFEST 

I dislike this. Please pass the variables that are needed in the
environment.

> +# If the user does not supply a version number, generate one which should play
> +# nicely with reproducible builds, ideally signalling that for a given pairing
> +# of buildroot version and config generates a consistent output.
> +ifndef $(ROOTFS_ACI_VERSION)

Please use:

ifeq ($(ROOTFS_ACI_VERSION),)

> +ROOTFS_ACI_VERSION = $(shell cat .config | sha256sum | cut -f1 -d' ')
> +endif

Then the Config.in comment about the version being mandatory should be
removed, because you generate a version number dynamically.

> +
> +# In continuing support of deterministic builds, allow the user to annotate a
> +# manifest with a timestamp but do not make this the default behavior.  In this
> +# case it will be the timestamp of when "make" is run.
> +ifeq ($(BR2_TARGET_ROOTFS_ACI_TIMESTAMP),y)
> +ROOTFS_ACI_TIMESTAMP = $(shell date --rfc-3339=ns | tr " " "T")
> +export ROOTFS_ACI_TIMESTAMP

Argh, don't expert variabls.

> +endif
> +
> +# For the generation of our ACI we pass the variables from the main
> +# configuration into a scrip to generate a JSON manifest unless the user
> +# provided their own manifest
> +ifneq ($(ROOTFS_ACI_MANIFEST),)
> +define ROOTFS_ACI_COPY_MANIFEST
> +	cp '$(ROOTFS_ACI_MANIFEST)' $(BINARIES_DIR)/manifest
> +endef
> +ROOTFS_ACI_PRE_GEN_HOOKS += ROOTFS_ACI_COPY_MANIFEST
> +else
> +define ROOTFS_ACI_GENERATE_MANIFEST
> +	fs/aci/aci.json.sh > $(BINARIES_DIR)/manifest

That's where the SED magic should be done. And you could also maybe do
it when a custom manifest is passed. See in fs/ubifs/ubi.mk what we're
doing with ubinize.cfg for an example.

> +endef
> +ROOTFS_ACI_PRE_GEN_HOOKS += ROOTFS_ACI_GENERATE_MANIFEST
> +endif
> +
> +# We then create the TAR archive of all components, making sure all paths are
> +# represented correctly. Unfortunately as per fs/common.mk:8 the only a single
> +# command is allowed.

Maybe this needs to be fixed?

>  The spec requires storing any relevant xattrs but as of 
> +# 2016-05-22 buildroot does not generate these, though tooling inside the
> +# target system can be added via the package BR2_PACKAGE_ATTR

I dislike the comments that mention a specific date.

> +define ROOTFS_ACI_CMD
> +	tar -cf $@ -C $(BINARIES_DIR) --owner=0 --group=0 manifest && \
> +	tar -rf $@ -C $(TARGET_DIR) --numeric-owner --transform 's,^\./,rootfs/,' .
> +endef
> +
> +# ACI images are required to end in ".aci" regardless of compression
> +ifneq ($(BR2_TARGET_ROOTFS_ACI_NONE),y)
> +define ROOTFS_ACI_MOVE
> +	mv -f $(BINARIES_DIR)/rootfs.aci$(ROOTFS_ACI_COMPRESS_EXT) \
> +		$(BINARIES_DIR)/rootfs.aci

Isn't a link more appropriate here?

> +endef
> +
> +ROOTFS_ACI_POST_GEN_HOOKS += ROOTFS_ACI_MOVE
> +endif
> +
> +ifeq ($(ROOTFS_ACI_NAME),)
> +ifeq ($(call qstrip,$(ROOTFS_ACI_MANIFEST)),)
> +$(error You supplied neither BR2_TARGET_ROOTFS_ACI_MANIFEST_PATH, \
> +ROOTFS_ACI_NAME, nor BR2_TARGET_ROOTFS_ACI_OPTIONS_NAME)

It's weird to have BR2_... and ROOTFS_ACI_... options in the same error
message. Doesn't seem good.

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

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

* [Buildroot] [v2] fs/aci: Add App Container Image (ACI) support to filesystems
  2016-05-23 15:07 [Buildroot] [v2] fs/aci: Add App Container Image (ACI) support to filesystems Brian 'redbeard' Harrington
  2016-05-24 13:14 ` Thomas Petazzoni
@ 2016-06-07 20:28 ` Thomas Petazzoni
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Petazzoni @ 2016-06-07 20:28 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, 23 May 2016 08:07:30 -0700, Brian 'redbeard' Harrington wrote:

> +# We then create the TAR archive of all components, making sure all paths are
> +# represented correctly. Unfortunately as per fs/common.mk:8 the only a single
> +# command is allowed.

FYI, as of a few minutes ago, this limitation has been lifted, thanks
to commit
https://git.busybox.net/buildroot/commit/?id=6dd7bbb59134799ed3d7343f238b3b02592faebf
from Yann.

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

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

end of thread, other threads:[~2016-06-07 20:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-23 15:07 [Buildroot] [v2] fs/aci: Add App Container Image (ACI) support to filesystems Brian 'redbeard' Harrington
2016-05-24 13:14 ` Thomas Petazzoni
2016-06-07 20:28 ` Thomas Petazzoni

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.