diff mbox series

[v3,3/3] autobuild-run: do reproducible builds tests if BR2_REPRODUCIBLE=y

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

Commit Message

Atharva Lele June 11, 2019, 12:34 p.m. UTC 
Signed-off-by: Atharva Lele <itsatharva@gmail.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---
Changes v2 -> v3:
  - use reproducible flag so that .config i snot 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)

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

Comments

Yann E. MORIN June 13, 2019, 8:23 p.m. UTC | #1 
On 2019-06-11 18:04 +0530, Atharva Lele spake thusly:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> ---
> Changes v2 -> v3:
>   - use reproducible flag so that .config i snot kept open for the entire
>     build duration. (suggested by arnout)

Sorry, but that is still the case, see below...

>   - remove reduntant logging (suggested by arnout)
> 
> 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 | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 5ba6092..4fbd507 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)

This whole if reproducible block is still indented inside the with
block, so you'r still having .config opened during the whole build.

You need to do (basically):

    with open(os.path.join(outputdir, ".config"), "r") as fconf:
        reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
    build = do_reproducible_build if reproducible else do_build
    ret = build(**kwargs)

Regards,
Yann E. MORIN.

>  
>          send_results(ret, **kwargs)
>  
> -- 
> 2.20.1
>
Atharva Lele June 14, 2019, 8:46 a.m. UTC | #2 
> You need to do (basically):

>    with open(os.path.join(outputdir, ".config"), "r") as fconf:
>        reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
>    build = do_reproducible_build if reproducible else do_build
>    ret = build(**kwargs)

It would still work if I just unindent the if-else block too right? This is
what I actually intended to do
It's working for me in testing..
Like this:

    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)

Regards,
Atharva Lele


On Fri, Jun 14, 2019 at 1:53 AM Yann E. MORIN <yann.morin.1998@free.fr>
wrote:

> On 2019-06-11 18:04 +0530, Atharva Lele spake thusly:
> > Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> >
> > ---
> > Changes v2 -> v3:
> >   - use reproducible flag so that .config i snot kept open for the entire
> >     build duration. (suggested by arnout)
>
> Sorry, but that is still the case, see below...
>
> >   - remove reduntant logging (suggested by arnout)
> >
> > 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 | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> > index 5ba6092..4fbd507 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)
>
> This whole if reproducible block is still indented inside the with
> block, so you'r still having .config opened during the whole build.
>
> You need to do (basically):
>
>     with open(os.path.join(outputdir, ".config"), "r") as fconf:
>         reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
>     build = do_reproducible_build if reproducible else do_build
>     ret = build(**kwargs)
>
> Regards,
> Yann E. MORIN.
>
> >
> >          send_results(ret, **kwargs)
> >
> > --
> > 2.20.1
> >
>
> --
>
> .-----------------.--------------------.------------------.--------------------.
> |  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.  |
>
> '------------------------------^-------^------------------^--------------------'
>
Arnout Vandecappelle June 14, 2019, 8:47 a.m. UTC | #3 
On 14/06/2019 10:46, Atharva Lele wrote:
>> You need to do (basically):
> 
>>    with open(os.path.join(outputdir, ".config"), "r") as fconf:
>>        reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
>>    build = do_reproducible_build if reproducible else do_build
>>    ret = build(**kwargs)
> 
> It would still work if I just unindent the if-else block too right? This is what
> I actually intended to do
> It's working for me in testing..
> Like this:
> 
>     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)

 Yes, and I prefer that version. Yann considers the '... if ... else ...'
construct more pythonese, but I kind of like explicit conditions :-)

 Regards,
 Arnout


[snip]
Atharva Lele June 14, 2019, 8:53 a.m. UTC | #4 
> Yann considers the '... if ... else ...' construct more pythonese,
> but I kind of like explicit conditions :-)

Haha I do see his point of view. For me, I've only worked with C for the
last 2 years at university and my last internship so it's a bit hard to
'talk' in
pythonese.

Regards,
Atharva Lele


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

>
>
> On 14/06/2019 10:46, Atharva Lele wrote:
> >> You need to do (basically):
> >
> >>    with open(os.path.join(outputdir, ".config"), "r") as fconf:
> >>        reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
> >>    build = do_reproducible_build if reproducible else do_build
> >>    ret = build(**kwargs)
> >
> > It would still work if I just unindent the if-else block too right? This
> is what
> > I actually intended to do
> > It's working for me in testing..
> > Like this:
> >
> >     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)
>
>  Yes, and I prefer that version. Yann considers the '... if ... else ...'
> construct more pythonese, but I kind of like explicit conditions :-)
>
>  Regards,
>  Arnout
>
>
> [snip]
>
Yann E. MORIN June 14, 2019, 8:35 p.m. UTC | #5 
Arnout, All,

On 2019-06-14 10:47 +0200, Arnout Vandecappelle spake thusly:
> On 14/06/2019 10:46, Atharva Lele wrote:
> >> You need to do (basically):
> > 
> >>    with open(os.path.join(outputdir, ".config"), "r") as fconf:
> >>        reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
> >>    build = do_reproducible_build if reproducible else do_build
> >>    ret = build(**kwargs)
> > 
> > It would still work if I just unindent the if-else block too right? This is what
> > I actually intended to do
> > It's working for me in testing..
> > Like this:
> > 
> >     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)
> 
>  Yes, and I prefer that version. Yann considers the '... if ... else ...'
> construct more pythonese, but I kind of like explicit conditions :-)

Well, if we use a language, we should write with the idioms of that
language. Python is not C, so we should not write Python as we would C.

That notwithstanding, I'm no Python expert either, so I'm also OK with
the more "traditional and conventional" ways. ;-)

Regards,
Yann E. MORIN.
diff mbox series

Patch

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