[yocto] [PATCHv2 09/11][auh] upgradehelper.py: Change policy for send emails and fix error passing

[Paul Eggleton]

Hi Aníbal,

On Friday 12 June 2015 20:10:45 Aníbal Limón wrote:
> Now send emails almost in all cases this give the maintainer
> patches and diff to continue work also if the build isn't
> succesful.
> 
> [YOCTO #7489]
> 
> Signed-off-by: Aníbal Limón <anibal.limon at linux.intel.com>
> ---
>  upgradehelper.py | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/upgradehelper.py b/upgradehelper.py
> index b1f075d..d065fba 100755
> --- a/upgradehelper.py
> +++ b/upgradehelper.py
> @@ -346,8 +346,6 @@ class Updater(object):
>                  self.git.clean_untracked()
>                  return
> 
> -        status = type(err).__name__
> -
>          # drop last upgrade from git. It's safer this way if the upgrade
> has # problems and other recipes depend on it. Give the other recipes a #
> chance...
> @@ -381,8 +379,14 @@ class Updater(object):
>                  "Attached are the patch, license diff (if change) and
> bitbake log.\n\n" \ "Regards,\nThe Upgrade Helper"
> 
> -            # don't bother maintainer with mail if the recipe is already up
> to date -            if status == "UpgradeNotNeededError":
> +            # if error only send email when useful infomration for
> maintainers exist +            if err and not (isinstance(err, PatchError)
> or \
> +               isinstance(err, ConfigureError) or \
> +               isinstance(err, CompilationError) or \
> +               isinstance(err, LicenseError)):
> +                D( "%s: Don't send email to maintainer because the error
> was " \ +                   "%s and the information isn't useful, please
> review it." \ +                    % (self.pn, type(err).__name__))
>                  return

I think a better approach here would be to have the classes of error that are 
likely to be fixable by the maintainer as inheriting from a particular class 
(e.g. MaintainerError) and then we can just check for that rather than having 
to extend this code every time we add a new type of error.

>              if self.maintainer in maintainer_override:
> @@ -478,6 +482,7 @@ class Updater(object):
> 
>          attempted_pkgs = 0
>          for self.pn, self.new_ver, self.maintainer in pkgs_to_upgrade:
> +            error = None
>              self.recipe = None
>              attempted_pkgs += 1
>              I(" ATTEMPT PACKAGE %d/%d" % (attempted_pkgs, total_pkgs))
> @@ -489,10 +494,6 @@ class Updater(object):
>                      step()
> 
>                  I(" %s: Upgrade SUCCESSFUL! Please test!" % self.pn)
> -                error = None
> -            except UpgradeNotNeededError as e:
> -                I(" %s: %s" % (self.pn, e.message))
> -                error = e

I'm confused by this. Won't this change result in UpgradeNotNeededError being 
treated as an actual error? Surely we don't actually want that?

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre