[Buildroot] [PATCH 1/2] package/cpio: add host version

[Buildroot] [PATCH 1/2] package/cpio: add host version

[Atharva Lele]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

The latest cpio has a --reproducible option, which may come handy when
we try to, well, be reproducible...

Reported-by: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
[Atharva: Made change pointed out by Arnout: remove HOST_CPIO_CONF_OPTS]
Signed-off-by: Atharva Lele <itsatharva@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Atharva Lele <itsatharva@gmail.com>
---
Changes v1 -> v2:
  - Add patch from Yann to my series
---
 package/cpio/cpio.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/cpio/cpio.mk b/package/cpio/cpio.mk
index 9a09279572..cc40838840 100644
--- a/package/cpio/cpio.mk
+++ b/package/cpio/cpio.mk
@@ -17,3 +17,4 @@ CPIO_DEPENDENCIES += argp-standalone
 endif
 
 $(eval $(autotools-package))
+$(eval $(host-autotools-package))
-- 
2.22.0

[Buildroot] [PATCH 2/2] fs/cpio: make cpio rootfs reproducible

[Atharva Lele]

Pass the recommended argument in the CPIO manual to make cpio archives
reproducible.

Reference: https://www.gnu.org/software/cpio/manual/cpio.html#Copy_002dpass-mode

Pre-patch diffoscope output: https://gitlab.com/snippets/1874745
Post-patch: https://gitlab.com/snippets/1874746

We can see that post-patch, the archive related differences are removed.
The differences are arising from utils/bin/getconf. This will have to
be investigated further. However, that is unrelated to cpio.

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
Changes v1 -> v2:
  - Added comment
  - Build host-cpio if we are reproducible, reason is --reproducible
    option does not exist prior to cpio 2.12. (suggested by Yann, Arnout)
---
 fs/cpio/cpio.mk | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/cpio/cpio.mk b/fs/cpio/cpio.mk
index 1f0cafe62f..521418f950 100644
--- a/fs/cpio/cpio.mk
+++ b/fs/cpio/cpio.mk
@@ -29,8 +29,16 @@ endif # BR2_ROOTFS_DEVICE_CREATION_STATIC
 
 ROOTFS_CPIO_PRE_GEN_HOOKS += ROOTFS_CPIO_ADD_INIT
 
+# --repoducible option was not introduced until cpio v2.12. Some distros
+# like Ubuntu 16.04 or Debian oldstable which package v2.11. Hence build
+# host-cpio.
+ifeq ($(BR2_REPRODUCIBLE), y)
+ROOTFS_CPIO_DEPENDENCIES += host-cpio
+CPIO_OPTS = --reproducible
+endif
+
 define ROOTFS_CPIO_CMD
-	cd $(TARGET_DIR) && find . | cpio --quiet -o -H newc > $@
+	cd $(TARGET_DIR) && find . | cpio $(CPIO_OPTS) --quiet -o -H newc > $@
 endef
 
 ifeq ($(BR2_TARGET_ROOTFS_CPIO_UIMAGE),y)
-- 
2.22.0

[Buildroot] [PATCH 1/2] package/cpio: add host version

[Yann E. MORIN]

Atharva, All,

On 2019-07-14 21:53 +0530, Atharva Lele spake thusly:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> The latest cpio has a --reproducible option, which may come handy when
> we try to, well, be reproducible...
> 
> Reported-by: Arnout Vandecappelle <arnout@mind.be>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> [Atharva: Made change pointed out by Arnout: remove HOST_CPIO_CONF_OPTS]

Actually, this is not very intereesting, because it does not say what
was in HOST_CPIO_CONF_OPTS. What we need to know is that we do not need
to force --bindir. So, I'd have written:
    [Atharva: don't force --bindir, as noticed by Arnout]

> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Atharva Lele <itsatharva@gmail.com>

Except for the minor nit above:

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

Thanks! :-)

Regards,
Yann E. MORIN.

> ---
> Changes v1 -> v2:
>   - Add patch from Yann to my series
> ---
>  package/cpio/cpio.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/cpio/cpio.mk b/package/cpio/cpio.mk
> index 9a09279572..cc40838840 100644
> --- a/package/cpio/cpio.mk
> +++ b/package/cpio/cpio.mk
> @@ -17,3 +17,4 @@ CPIO_DEPENDENCIES += argp-standalone
>  endif
>  
>  $(eval $(autotools-package))
> +$(eval $(host-autotools-package))
> -- 
> 2.22.0
> 
> _______________________________________________
> 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 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/2] fs/cpio: make cpio rootfs reproducible

[Yann E. MORIN]

Atharva, All,

