[v3,2/3] autobuild-run: initial implementation of do_reproducible_build()
diff mbox series

Message ID 20190611123416.11533-2-itsatharva@gmail.com
State Superseded
Headers show
Series
  • [v3,1/3] autobuild-run: initial implementation of check_reproducibility()
Related show

Commit Message

Atharva Lele June 11, 2019, 12:34 p.m. UTC
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>

---
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)

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

Comments

Yann E. MORIN June 13, 2019, 8:13 p.m. UTC | #1
Atharva, All,

On 2019-06-11 18:04 +0530, Atharva Lele spake thusly:
> 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>

Yet, a few comments, see below...

> ---
> 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)
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> ---
>  scripts/autobuild-run | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index ba5b337..5ba6092 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']
> +    

Empty line made of spaces.

> +    # 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
> +    

Ditto.

> +    # 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)

Since we do create a small Makefile wrapper in $(O), you could simplify
this a little with:

    subprocess.call(["make", "-C", outputdir, "clean"], stdout=f, stderr=f)

Ditto in the existing do_build() function. Can be improved upon when you
introduce the Builder class.

Regards,
Yann E. MORIN.

> +    # 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
>
Arnout Vandecappelle June 14, 2019, 7:58 a.m. UTC | #2
On 13/06/2019 22:13, Yann E. MORIN wrote:
> Atharva, All,
> 
> On 2019-06-11 18:04 +0530, Atharva Lele spake thusly:
[snip]
>> +    # 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)
> 
> Since we do create a small Makefile wrapper in $(O), you could simplify
> this a little with:
> 
>     subprocess.call(["make", "-C", outputdir, "clean"], stdout=f, stderr=f)
> 
> Ditto in the existing do_build() function. Can be improved upon when you
> introduce the Builder class.

 This is bikeshedding, obviously, but I don't like that proposal. I think an
explicit O= -C is easier to read and understand.

 Shorter is not better :-)

 Regards,
 Arnout
Atharva Lele June 14, 2019, 8:47 a.m. UTC | #3
> I think an explicit O= -C is easier to read and understand.

I do agree with this.

Regards,
Atharva Lele


On Fri, Jun 14, 2019 at 1:28 PM Arnout Vandecappelle <arnout@mind.be> wrote:

>
>
> On 13/06/2019 22:13, Yann E. MORIN wrote:
> > Atharva, All,
> >
> > On 2019-06-11 18:04 +0530, Atharva Lele spake thusly:
> [snip]
> >> +    # 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)
> >
> > Since we do create a small Makefile wrapper in $(O), you could simplify
> > this a little with:
> >
> >     subprocess.call(["make", "-C", outputdir, "clean"], stdout=f,
> stderr=f)
> >
> > Ditto in the existing do_build() function. Can be improved upon when you
> > introduce the Builder class.
>
>  This is bikeshedding, obviously, but I don't like that proposal. I think
> an
> explicit O= -C is easier to read and understand.
>
>  Shorter is not better :-)
>
>  Regards,
>  Arnout
>
>

Patch
diff mbox series

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index ba5b337..5ba6092 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