Patchwork [core,2/2] baker.py: only print if there is actually a prebaked file available

login
register
mail settings
Submitter kim.hansen@prevas.dk
Date Nov. 21, 2013, 9:30 a.m.
Message ID <1385026202-2177-1-git-send-email-kim.hansen@prevas.dk>
Download mbox | patch
Permalink /patch/293129/
State Under Review
Delegated to: Esben Haabendal
Headers show

Comments

kim.hansen@prevas.dk - Nov. 21, 2013, 9:30 a.m.
From: Kim Højgaard-Hansen <kiho@prevas.dk>

---
 lib/oelite/baker.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Esben Haabendal - Nov. 27, 2013, 1:57 p.m.
<kim.hansen@prevas.dk> writes:

> From: Kim Højgaard-Hansen <kiho@prevas.dk>
>
> ---
>  lib/oelite/baker.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/oelite/baker.py b/lib/oelite/baker.py
> index dfb99d2..dbe1663 100644
> --- a/lib/oelite/baker.py
> +++ b/lib/oelite/baker.py
> @@ -487,7 +487,8 @@ class OEliteBaker:
>                  deploy_dir, package.type,
>                  package.arch + (package.recipe.meta.get("EXTRA_ARCH") or ""),
>                  "%s_%s_%s.tar"%(package.name, recipe.version, buildhash))
> -            debug("will use from build: %s"%(filename))
> +            if(os.path.exists(filename)):
> +                debug("will use from build: %s"%(filename))
>              self.runq.set_package_filename(package.id, filename)
>  
>          self.runq.mark_primary_runq_depends()

Hmm... I don't think it makes much sense to check for the existance of
the file at this stage. We haven't actually run any tasks yet, so even
if the file exists at this point, we might or might not actually use the
file in the build (or we might end up rebuilding it before using it).

Or what am I not seeing here?

/Esben
Kim Højgaard-Hansen - Nov. 28, 2013, 9:23 a.m.
On 27/11/13 14:57, Esben Haabendal wrote:
> <kim.hansen@prevas.dk> writes:
>
>> From: Kim Højgaard-Hansen <kiho@prevas.dk>
>>
>> ---
>>   lib/oelite/baker.py | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/oelite/baker.py b/lib/oelite/baker.py
>> index dfb99d2..dbe1663 100644
>> --- a/lib/oelite/baker.py
>> +++ b/lib/oelite/baker.py
>> @@ -487,7 +487,8 @@ class OEliteBaker:
>>                   deploy_dir, package.type,
>>                   package.arch + (package.recipe.meta.get("EXTRA_ARCH") or ""),
>>                   "%s_%s_%s.tar"%(package.name, recipe.version, buildhash))
>> -            debug("will use from build: %s"%(filename))
>> +            if(os.path.exists(filename)):
>> +                debug("will use from build: %s"%(filename))
>>               self.runq.set_package_filename(package.id, filename)
>>   
>>           self.runq.mark_primary_runq_depends()
> Hmm... I don't think it makes much sense to check for the existance of
> the file at this stage. We haven't actually run any tasks yet, so even
> if the file exists at this point, we might or might not actually use the
> file in the build (or we might end up rebuilding it before using it).
>
> Or what am I not seeing here?
>
> /Esben
>
currently, when running with debug output (which I prefer to easily 
follow what is going on) you will get a lot of messages like "Will use 
build from ..." even if have deleted tmp completely. That is quite 
confusing :) So only to avoid that

/Kim
Esben Haabendal - Nov. 28, 2013, 1:11 p.m.
Kim Højgaard-Hansen <kiho@prevas.dk> writes:

> On 27/11/13 14:57, Esben Haabendal wrote:
>
>     <kim.hansen@prevas.dk> writes:
>
>
>         From: Kim Højgaard-Hansen <kiho@prevas.dk>
>
>         ---
>          lib/oelite/baker.py | 3 ++-
>          1 file changed, 2 insertions(+), 1 deletion(-)
>
>         diff --git a/lib/oelite/baker.py b/lib/oelite/baker.py
>         index dfb99d2..dbe1663 100644
>         --- a/lib/oelite/baker.py
>         +++ b/lib/oelite/baker.py
>         @@ -487,7 +487,8 @@ class OEliteBaker:
>                          deploy_dir, package.type,
>                          package.arch + (package.recipe.meta.get("EXTRA_ARCH") or ""),
>                          "%s_%s_%s.tar"%(package.name, recipe.version, buildhash))
>         -            debug("will use from build: %s"%(filename))
>         +            if(os.path.exists(filename)):
>         +                debug("will use from build: %s"%(filename))
>                      self.runq.set_package_filename(package.id, filename)
>
>                  self.runq.mark_primary_runq_depends()
>
>     Hmm... I don't think it makes much sense to check for the existance of
>     the file at this stage. We haven't actually run any tasks yet, so even
>     if the file exists at this point, we might or might not actually use the
>     file in the build (or we might end up rebuilding it before using it).
>
>     Or what am I not seeing here?
>
>     /Esben
>
>
> currently, when running with debug output (which I prefer to easily
> follow what is going on) you will get a lot of messages like "Will use
> build from ..." even if have deleted tmp completely. That is quite
> confusing :) So only to avoid that

Will it does not say "will use build from ...", but rather "will use
from build: ...".  The message is that it will use the package from the
build you are about to start.  If the file exists in advance, it will be
overwritten before it is used.  And as such, the check does not make
sense.

But clearly, a better wording would make sense.

/Esben

Patch

diff --git a/lib/oelite/baker.py b/lib/oelite/baker.py
index dfb99d2..dbe1663 100644
--- a/lib/oelite/baker.py
+++ b/lib/oelite/baker.py
@@ -487,7 +487,8 @@  class OEliteBaker:
                 deploy_dir, package.type,
                 package.arch + (package.recipe.meta.get("EXTRA_ARCH") or ""),
                 "%s_%s_%s.tar"%(package.name, recipe.version, buildhash))
-            debug("will use from build: %s"%(filename))
+            if(os.path.exists(filename)):
+                debug("will use from build: %s"%(filename))
             self.runq.set_package_filename(package.id, filename)
 
         self.runq.mark_primary_runq_depends()