[Buildroot] [PATCH v2 1/3] autobuild-run: initial implementation of check_reproducibility()

[Buildroot] [PATCH v2 1/3] autobuild-run: initial implementation of check_reproducibility()

[Atharva Lele]

Find out differences in built images, if any

Add diffoscope to list of optional programs

Signed-off-by: Atharva Lele <itsatharva@gmail.com>

---
Changes v1 -> v2:
  - move diffoscope output to results dir (suggested by arnout)
  - fix make printvars call
  - Add diffoscope to DEFAULT_OPTIONAL_PROGS

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index ef2f2a5..5d76583 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -204,7 +204,7 @@ def get_branch():
 
 class SystemInfo:
     DEFAULT_NEEDED_PROGS = ["make", "git", "gcc"]
-    DEFAULT_OPTIONAL_PROGS = ["bzr", "java", "javac", "jar"]
+    DEFAULT_OPTIONAL_PROGS = ["bzr", "diffoscope", "java", "javac", "jar"]
 
     def __init__(self):
         self.needed_progs = list(self.__class__.DEFAULT_NEEDED_PROGS)
@@ -394,6 +394,45 @@ def stop_on_build_hang(monitor_thread_hung_build_flag,
                 break
         monitor_thread_stop_flag.wait(30)
 
+def check_reproducibility(**kwargs):
+    """Check reproducibility of builds
+
+    Use diffoscope on the built images, if diffoscope is not
+    installed, fallback to cmp
+    """
+
+    log = kwargs['log']
+    idir = "instance-%d" % kwargs['instance']
+    outputdir = os.path.join(idir, "output")
+    srcdir = os.path.join(idir, "buildroot")
+    diffoscope_output = os.path.join(outputdir, "results", "diffoscope-results.txt")
+    # Using only tar images for now
+    build_1_image = os.path.join(outputdir, "images-1", "rootfs.tar")
+    build_2_image = os.path.join(outputdir, "images", "rootfs.tar")
+
+    # Prefix to point diffoscope towards cross-tools
+    prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", srcdir, "printvars", "VARS=TARGET_CROSS"])
+    # Remove TARGET_CROSS= and \n from the string
+    prefix = prefix[13:-1]
+
+    if kwargs['sysinfo'].has("diffoscope"):
+        log_write(log, "INFO: running diffoscope on images")
+        ret = subprocess.call(["diffoscope", build_1_image, build_2_image,
+                            "--tool-prefix-binutils", prefix,
+                            "--text", diffoscope_output])
+    else:
+        if e.errno == errno.ENOENT:
+            log_write(log, "INFO: diffoscope not installed, falling back to cmp")
+        ret = subprocess.call(["cmp", "build_1_image", "build_2_image", "-b", ">", diffoscope_output])
+    
+    if ret != 0:
+        log_write(log, "INFO: Build is non-reproducible.")
+        return -1
+    
+    # rootfs images match byte-for-byte -> reproducible image
+    log_write(log, "INFO: Build is reproducible!")
+    return 0
+
 def do_build(**kwargs):
     """Run the build itself"""
 
-- 
2.20.1

[Buildroot] [PATCH v2 2/3] autobuild-run: initial implementation of do_reproducible_build()

[Atharva Lele]

This new function will call do_build() twice to produce two builds
and then check their reproducibility

Signed-off-by: Atharva Lele <itsatharva@gmail.com>

---
Changes v1 -> v2:
  - make clean outputs to devnull (suggested by arnout)
  - remove unnecessary arguments to make clean (suggested by arnout)

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 5d76583..a4ddb5b 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -494,6 +494,46 @@ def do_build(**kwargs):
     log_write(log, "INFO: build successful")
     return 0
 
+def do_reproducible_build(**kwargs):
+    """Run the builds for reproducibility testing
+
+    Build twice with the same configuration. Calls do_build() to
+    perform the actual build.
+    """
+
+    idir = "instance-%d" % kwargs['instance']
+    outputdir = os.path.join(idir, "output")
+    log = kwargs['log']
+    
+    # Start the first build
+    log_write(log, "INFO: Reproducible Build Test, starting build 1")
+    ret = do_build(**kwargs)
+    if ret != 0:
+        log_write(log, "INFO: build 1 failed, skipping build 2")
+        return ret
+    
+    # First build has been built, move files and start build 2
+    os.rename(os.path.join(outputdir, "images"), os.path.join(outputdir, "images-1"))
+
+    # Clean up build 1
+    if kwargs['debug']:
+        devnull = log
+    else:
+        devnull = open(os.devnull, "w")
+    cmd = ["make", "O=%s" % outputdir, "clean"]
+    subprocess.call(cmd, stdout=devnull, stderr=devnull)
+
+    # Start the second build
+    log_write(log, "INFO: Reproducible Build Test, starting build 2")
+    ret = do_build(**kwargs)
+    if ret != 0:
+        log_write(log, "INFO: build 2 failed")
+        return ret
+
+    # Assuming both have built successfully
+    ret = check_reproducibility(**kwargs)
+    return ret
+
 def send_results(result, **kwargs):
     """Prepare and store/send tarball with results
 
-- 
2.20.1

[Buildroot] [PATCH v2 3/3] autobuild-run: do reproducible builds tests if BR2_REPRODUCIBLE=y

[Atharva Lele]

Signed-off-by: Atharva Lele <itsatharva@gmail.com>

---
Changes v1 -> v2:
  - add trailing newline character to BR2_REPRODUCIBLE=y (suggested by arnout)

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index a4ddb5b..bdbb48b 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -731,7 +731,15 @@ def run_instance(**kwargs):
             log_write(kwargs['log'], "WARN: failed to generate configuration")
             continue
 
-        ret = do_build(**kwargs)
+        # Check if the build test is supposed to be a reproducible test
+        outputdir = os.path.abspath(os.path.join(idir, "output"))
+        with open(os.path.join(outputdir, ".config"), "r") as fconf:
+            if "BR2_REPRODUCIBLE=y\n" in fconf.read():
+                log_write(kwargs['log'], "INFO: Reproducible Build Test")
+                ret = do_reproducible_build(**kwargs)
+            else:
+                log_write(kwargs['log'], "INFO: Non Reproducible Build Test")
+                ret = do_build(**kwargs)
 
         send_results(ret, **kwargs)
 
-- 
2.20.1

[Buildroot] [PATCH v2 2/3] autobuild-run: initial implementation of do_reproducible_build()

[Thomas Petazzoni]

Hello,

On Fri,  7 Jun 2019 12:16:56 +0530
Atharva Lele <itsatharva@gmail.com> wrote:

> This new function will call do_build() twice to produce two builds
> and then check their reproducibility
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

So, if I read the implementation correctly, in this first step, we
build twice in the same output path, i.e the only variation between the
first and the second build is the time at which the build takes place.

We discussed on IRC that we could easily also change the output path
(i.e build location) between the first and second build. But perhaps
this is something that is planned to do in a second step, and you want
to make progress with a simpler initial solution ?

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

[Buildroot] [PATCH v2 2/3] autobuild-run: initial implementation of do_reproducible_build()

[Arnout Vandecappelle]

On 07/06/2019 10:33, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri,  7 Jun 2019 12:16:56 +0530
> Atharva Lele <itsatharva@gmail.com> wrote:
> 
>> This new function will call do_build() twice to produce two builds
>> and then check their reproducibility
>>
>> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> 
> So, if I read the implementation correctly, in this first step, we
> build twice in the same output path, i.e the only variation between the
> first and the second build is the time at which the build takes place.
> 
> We discussed on IRC that we could easily also change the output path
> (i.e build location) between the first and second build. But perhaps
> this is something that is planned to do in a second step, and you want
> to make progress with a simpler initial solution ?

 Exactly.

 Though the solution with a different output path is probably even simpler, in
the end.

 The issue, though, is that it requires a bit of refactoring of the
autobuild-run code, so it takes a bit of time to develop and even more time to
review. On the other hand, we want to start running reproducibility tests as
soon as possible, so we can get some results from the autobuilders and have an
idea of how bad it is.

 Therefore, we've chosen to resubmit the patches with the same output dir and
hope that they can be applied quickly. Hint, hint :-)

 Regards,
 Arnout

[Buildroot] [PATCH v2 2/3] autobuild-run: initial implementation of do_reproducible_build()

[Atharva Lele]

> But perhaps this is something that is planned to do in a second step,
> and you want to make progress with a simpler initial solution ?

Precisely. Varying the build location is planned for the second step.

Regards,
Atharva Lele


On Fri, Jun 7, 2019 at 2:03 PM Thomas Petazzoni <
thomas.petazzoni@bootlin.com> wrote:

> Hello,
>
> On Fri,  7 Jun 2019 12:16:56 +0530
> Atharva Lele <itsatharva@gmail.com> wrote:
>
> > This new function will call do_build() twice to produce two builds
> > and then check their reproducibility
> >
> > Signed-off-by: Atharva Lele <itsatharva@gmail.com>
>
> So, if I read the implementation correctly, in this first step, we
> build twice in the same output path, i.e the only variation between the
> first and the second build is the time at which the build takes place.
>
> We discussed on IRC that we could easily also change the output path
> (i.e build location) between the first and second build. But perhaps
> this is something that is planned to do in a second step, and you want
> to make progress with a simpler initial solution ?
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190607/e295ab52/attachment.html>

[Buildroot] [PATCH v2 2/3] autobuild-run: initial implementation of do_reproducible_build()

[Thomas Petazzoni]

On Fri, 7 Jun 2019 10:39:40 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>  Exactly.
> 
>  Though the solution with a different output path is probably even simpler, in
> the end.
> 
>  The issue, though, is that it requires a bit of refactoring of the
> autobuild-run code, so it takes a bit of time to develop and even more time to
> review. On the other hand, we want to start running reproducibility tests as
> soon as possible, so we can get some results from the autobuilders and have an
> idea of how bad it is.

Makes sense.

>  Therefore, we've chosen to resubmit the patches with the same output dir and
> hope that they can be applied quickly. Hint, hint :-)

Hehe :-)

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

[Buildroot] [PATCH v2 1/3] autobuild-run: initial implementation of check_reproducibility()

[Arnout Vandecappelle]

 Hi Atharva,

On 07/06/2019 08:46, Atharva Lele wrote:
> Find out differences in built images, if any
> 
> Add diffoscope to list of optional programs

 The commit message could use a little more love. For sure, sentences should be
sentences, so end with a period. But also the flow could be improved a bit, like:

For reproducible builds, we want to be able to tell there are differences
between two builds of the same configuration, and also to pinpoint why these
differences happen. The diffoscope tool looks inside different kinds of files
and identifies exactly where the differences come from.

check_reproduciblity() runs the diffoscope tool on two output directories, which
are expected to be in output/images and output/images-1. diffoscope uses
objdump, so it needs to the cross-compile prefix; this is retrieved from the
TARGET_CROSS variable.

Since the diffoscope tool may not be installed, fall back to cmp if it isn't.
Add diffoscope to the list of optional programs to avoid checking for its
presence over and over.

> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> 
> ---
> Changes v1 -> v2:
>   - move diffoscope output to results dir (suggested by arnout)
>   - fix make printvars call
>   - Add diffoscope to DEFAULT_OPTIONAL_PROGS
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> ---
>  scripts/autobuild-run | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index ef2f2a5..5d76583 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -204,7 +204,7 @@ def get_branch():
>  
>  class SystemInfo:
>      DEFAULT_NEEDED_PROGS = ["make", "git", "gcc"]
> -    DEFAULT_OPTIONAL_PROGS = ["bzr", "java", "javac", "jar"]
> +    DEFAULT_OPTIONAL_PROGS = ["bzr", "diffoscope", "java", "javac", "jar"]
>  
>      def __init__(self):
>          self.needed_progs = list(self.__class__.DEFAULT_NEEDED_PROGS)
> @@ -394,6 +394,45 @@ def stop_on_build_hang(monitor_thread_hung_build_flag,
>                  break
>          monitor_thread_stop_flag.wait(30)
>  
> +def check_reproducibility(**kwargs):
> +    """Check reproducibility of builds
> +
> +    Use diffoscope on the built images, if diffoscope is not
> +    installed, fallback to cmp
> +    """
> +
> +    log = kwargs['log']
> +    idir = "instance-%d" % kwargs['instance']
> +    outputdir = os.path.join(idir, "output")
> +    srcdir = os.path.join(idir, "buildroot")
> +    diffoscope_output = os.path.join(outputdir, "results", "diffoscope-results.txt")
> +    # Using only tar images for now

 Actually, why? We could just pass the entire directory to diffoscope, no? Some
packages install additional images which we also want to compare, e.g. xen.

 Hm, that would make the cmp part more difficult though... So let's keep it like
this for the time being.


> +    build_1_image = os.path.join(outputdir, "images-1", "rootfs.tar")
> +    build_2_image = os.path.join(outputdir, "images", "rootfs.tar")
> +
> +    # Prefix to point diffoscope towards cross-tools
> +    prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", srcdir, "printvars", "VARS=TARGET_CROSS"])
> +    # Remove TARGET_CROSS= and \n from the string
> +    prefix = prefix[13:-1]

 Since this is only used for diffoscope, move it inside the condition.

> +
> +    if kwargs['sysinfo'].has("diffoscope"):
> +        log_write(log, "INFO: running diffoscope on images")
> +        ret = subprocess.call(["diffoscope", build_1_image, build_2_image,
> +                            "--tool-prefix-binutils", prefix,
> +                            "--text", diffoscope_output])
> +    else:
> +        if e.errno == errno.ENOENT:

 Please test your patch before submitting :-) This is a leftover from when you
used an exception. It is no longer valid.

> +            log_write(log, "INFO: diffoscope not installed, falling back to cmp")
> +        ret = subprocess.call(["cmp", "build_1_image", "build_2_image", "-b", ">", diffoscope_output])

 Again, you haven't tested this. If you want to use shell redirection, you have
to add shell=True argument. However, you shouldn't do that; instead, use
stdout=open(diffoscope_output, 'w'). So:

        ret = subprocess.coll(["cmp", "-b", build_1_image, build_2_image],
                              stdout=open(diffoscope_output, 'w'))

 Even better would be to do it with a 'with' construct for opening the output file.

 Regards,
 Arnout

> +    
> +    if ret != 0:
> +        log_write(log, "INFO: Build is non-reproducible.")
> +        return -1
> +    
> +    # rootfs images match byte-for-byte -> reproducible image
> +    log_write(log, "INFO: Build is reproducible!")
> +    return 0
> +
>  def do_build(**kwargs):
>      """Run the build itself"""
>  
> 

[Buildroot] [PATCH v2 2/3] autobuild-run: initial implementation of do_reproducible_build()

[Arnout Vandecappelle]

On 07/06/2019 08:46, Atharva Lele wrote:
> This new function will call do_build() twice to produce two builds
> and then check their reproducibility

 This commit message is much more complete :-). Just a missing period at the end.

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> 
> ---
> Changes v1 -> v2:
>   - make clean outputs to devnull (suggested by arnout)
>   - remove unnecessary arguments to make clean (suggested by arnout)
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> ---
>  scripts/autobuild-run | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 5d76583..a4ddb5b 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -494,6 +494,46 @@ def do_build(**kwargs):
>      log_write(log, "INFO: build successful")
>      return 0
>  
> +def do_reproducible_build(**kwargs):
> +    """Run the builds for reproducibility testing
> +
> +    Build twice with the same configuration. Calls do_build() to
> +    perform the actual build.
> +    """
> +
> +    idir = "instance-%d" % kwargs['instance']
> +    outputdir = os.path.join(idir, "output")
> +    log = kwargs['log']
> +    
> +    # Start the first build
> +    log_write(log, "INFO: Reproducible Build Test, starting build 1")
> +    ret = do_build(**kwargs)
> +    if ret != 0:
> +        log_write(log, "INFO: build 1 failed, skipping build 2")
> +        return ret
> +    
> +    # First build has been built, move files and start build 2
> +    os.rename(os.path.join(outputdir, "images"), os.path.join(outputdir, "images-1"))
> +
> +    # Clean up build 1
> +    if kwargs['debug']:
> +        devnull = log
> +    else:
> +        devnull = open(os.devnull, "w")
> +    cmd = ["make", "O=%s" % outputdir, "clean"]
> +    subprocess.call(cmd, stdout=devnull, stderr=devnull)
> +
> +    # Start the second build
> +    log_write(log, "INFO: Reproducible Build Test, starting build 2")
> +    ret = do_build(**kwargs)
> +    if ret != 0:
> +        log_write(log, "INFO: build 2 failed")
> +        return ret
> +
> +    # Assuming both have built successfully
> +    ret = check_reproducibility(**kwargs)
> +    return ret
> +
>  def send_results(result, **kwargs):
>      """Prepare and store/send tarball with results
>  
> 

[Buildroot] [PATCH v2 3/3] autobuild-run: do reproducible builds tests if BR2_REPRODUCIBLE=y

[Arnout Vandecappelle]

On 07/06/2019 08:46, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> 
> ---
> Changes v1 -> v2:
>   - add trailing newline character to BR2_REPRODUCIBLE=y (suggested by arnout)
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> ---
>  scripts/autobuild-run | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index a4ddb5b..bdbb48b 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -731,7 +731,15 @@ def run_instance(**kwargs):
>              log_write(kwargs['log'], "WARN: failed to generate configuration")
>              continue
>  
> -        ret = do_build(**kwargs)
> +        # Check if the build test is supposed to be a reproducible test
> +        outputdir = os.path.abspath(os.path.join(idir, "output"))
> +        with open(os.path.join(outputdir, ".config"), "r") as fconf:
> +            if "BR2_REPRODUCIBLE=y\n" in fconf.read():
> +                log_write(kwargs['log'], "INFO: Reproducible Build Test")
> +                ret = do_reproducible_build(**kwargs)

 It's maybe a little bit a silly thing or a micro-optimisation, but this way,
the .config file is kept open during the entire build. So I think a better way
to do it would be:

        with ...:
            reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
        if reproducible:
            ...


> +            else:
> +                log_write(kwargs['log'], "INFO: Non Reproducible Build Test")

 Both log messages are redundant, I think. The reproducible one is immediately
followed by "INFO: Reproducible Build Test, starting build 1". The
non-reproducible one is immediately followed by "INFO: build started" which also
makes it pretty clear it's not a reproducible build.

 Regards,
 Arnout

> +                ret = do_build(**kwargs)
>  
>          send_results(ret, **kwargs)
>  
>