[Buildroot] [PATCH v5 1/4] autobuild-run: move creation of result directory to run_instance()

[Buildroot] [PATCH v5 1/4] autobuild-run: move creation of result directory to run_instance()

[Atharva Lele]

We need the result directory to be present when the check_reproducibility()
function (implemented in the next patch) executes. As of now the results
directory is created in the send_results() function which executes after
check_reproducibility() will.

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>

---
Changes v4: Add new patch to series
---
 scripts/autobuild-run | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index ef2f2a5..190a254 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -470,8 +470,6 @@ def send_results(result, **kwargs):
     srcdir = os.path.join(idir, "buildroot")
     resultdir = os.path.join(outputdir, "results")
 
-    os.mkdir(resultdir)
-
     shutil.copyfile(os.path.join(outputdir, ".config"),
                     os.path.join(resultdir, "config"))
     shutil.copyfile(os.path.join(outputdir, "defconfig"),
@@ -647,6 +645,9 @@ def run_instance(**kwargs):
         if ret != 0:
             continue
 
+        resultdir = os.path.join(idir, "output", "results")
+        os.mkdir(resultdir)
+
         ret = gen_config(**kwargs)
         if ret != 0:
             log_write(kwargs['log'], "WARN: failed to generate configuration")
-- 
2.20.1

[Buildroot] [PATCH v5 2/4] autobuild-run: initial implementation of check_reproducibility()

[Atharva Lele]

For reproducible builds, we want to find out if there are any differences
between two builds having the same configuration, and to find the reason behind
them. The diffoscope tool looks inside different types of files to see where the
differences lie.

check_reproducibility() runs diffoscope on two output directories which are
expected in output/images and output/images-1. Since it uses objdump, it needs
to be provided the cross-compile prefix which is derived from the TARGET_CROSS
variable.

Since diffoscope may not be installed, we fall back to cmp for byte-by-byte
comparison. We add diffoscope to list of optional programs to avoid repeated
checking of its presence.

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>

---
Changes v4 -> v5:
  - Fix cmp fallback command

Changes v3 -> v4:
  - Removed spaces on empty lines (suggested by Yann)

Changes v2 -> v3:
  - Use file size of reproducible_results to check reproducibility status, rather than
    exit status of cmp or diff (suggested by arnout)
  - Rename results file (diffoscope_output.txt -> reproducible_results) to avoid confusion
  - Change handling of diffoscope output text file using with open()
  - Changed commit message to have all necessary info (suggested by arnout)
  - Removed leftover code from when I was using an exception to handle
    diffoscope presense (thanks to arnout)

Changes v1 -> v2:
  - move diffoscope output to results dir (suggested by arnout)
  - fix make printvars call
  - Add diffoscope to DEFAULT_OPTIONAL_PROGS
---
 scripts/autobuild-run | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 190a254..3f5af65 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,43 @@ 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")
+    reproducible_results = os.path.join(outputdir, "results", "reproducible_results")
+    # 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")
+
+    with open(reproducible_results, 'w') as diff:
+        if kwargs['sysinfo'].has("diffoscope"):
+            # 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]
+            log_write(log, "INFO: running diffoscope on images")
+            subprocess.call(["diffoscope", build_1_image, build_2_image,
+                                "--tool-prefix-binutils", prefix], stdout=diff, stderr=log)
+        else:
+            log_write(log, "INFO: diffoscope not installed, falling back to cmp")
+            subprocess.call(["cmp", "-b", build_1_image, build_2_image], stdout=diff, stderr=log)
+
+    if os.stat(reproducible_results).st_size > 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 v5 3/4] 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>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>

---
Changes v3 -> v4:
  - Remove spaces on empty lines (suggested by Yann)

Changes v2 -> v3:
  - make clean outputs to logfile, similar to other make calls (suggested by y_morin)
  - Output dir needs abspath (suggested by y_morin)
  - Directly use make clean as arguments rather than putting it into a variable
  - Added missing period at end of commit message (suggested by arnout)

Changes v1 -> v2:
  - make clean outputs to devnull (suggested by arnout)
  - remove unnecessary arguments to make clean (suggested by arnout)
---
 scripts/autobuild-run | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 3f5af65..339fbde 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -492,6 +492,43 @@ 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.abspath(os.path.join(idir, "output"))
+    srcdir = os.path.join(idir, "buildroot")
+    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
+    f = open(os.path.join(outputdir, "logfile"), "w+")
+    subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir, "clean"], stdout=f, stderr=f)
+
+    # 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 v5 4/4] autobuild-run: do reproducible builds tests if BR2_REPRODUCIBLE=y

[Atharva Lele]

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>

---
Changes v3 -> v4:
  - Fix indentation level for if-else, otherwise file still remains open
    for the duration of the build.

Changes v2 -> v3:
  - use reproducible flag so that .config is not kept open for the entire
    build duration. (suggested by arnout)
  - remove reduntant logging (suggested by arnout)

Changes v1 -> v2:
  - add trailing newline character to BR2_REPRODUCIBLE=y (suggested by arnout)
---
 scripts/autobuild-run | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 339fbde..601fb31 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -727,7 +727,14 @@ 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:
+            reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
+        if reproducible:
+            ret = do_reproducible_build(**kwargs)
+        else:
+            ret = do_build(**kwargs)
 
         send_results(ret, **kwargs)
 
-- 
2.20.1

[Buildroot] [PATCH v5 1/4] autobuild-run: move creation of result directory to run_instance()

[Thomas Petazzoni]

Hello,

On Mon, 17 Jun 2019 15:04:53 +0530
Atharva Lele <itsatharva@gmail.com> wrote:

> We need the result directory to be present when the check_reproducibility()
> function (implemented in the next patch) executes. As of now the results
> directory is created in the send_results() function which executes after
> check_reproducibility() will.
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>
> 
> ---
> Changes v4: Add new patch to series
> ---
>  scripts/autobuild-run | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

I have applied the series to buildroot-test, thanks!

One thing I was not a big fan of was PATCH 1/4, perhaps the
check_reproducibility() should have stored its result in some other
place, and keep the send_result() function to collect all the result
artifacts into the results/ folder. But since it's a minor detail and
we want to move forward with this, I applied as-is.

I have just restarted the Bootlin autobuilder machine with this new
version of autobuild-run, we'll see what happens :-)

Thanks!

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

[Buildroot] [PATCH v5 1/4] autobuild-run: move creation of result directory to run_instance()

[Thomas Petazzoni]

Hello Atharva,

On Mon, 17 Jun 2019 20:06:09 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> I have applied the series to buildroot-test, thanks!

So, the first results in the autobuilders with this applied have
appeared:

  http://autobuild.buildroot.net/results/0f3/0f35347a0d4b29fa7b657f42eb3d4d176ad75410/
  http://autobuild.buildroot.net/results/128/128675664dbce8f35e5a710e85791708263a92f8/

I see two problems:

 - The output in build-end.log does not show any indication that the
   overall build failure is caused by the build being non-reproducible.
   What build-end.log shows is just a "make legal-info" that ends up
   successfully, which is very confusing since the build status is
   "failure", but the log doesn't show any sort of failure.

   I think the check_reproducibility() function should at least show
   something in the build log.

 - The reason of the failures, visible at
   http://autobuild.buildroot.net/?submitter=Thomas+Petazzoni+(Bootlin+server)&status=NOK
   are just "unknown". We really want to have a better "reason". I
   guess the easiest here would be to move the calculation of the
   "reason" into autobuild-run instead of having it server-side, as
   Arnout has suggested in a separate discussion yesterday.

Best regards,

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

[Buildroot] [PATCH v5 1/4] autobuild-run: move creation of result directory to run_instance()

[Arnout Vandecappelle]

On 18/06/2019 09:14, Thomas Petazzoni wrote:
> Hello Atharva,
> 
> On Mon, 17 Jun 2019 20:06:09 +0200
> Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
> 
>> I have applied the series to buildroot-test, thanks!
> 
> So, the first results in the autobuilders with this applied have
> appeared:
> 
>   http://autobuild.buildroot.net/results/0f3/0f35347a0d4b29fa7b657f42eb3d4d176ad75410/
>   http://autobuild.buildroot.net/results/128/128675664dbce8f35e5a710e85791708263a92f8/
> 
> I see two problems:
> 
>  - The output in build-end.log does not show any indication that the
>    overall build failure is caused by the build being non-reproducible.
>    What build-end.log shows is just a "make legal-info" that ends up
>    successfully, which is very confusing since the build status is
>    "failure", but the log doesn't show any sort of failure.
> 
>    I think the check_reproducibility() function should at least show
>    something in the build log.

 You can see it through the existence of the "reproducible_results" file.

 But it would indeed be a good idea to use that file as the build log instead of
the normal log.

 Atharva?


>  - The reason of the failures, visible at
>    http://autobuild.buildroot.net/?submitter=Thomas+Petazzoni+(Bootlin+server)&status=NOK
>    are just "unknown". We really want to have a better "reason". I
>    guess the easiest here would be to move the calculation of the
>    "reason" into autobuild-run instead of having it server-side, as
>    Arnout has suggested in a separate discussion yesterday.

 Ack, that was the plan. The "unkown" is exactly what we expected. I think it's
OK for a few weeks.

 Regards,
 Arnout

[Buildroot] [PATCH v5 1/4] autobuild-run: move creation of result directory to run_instance()

[Thomas Petazzoni]

On Tue, 18 Jun 2019 09:34:54 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> >  - The output in build-end.log does not show any indication that the
> >    overall build failure is caused by the build being non-reproducible.
> >    What build-end.log shows is just a "make legal-info" that ends up
> >    successfully, which is very confusing since the build status is
> >    "failure", but the log doesn't show any sort of failure.
> > 
> >    I think the check_reproducibility() function should at least show
> >    something in the build log.  
> 
>  You can see it through the existence of the "reproducible_results" file.

Yes, I know, I have read the autobuild-run patches before applying
them :-)

Still the immediate thing you click on when you see a failure is the
build log, and when there's no apparent failure it's confusing.

>  But it would indeed be a good idea to use that file as the build log instead of
> the normal log.

Correct.

> >  - The reason of the failures, visible at
> >    http://autobuild.buildroot.net/?submitter=Thomas+Petazzoni+(Bootlin+server)&status=NOK
> >    are just "unknown". We really want to have a better "reason". I
> >    guess the easiest here would be to move the calculation of the
> >    "reason" into autobuild-run instead of having it server-side, as
> >    Arnout has suggested in a separate discussion yesterday.  
> 
>  Ack, that was the plan. The "unkown" is exactly what we expected. I think it's
> OK for a few weeks.

Yes, it is OK for now, but it would be good to work on fixing this.

Best regards,

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

[Buildroot] [PATCH v5 1/4] autobuild-run: move creation of result directory to run_instance()

[Atharva Lele]

On Tue, Jun 18, 2019 at 1:04 PM Arnout Vandecappelle <arnout@mind.be> wrote:

>
>  But it would indeed be a good idea to use that file as the build log
> instead of
> the normal log.
>
>  Atharva?
>

Yes, that'll be what I work on once I finish the builder-class. Is that OK?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190618/07e844e8/attachment.html>