Message ID | 20190611123416.11533-2-itsatharva@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [v3,1/3] autobuild-run: initial implementation of check_reproducibility() | expand |
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 >
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
> 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 > >
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