On 2019-07-14 21:53 +0530, Atharva Lele spake thusly:
> Pass the recommended argument in the CPIO manual to make cpio archives
> reproducible.
> 
> Reference: https://www.gnu.org/software/cpio/manual/cpio.html#Copy_002dpass-mode
> 
> Pre-patch diffoscope output: https://gitlab.com/snippets/1874745
> Post-patch: https://gitlab.com/snippets/1874746
> 
> We can see that post-patch, the archive related differences are removed.
> The differences are arising from utils/bin/getconf. This will have to
> be investigated further. However, that is unrelated to cpio.
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> ---
> Changes v1 -> v2:
>   - Added comment
>   - Build host-cpio if we are reproducible, reason is --reproducible
>     option does not exist prior to cpio 2.12. (suggested by Yann, Arnout)
> ---
>  fs/cpio/cpio.mk | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/cpio/cpio.mk b/fs/cpio/cpio.mk
> index 1f0cafe62f..521418f950 100644
> --- a/fs/cpio/cpio.mk
> +++ b/fs/cpio/cpio.mk
> @@ -29,8 +29,16 @@ endif # BR2_ROOTFS_DEVICE_CREATION_STATIC
>  
>  ROOTFS_CPIO_PRE_GEN_HOOKS += ROOTFS_CPIO_ADD_INIT
>  
> +# --repoducible option was not introduced until cpio v2.12. Some distros
> +# like Ubuntu 16.04 or Debian oldstable which package v2.11. Hence build
> +# host-cpio.
> +ifeq ($(BR2_REPRODUCIBLE), y)
> +ROOTFS_CPIO_DEPENDENCIES += host-cpio
> +CPIO_OPTS = --reproducible

Having a pure assignment in a condition is risky for the future. If we
were to later add new unconditional options, we would add them before
this part (as is customary to do: uncoditional stuff goes before
conditional stuff). In this case, the assignment here would override the
uncoditional options, which is not nice.

Instead, I think we prefer an append-assignment in conditional blocks,
even if the variable is not already defined.

Arnout, Thomas, can you confirm/infirm my opinion? ;-)

Otherwise:

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

Regards,
Yann E. MORIN.

> +endif
> +
>  define ROOTFS_CPIO_CMD
> -	cd $(TARGET_DIR) && find . | cpio --quiet -o -H newc > $@
> +	cd $(TARGET_DIR) && find . | cpio $(CPIO_OPTS) --quiet -o -H newc > $@
>  endef
>  
>  ifeq ($(BR2_TARGET_ROOTFS_CPIO_UIMAGE),y)
> -- 
> 2.22.0
> 
> _______________________________________________
> 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 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/2] fs/cpio: make cpio rootfs reproducible

[Yann E. MORIN]

Atharva, All,

On 2019-07-14 18:47 +0200, Yann E. MORIN spake thusly:
> On 2019-07-14 21:53 +0530, Atharva Lele spake thusly:
> > Pass the recommended argument in the CPIO manual to make cpio archives
> > reproducible.
[--SNIP--]
> > +# --repoducible option was not introduced until cpio v2.12. Some distros
> > +# like Ubuntu 16.04 or Debian oldstable which package v2.11. Hence build
> > +# host-cpio.
> > +ifeq ($(BR2_REPRODUCIBLE), y)
> > +ROOTFS_CPIO_DEPENDENCIES += host-cpio
> > +CPIO_OPTS = --reproducible

In addition to my previous comments: the variable shall be prefixed with
ROOTFS_CPIO_, not just with CPIO_, otherwise it could clash with variables
from the cpio package itself.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/2] fs/cpio: make cpio rootfs reproducible

[Atharva Lele]

Yann, all:

Replying to both of Yann's emails in this one:

On Sunday, July 14, 2019 10:17:25 PM IST Yann E. MORIN wrote:
> Having a pure assignment in a condition is risky for the future. If we
> were to later add new unconditional options, we would add them before
> this part (as is customary to do: uncoditional stuff goes before
> conditional stuff). In this case, the assignment here would override the
> uncoditional options, which is not nice.
> 
> Instead, I think we prefer an append-assignment in conditional blocks,
> even if the variable is not already defined.

Ah yes that makes sense. I'll change it to an append-assignment in v2.
Thanks!

On Sunday, July 14, 2019 10:51:33 PM IST Yann E. MORIN wrote:
> Atharva, All,
> 
> On 2019-07-14 18:47 +0200, Yann E. MORIN spake thusly:
> > On 2019-07-14 21:53 +0530, Atharva Lele spake thusly:
> > > Pass the recommended argument in the CPIO manual to make cpio archives
> > > reproducible.
> 
> [--SNIP--]
> 
> > > +# --repoducible option was not introduced until cpio v2.12. Some
> > > distros
> > > +# like Ubuntu 16.04 or Debian oldstable which package v2.11. Hence
> > > build
> > > +# host-cpio.
> > > +ifeq ($(BR2_REPRODUCIBLE), y)
> > > +ROOTFS_CPIO_DEPENDENCIES += host-cpio
> > > +CPIO_OPTS = --reproducible
> 
> In addition to my previous comments: the variable shall be prefixed with
> ROOTFS_CPIO_, not just with CPIO_, otherwise it could clash with variables
> from the cpio package itself.

Agreed, fixing it in v2.

> Regards,
> Yann E. MORIN.

Thanks for the review!

Regards,
Atharva Lele