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