[Buildroot] [PATCH buildroot-test 0/4] add support for outdated packages

[Buildroot] [PATCH buildroot-test 0/4] add support for outdated packages

[Victor Huesca]

This patch provide a new category in daily-mails in addition to build-results: outdated
packages.

The work of adding support for json output in buildroot's `pkg-stats` allow to reuse its
informations in other scripts. The commit add support for extracting packages version and
informs maintainers about packages they maintains that have a newer version.

This patch also improve cli by adding multiple options (see the last commit) since we may
not want to always include the 'outdated packages' notification.

Victor Huesca (4):
  utils/daily-mail: add information about version that may need an
    update
  utils/daily-mail: allow to print a header for results section
  utils/daily-mail: add some debug information for the Notification
    class
  utils/daily-mail: add multiple argument to ease modularity of the
    script

 utils/daily-mail | 185 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 154 insertions(+), 31 deletions(-)

-- 
2.21.0

[Buildroot] [PATCH buildroot-test 1/4] utils/daily-mail: add information about version that may need an update

[Victor Huesca]

This patch use the output of buildroot pkg-stats to inform maintainers about packages
they maintain that may need an update.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 96 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 83 insertions(+), 13 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index cf600c5..4ab7710 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -11,10 +11,15 @@ from datetime import date, timedelta
 import localconfig
 import csv
 from collections import defaultdict
+import json
+from packaging import version
+import re
 
 sys.path.append(os.path.join(localconfig.brbase, "utils"))
 import getdeveloperlib
 
+RE_HASH_40 = re.compile(r'.*[a-fA-F0-9]{40}.*')
+
 baseurl = "autobuild.buildroot.net"
 http_baseurl = "http://" + baseurl
 
@@ -59,6 +64,7 @@ class Notification:
     def __init__(self):
         self.arch_notifications = defaultdict(list)
         self.package_notifications = defaultdict(list)
+        self.package_version_notification = []
 
 # Calculate the list of .mk files in the Buildroot source tree, will
 # be used to guess the name of the packages that caused build
@@ -139,6 +145,19 @@ def add_package_notification(branch, notifications, build_result):
         n.package_notifications[branch].append(build_result)
     build_result['orphan'] = orphan
 
+def add_outdated_pkg_notification(notifications, outdated_pkg):
+    for pkg in outdated_pkg:
+        orphan = True
+        for dev in developers:
+            if pkg['name'] in dev.packages:
+                orphan = False
+                notif = get_notification_for_dev(notifications, dev)
+                notif.package_version_notification.append(pkg)
+        if orphan:
+            pkg['orphan'] = True
+            notif = get_notification_for_dev(notifications, get_orphan_developer())
+            notif.package_version_notification.append(pkg)
+
 def show_results(results, show_status, show_orphan=False):
     contents = ""
     for r in results:
@@ -157,33 +176,55 @@ def show_results(results, show_status, show_orphan=False):
             orphan_str = ""
         url = http_baseurl + "/results/" + r['identifier']
         if show_status:
-            contents += "%12s | %30s | %3s | %40s" % (arch, reason, status_str, url)
+            contents += "%12s | %30s | %3s | %79s" % (arch, reason, status_str, url)
         else:
-            contents += "%12s | %30s | %40s" % (arch, reason, url)
+            contents += "%12s | %30s | %79s" % (arch, reason, url)
         if show_orphan:
             contents += " | %4s\n" % (orphan_str)
         else:
             contents += "\n"
     return contents
 
+def show_outdated(packages, show_orphan=False, show_header=False):
+    contents = ''
+    if show_header:
+        contents += '{:^30} | {:^8} | {:^44} | {:^12} | {:^12}'.format('name', 'found by', 
+                                                                     'link to release-moinotring.org', 
+                                                                     'version', 'upstream')
+        if show_orphan:
+            contents += ' | {:^5}'.format('orph?')
+        contents += '\n{0:-^30}-+-{0:-^8}-+-{0:-^44}-+-{0:-^12}-+-{0:-^12}-'.format('')
+        if show_orphan:
+            contents += '+-{:-^5}-'.format('')
+        contents += '\n'
+
+    for pkg in packages:
+        pkg_str = { k: (v[:9] + '...' if k in ('version', 'upstream') and len(v) > 12 else v) for k, v in pkg.items() }
+        orphan_str = 'ORHP' if 'orphan' in pkg and pkg['orphan'] else ''
+        contents += '{name:>30} | {from:^8} | https://release-monitoring.org/project/{id:0>5} | {version:12} | {upstream:12}'.format(**pkg_str)
+        if show_orphan:
+            contents += ' | {:4}'.format(orphan_str)
+        contents += '\n'
+    return contents
+
 # Send the e-mails to the individual developers
 def developers_email(smtp, branches, notifications, datestr, dry_run):
-    for k, v in notifications.iteritems():
-        to = k.name
+    for dev, notif in notifications.iteritems():
+        to = dev.name
         email_from = localconfig.fromaddr
         subject = "[%s] Your build results for %s" % (baseurl, datestr)
         contents = "Hello,\n\n"
         contents += textwrap.fill("This is the list of Buildroot build failures that occured on %s, and for which you are a registered architecture developer or package developer. Please help us improving the quality of Buildroot by investigating those build failures and sending patches to fix them. Thanks!" % datestr)
         contents += "\n\n"
-        show_orphan = k.name == ORPHAN_DEVELOPER
+        show_orphan = dev.name == ORPHAN_DEVELOPER
 
         for branch in branches:
-            if v.arch_notifications.has_key(branch):
-                archs = v.arch_notifications[branch]
+            if notif.arch_notifications.has_key(branch):
+                archs = notif.arch_notifications[branch]
             else:
                 archs = []
-            if v.package_notifications.has_key(branch):
-                packages = v.package_notifications[branch]
+            if notif.package_notifications.has_key(branch):
+                packages = notif.package_notifications[branch]
             else:
                 packages = []
 
@@ -203,6 +244,12 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
 
             contents += "\n"
 
+        outdated = notif.package_version_notification
+        if len(outdated) != 0:
+            contents += "Packages you maintain that have a new upstream version:\n\n"
+            contents += show_outdated(outdated, show_orphan=show_orphan)
+            contents += '\n'
+
         contents += "-- \n"
         contents += http_baseurl
         if dry_run:
@@ -220,7 +267,7 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
             msg['From'] = email_from
             msg['Date'] = formatdate()
             smtp.sendmail(email_from, to, msg.as_string())
-            print "To: %s" % k.name
+            print "To: %s" % dev.name
 
 def global_email_branch_result(results, results_by_reason, branch):
     contents = "Results for branch '%s'\n" % branch
@@ -241,7 +288,7 @@ def global_email_branch_result(results, results_by_reason, branch):
     return contents
 
 # Send the global e-mail to the mailing list
-def global_email(smtp, results, results_by_reason, datestr, overall, dry_run):
+def global_email(smtp, results, results_by_reason, datestr, overall, dry_run, outdated=None):
     to = "buildroot at buildroot.org"
     email_from = localconfig.fromaddr
     subject = "[%s] Build results for %s" % (baseurl, datestr)
@@ -260,6 +307,11 @@ def global_email(smtp, results, results_by_reason, datestr, overall, dry_run):
         if len(results[branch]) == 0:
             continue
         contents += global_email_branch_result(results[branch], results_by_reason[branch], branch)
+    if outdated:
+        contents += '\n\n'
+        contents += "Packages having a newer version\n"
+        contents += "===============================\n\n"
+        contents += show_outdated(outdated, show_orphan=True)    
     contents += "\n"
     contents += "-- \n"
     contents += http_baseurl
@@ -304,7 +356,7 @@ def get_build_results_grouped_by_reason(db, datestr, branches):
 # Prepare the notifications{} dict for the notifications to individual
 # developers, based on architecture developers and package
 # developers
-def calculate_notifications(results):
+def calculate_notifications(results, outdated_pkg):
     notifications = {}
     for branch in results.keys():
         for result in results[branch]:
@@ -313,8 +365,25 @@ def calculate_notifications(results):
                 continue
             add_arch_notification(branch, notifications, result)
             add_package_notification(branch, notifications, result)
+    add_outdated_pkg_notification(notifications, outdated_pkg)
     return notifications
 
+def get_outdated_pkg(path):
+    with open(path, 'r') as f:
+        stats = json.load(f)
+    s = []
+    for name, pkg in stats['packages'].items():
+        status, latest_ver, p_id = pkg['latest_version']
+        cur_ver = pkg['current_version']
+        if  status in (2, 3) and cur_ver and latest_ver and not RE_HASH_40.match(cur_ver) \
+                             and version.parse(str(cur_ver)) < version.parse(str(latest_ver)):
+            s.append( { 'name': str(name), 
+                        'id': p_id, 
+                        'version': str(cur_ver), 
+                        'upstream': str(latest_ver),
+                        'from': 'MAPPING' if status == 2 else 'NAME' } )
+    return sorted(s, key=lambda pkg: pkg['name'])
+
 def __main__():
     yesterday = date.today() - timedelta(1)
     yesterday_str = yesterday.strftime('%Y-%m-%d')
@@ -327,7 +396,8 @@ def __main__():
     overall_stats = get_overall_stats(db, yesterday_str, branches)
     results = get_build_results(db, yesterday_str, branches)
     results_by_reason = get_build_results_grouped_by_reason(db, yesterday_str, branches)
-    notifications = calculate_notifications(results)
+    outdated_pkg = get_outdated_pkg(localconfig.pkg_stats)
+    notifications = calculate_notifications(results, outdated_pkg)
     dry_run = False
     if len(sys.argv) == 2 and sys.argv[1] == "--dry-run":
         dry_run = True
-- 
2.21.0

[Buildroot] [PATCH buildroot-test 1/4] utils/daily-mail: add information about version that may need an update

[Thomas Petazzoni]

On Mon,  8 Jul 2019 10:14:03 +0200
Victor Huesca <victor.huesca@bootlin.com> wrote:

>  def show_results(results, show_status, show_orphan=False):
>      contents = ""
>      for r in results:
> @@ -157,33 +176,55 @@ def show_results(results, show_status, show_orphan=False):
>              orphan_str = ""
>          url = http_baseurl + "/results/" + r['identifier']
>          if show_status:
> -            contents += "%12s | %30s | %3s | %40s" % (arch, reason, status_str, url)
> +            contents += "%12s | %30s | %3s | %79s" % (arch, reason, status_str, url)
>          else:
> -            contents += "%12s | %30s | %40s" % (arch, reason, url)
> +            contents += "%12s | %30s | %79s" % (arch, reason, url)

These two changes are not related to this commit.

>          if show_orphan:
>              contents += " | %4s\n" % (orphan_str)
>          else:
>              contents += "\n"
>      return contents
>  
> +def show_outdated(packages, show_orphan=False, show_header=False):

To be honest, I'm not so sure about all the show_header stuff. Just
make it unconditional, i.e show headers all the time. I don't think the
extra complexity is really worth it.

> +    contents = ''
> +    if show_header:
> +        contents += '{:^30} | {:^8} | {:^44} | {:^12} | {:^12}'.format('name', 'found by', 
> +                                                                     'link to release-moinotring.org', 

Typo on "link to release-moinotring.org".

> +                                                                     'version', 'upstream')
> +        if show_orphan:
> +            contents += ' | {:^5}'.format('orph?')
> +        contents += '\n{0:-^30}-+-{0:-^8}-+-{0:-^44}-+-{0:-^12}-+-{0:-^12}-'.format('')
> +        if show_orphan:
> +            contents += '+-{:-^5}-'.format('')
> +        contents += '\n'

For consistency with the rest of the code, you should use "%12s" style,
or have a preparation commit that converts the existing code to
the .format() thing. But mixing both is not nice.

> +    for pkg in packages:
> +        pkg_str = { k: (v[:9] + '...' if k in ('version', 'upstream') and len(v) > 12 else v) for k, v in pkg.items() }

Meh, that's a bit quirky. It at least deserves a comment I believe.

> +        orphan_str = 'ORHP' if 'orphan' in pkg and pkg['orphan'] else ''
> +        contents += '{name:>30} | {from:^8} | https://release-monitoring.org/project/{id:0>5} | {version:12} | {upstream:12}'.format(**pkg_str)
> +        if show_orphan:
> +            contents += ' | {:4}'.format(orphan_str)
> +        contents += '\n'
> +    return contents
> +
>  # Send the e-mails to the individual developers
>  def developers_email(smtp, branches, notifications, datestr, dry_run):
> -    for k, v in notifications.iteritems():
> -        to = k.name
> +    for dev, notif in notifications.iteritems():

The change from k, v to dev, notif should be in a separate commit. It's
a good change, but it's unrelated.

> +        to = dev.name
>          email_from = localconfig.fromaddr
>          subject = "[%s] Your build results for %s" % (baseurl, datestr)
>          contents = "Hello,\n\n"
>          contents += textwrap.fill("This is the list of Buildroot build failures that occured on %s, and for which you are a registered architecture developer or package developer. Please help us improving the quality of Buildroot by investigating those build failures and sending patches to fix them. Thanks!" % datestr)

We will probably want to change this text, because the e-mail will no
longer be just about build failures.

>          contents += "\n\n"
> -        show_orphan = k.name == ORPHAN_DEVELOPER
> +        show_orphan = dev.name == ORPHAN_DEVELOPER
>  
>          for branch in branches:
> -            if v.arch_notifications.has_key(branch):
> -                archs = v.arch_notifications[branch]
> +            if notif.arch_notifications.has_key(branch):
> +                archs = notif.arch_notifications[branch]
>              else:
>                  archs = []
> -            if v.package_notifications.has_key(branch):
> -                packages = v.package_notifications[branch]
> +            if notif.package_notifications.has_key(branch):
> +                packages = notif.package_notifications[branch]

All these changes are unrelated to the outdated package change, they
are related to the k,v -> dev,notif change, which should be in a
separate commit.

>              else:
>                  packages = []
>  
> @@ -203,6 +244,12 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
>  
>              contents += "\n"
>  
> +        outdated = notif.package_version_notification

I think we should call this notif.outdated_package_notifications

> +        if len(outdated) != 0:
> +            contents += "Packages you maintain that have a new upstream version:\n\n"
> +            contents += show_outdated(outdated, show_orphan=show_orphan)
> +            contents += '\n'
> +
>          contents += "-- \n"
>          contents += http_baseurl
>          if dry_run:
> @@ -220,7 +267,7 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
>              msg['From'] = email_from
>              msg['Date'] = formatdate()
>              smtp.sendmail(email_from, to, msg.as_string())
> -            print "To: %s" % k.name
> +            print "To: %s" % dev.name

Same as above: should be in a separate commit.

> +def get_outdated_pkg(path):
> +    with open(path, 'r') as f:
> +        stats = json.load(f)
> +    s = []
> +    for name, pkg in stats['packages'].items():
> +        status, latest_ver, p_id = pkg['latest_version']
> +        cur_ver = pkg['current_version']
> +        if  status in (2, 3) and cur_ver and latest_ver and not RE_HASH_40.match(cur_ver) \
> +                             and version.parse(str(cur_ver)) < version.parse(str(latest_ver)):

Can we do it like this:

		if status not in (2, 3):
			continue

		if cur_ver is None or latest_ver is None:
			continue

		if RE_HASH_40.match(cur_ver):
			continue

		if version.parse(str(cur_ver)) >= version.parse(str(latest_ver)):
			continue

and then append to the list.

>  def __main__():
>      yesterday = date.today() - timedelta(1)
>      yesterday_str = yesterday.strftime('%Y-%m-%d')
> @@ -327,7 +396,8 @@ def __main__():
>      overall_stats = get_overall_stats(db, yesterday_str, branches)
>      results = get_build_results(db, yesterday_str, branches)
>      results_by_reason = get_build_results_grouped_by_reason(db, yesterday_str, branches)
> -    notifications = calculate_notifications(results)
> +    outdated_pkg = get_outdated_pkg(localconfig.pkg_stats)
> +    notifications = calculate_notifications(results, outdated_pkg)

The new notifications don't get used anywhere, because global_email()
is never passed the outdated package details. This is due to the patch
ordering issue. Once you fix the patch ordering/organization problem,
you'll be able to add the outdated package support in one patch
properly.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH buildroot-test 2/4] utils/daily-mail: allow to print a header for results section

[Victor Huesca]

This table quite clear without header but as I had a template for the outdated
table I also added the possibility to include one for results too.

Note that this commit only add the option to print the header but does not
enable it, so the existing behavior is preserved.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index 4ab7710..b4d0a56 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -158,8 +158,25 @@ def add_outdated_pkg_notification(notifications, outdated_pkg):
             notif = get_notification_for_dev(notifications, get_orphan_developer())
             notif.package_version_notification.append(pkg)
 
-def show_results(results, show_status, show_orphan=False):
+
+def show_results(results, show_status, show_orphan=False, show_header=False):
     contents = ""
+    if show_header:
+        if show_status:
+            contents += '{:^12} | {:^30} | {:^3} | {:^79}'.format('arch', 'reason', 'OK?', 'url')
+        else:
+            contents += '{:^12} | {:^30} | {:^79}'.format('arch', 'reason', 'url')
+        if show_orphan:
+            contents += ' | {:^5}'.format('orph?')
+
+        if show_status:
+            contents += '\n{0:-^12}-+-{0:-^30}-+-{0:-^3}-+-{0:-^79}-'.format('')
+        else:
+            contents += '\n{0:-^12}-+-{0:-^30}-+-{0:-^79}-'.format('')
+        if show_orphan:
+            contents += '+-{:-^5}-'.format('')
+        contents += '\n'
+
     for r in results:
         arch = r['arch']
         reason = r['reason']
-- 
2.21.0

[Buildroot] [PATCH buildroot-test 2/4] utils/daily-mail: allow to print a header for results section

[Thomas Petazzoni]

Hello,

On Mon,  8 Jul 2019 10:14:04 +0200
Victor Huesca <victor.huesca@bootlin.com> wrote:

> This table quite clear without header but as I had a template for the outdated
> table I also added the possibility to include one for results too.
> 
> Note that this commit only add the option to print the header but does not
> enable it, so the existing behavior is preserved.
> 
> Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
> ---
>  utils/daily-mail | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/daily-mail b/utils/daily-mail
> index 4ab7710..b4d0a56 100755
> --- a/utils/daily-mail
> +++ b/utils/daily-mail
> @@ -158,8 +158,25 @@ def add_outdated_pkg_notification(notifications, outdated_pkg):
>              notif = get_notification_for_dev(notifications, get_orphan_developer())
>              notif.package_version_notification.append(pkg)
>  
> -def show_results(results, show_status, show_orphan=False):
> +
> +def show_results(results, show_status, show_orphan=False, show_header=False):

As I replied in my review of PATCH 1/4, please make this unconditional.
And this patch should come *before* the outdated package feature.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH buildroot-test 3/4] utils/daily-mail: add some debug information for the Notification class

[Victor Huesca]

It was useful to me when working on adding support for new notification
so it may help someone else.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/utils/daily-mail b/utils/daily-mail
index b4d0a56..85a1c4e 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -65,6 +65,18 @@ class Notification:
         self.arch_notifications = defaultdict(list)
         self.package_notifications = defaultdict(list)
         self.package_version_notification = []
+    def __repr__(self):
+        things = []
+        if len(self.arch_notifications):
+            things.append( '{} archs'.format(len(self.arch_notifications)) )
+        if len(self.package_notifications):
+            things.append( '{} pkgs'.format(len(self.package_notifications)) )
+        if len(self.package_version_notification):
+            things.append( '{} outdated pkgs'.format(len(self.package_version_notification)) )
+        if things:
+            return 'Notification <' + ', '.join(things) + '>'
+        else:
+            return None
 
 # Calculate the list of .mk files in the Buildroot source tree, will
 # be used to guess the name of the packages that caused build
-- 
2.21.0

[Buildroot] [PATCH buildroot-test 3/4] utils/daily-mail: add some debug information for the Notification class

[Thomas Petazzoni]

On Mon,  8 Jul 2019 10:14:05 +0200
Victor Huesca <victor.huesca@bootlin.com> wrote:

> It was useful to me when working on adding support for new notification
> so it may help someone else.
> 
> Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>

This should be added before the outdated package feature, and then the
patch adding the outdated feature will just add the two lines about
self.package_version_notification.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH buildroot-test 4/4] utils/daily-mail: add multiple argument to ease modularity of the script

[Victor Huesca]

Options added:
- Include out-to-date packages
- Specify a custom date instead of today
- Enable/disable a header for tables displayed
- Specify branches to include instead of reading the 'branches' file
- Allow to send emails to developers only / to the maillist only

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 66 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index 85a1c4e..9c64645 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -14,11 +14,13 @@ from collections import defaultdict
 import json
 from packaging import version
 import re
+import argparse
 
 sys.path.append(os.path.join(localconfig.brbase, "utils"))
 import getdeveloperlib
 
 RE_HASH_40 = re.compile(r'.*[a-fA-F0-9]{40}.*')
+RE_DATE = re.compile(r'^\d\d\d\d-\d\d-\d\d$')
 
 baseurl = "autobuild.buildroot.net"
 http_baseurl = "http://" + baseurl
@@ -237,7 +239,7 @@ def show_outdated(packages, show_orphan=False, show_header=False):
     return contents
 
 # Send the e-mails to the individual developers
-def developers_email(smtp, branches, notifications, datestr, dry_run):
+def developers_email(smtp, branches, notifications, datestr, dry_run, show_header=False):
     for dev, notif in notifications.iteritems():
         to = dev.name
         email_from = localconfig.fromaddr
@@ -265,18 +267,18 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
 
             if len(archs) != 0:
                 contents += "Build failures related to your architectures:\n\n"
-                contents += show_results(archs, show_status=False, show_orphan=False)
+                contents += show_results(archs, show_status=False, show_orphan=False, show_header=show_header)
                 contents += "\n\n"
             if len(packages) != 0:
                 contents += "Build failures related to your packages:\n\n"
-                contents += show_results(packages, show_status=False, show_orphan=show_orphan)
+                contents += show_results(packages, show_status=False, show_orphan=show_orphan, show_header=show_header)
 
             contents += "\n"
 
         outdated = notif.package_version_notification
         if len(outdated) != 0:
             contents += "Packages you maintain that have a new upstream version:\n\n"
-            contents += show_outdated(outdated, show_orphan=show_orphan)
+            contents += show_outdated(outdated, show_orphan=show_orphan, show_header=show_header)
             contents += '\n'
 
         contents += "-- \n"
@@ -298,11 +300,14 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
             smtp.sendmail(email_from, to, msg.as_string())
             print "To: %s" % dev.name
 
-def global_email_branch_result(results, results_by_reason, branch):
+def global_email_branch_result(results, results_by_reason, branch, show_header=False):
     contents = "Results for branch '%s'\n" % branch
     contents += "=====================" + "=" * len(branch) + "\n\n"
     contents += "Classification of failures by reason\n"
     contents += "------------------------------------\n\n"
+    if show_header:
+        contents += "{:>30} | {:<2}\n".format('reason', 'n')
+        contents += "{0:-^30}-+-{0:-^2}\n".format('')
     for r in results_by_reason:
         reason = r['reason']
         if len(reason) > 30:
@@ -312,35 +317,37 @@ def global_email_branch_result(results, results_by_reason, branch):
     contents += "\n\n"
     contents += "Detail of failures\n"
     contents += "------------------\n\n"
-    contents += show_results(results, show_status=True, show_orphan=True)
+    contents += show_results(results, show_status=True, show_orphan=True, show_header=show_header)
     contents += "\n"
     return contents
 
 # Send the global e-mail to the mailing list
-def global_email(smtp, results, results_by_reason, datestr, overall, dry_run, outdated=None):
+def global_email(smtp, results, results_by_reason, datestr, overall, outdated, dry_run, show_header=False):
     to = "buildroot at buildroot.org"
     email_from = localconfig.fromaddr
     subject = "[%s] Build results for %s" % (baseurl, datestr)
     contents = "Hello,\n\n"
     contents += "Build statistics for %s\n" % datestr
     contents += "===============================\n\n"
-    contents += "      branch |  OK | NOK | TIM | TOT |\n"
+    contents += "      branch |  OK | NOK | TIM | TOT\n"
+    if show_header:
+        contents += "-------------+-----+-----+-----+-----\n"
     for branch in sorted(overall.iterkeys()):
         stats = overall[branch]
         if stats[3] == 0:
             continue
-        contents += "  %10s | %3d | %3d | %3d | %3d |\n" % \
+        contents += "  %10s | %3d | %3d | %3d | %3d\n" % \
                     (branch, stats[0], stats[1], stats[2], stats[3])
     contents += "\n"
     for branch in results.keys():
         if len(results[branch]) == 0:
             continue
-        contents += global_email_branch_result(results[branch], results_by_reason[branch], branch)
+        contents += global_email_branch_result(results[branch], results_by_reason[branch], branch, show_header=show_header)
     if outdated:
         contents += '\n\n'
         contents += "Packages having a newer version\n"
         contents += "===============================\n\n"
-        contents += show_outdated(outdated, show_orphan=True)    
+        contents += show_outdated(outdated, show_orphan=True, show_header=show_header)
     contents += "\n"
     contents += "-- \n"
     contents += http_baseurl
@@ -413,10 +420,29 @@ def get_outdated_pkg(path):
                         'from': 'MAPPING' if status == 2 else 'NAME' } )
     return sorted(s, key=lambda pkg: pkg['name'])
 
+def parse_args():
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--dry-run', action='store_true', help='Do not send email, only show output')
+    parser.add_argument('--outdated', action='store_true', help='Include outdated packages in emails')
+    parser.add_argument('--date', action='store', help='Use this date instead of today')
+    parser.add_argument('--no-header', dest='header', action='store_false', help='Dont not print header for the different tables')
+    parser.add_argument('--branches', dest='branches', action='store', help='List of branches (comma separated)')
+    group = parser.add_mutually_exclusive_group()
+    group.add_argument('--global-only', dest='dev_email', action='store_false', help='Only send global email')
+    group.add_argument('--dev-only', dest='global_email', action='store_false', help='Only send developers email')
+    args = parser.parse_args()
+    if args.date and not RE_DATE.match(args.date):
+        parser.error('date must be in format YYYY-MM-DD')
+    return args
+
 def __main__():
-    yesterday = date.today() - timedelta(1)
-    yesterday_str = yesterday.strftime('%Y-%m-%d')
-    branches = get_branches()
+    args = parse_args()
+    if not args.date:
+        yesterday = date.today() - timedelta(1)
+        yesterday_str = yesterday.strftime('%Y-%m-%d')
+    else:
+        yesterday_str = args.date
+    branches = get_branches() if not args.branches else args.branches.split(',')
 
     db = _mysql.connect(host=localconfig.host,
                         user=localconfig.user,
@@ -425,17 +451,15 @@ def __main__():
     overall_stats = get_overall_stats(db, yesterday_str, branches)
     results = get_build_results(db, yesterday_str, branches)
     results_by_reason = get_build_results_grouped_by_reason(db, yesterday_str, branches)
-    outdated_pkg = get_outdated_pkg(localconfig.pkg_stats)
+    outdated_pkg = get_outdated_pkg(localconfig.pkg_stats) if args.outdated else []
     notifications = calculate_notifications(results, outdated_pkg)
-    dry_run = False
-    if len(sys.argv) == 2 and sys.argv[1] == "--dry-run":
-        dry_run = True
     smtp = smtplib.SMTP(localconfig.smtphost, localconfig.smtpport)
     smtp.starttls()
     smtp.login(localconfig.smtpuser, localconfig.smtppass)
-    developers_email(smtp, branches, notifications, yesterday_str, dry_run)
-    global_email(smtp, results, results_by_reason, yesterday_str,
-                 overall_stats, dry_run)
+    if args.dev_email:
+        developers_email(smtp, branches, notifications, yesterday_str, args.dry_run, args.header)
+    if args.global_email:
+        global_email(smtp, results, results_by_reason, yesterday_str, overall_stats, outdated_pkg, args.dry_run, args.header)
     smtp.quit()
 
 __main__()
-- 
2.21.0

[Buildroot] [PATCH buildroot-test 4/4] utils/daily-mail: add multiple argument to ease modularity of the script

[Thomas Petazzoni]

Hello,

On Mon,  8 Jul 2019 10:14:06 +0200
Victor Huesca <victor.huesca@bootlin.com> wrote:

> Options added:
> - Include out-to-date packages
> - Specify a custom date instead of today
> - Enable/disable a header for tables displayed
> - Specify branches to include instead of reading the 'branches' file
> - Allow to send emails to developers only / to the maillist only
> 
> Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>

As I replied to the cover letter, this is doing too many things at
once. Please do this argument parsing addition at the beginning of the
series, one argument at a time.


>  # Send the global e-mail to the mailing list
> -def global_email(smtp, results, results_by_reason, datestr, overall, dry_run, outdated=None):
> +def global_email(smtp, results, results_by_reason, datestr, overall, outdated, dry_run, show_header=False):
>      to = "buildroot at buildroot.org"
>      email_from = localconfig.fromaddr
>      subject = "[%s] Build results for %s" % (baseurl, datestr)
>      contents = "Hello,\n\n"
>      contents += "Build statistics for %s\n" % datestr
>      contents += "===============================\n\n"
> -    contents += "      branch |  OK | NOK | TIM | TOT |\n"
> +    contents += "      branch |  OK | NOK | TIM | TOT\n"

This change is not really related.

> +    if show_header:
> +        contents += "-------------+-----+-----+-----+-----\n"

This is also not related to adding argument parsing, but related to
adding headers to existing tables. Should be a separate commit.

>      for branch in sorted(overall.iterkeys()):
>          stats = overall[branch]
>          if stats[3] == 0:
>              continue
> -        contents += "  %10s | %3d | %3d | %3d | %3d |\n" % \
> +        contents += "  %10s | %3d | %3d | %3d | %3d\n" % \
>                      (branch, stats[0], stats[1], stats[2], stats[3])

Ditto, unrelated to argument parsing.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH buildroot-test 0/4] add support for outdated packages

[Thomas Petazzoni]

Hello,

As usual, this is good work, glad to see that we're finally getting
e-mail notifications for this. See below for some comments.

On Mon,  8 Jul 2019 10:14:02 +0200
Victor Huesca <victor.huesca@bootlin.com> wrote:

> Victor Huesca (4):
>   utils/daily-mail: add information about version that may need an
>     update
>   utils/daily-mail: allow to print a header for results section
>   utils/daily-mail: add some debug information for the Notification
>     class
>   utils/daily-mail: add multiple argument to ease modularity of the
>     script

The main comment I have about the series is that it's not organized in
the proper order. For example, in PATCH 1/4, you add a new argument to
global_email(), which is not used anywhere.

I think you should instead introduce argparse, with just the --dry-run
option first. Then introduce the --branches option in one commit, then
introduce the --date option in another commit, then introduce the
--global-only and --dev-only options in another commit. Then the
introduction of the outdated changes stuff. I.e, really split
independent changes in separate commits.

Speaking of global-only and dev-only, perhaps we should do something
nicer, such as:

	--dest=dev,global --data=autobuild,outdated,defconfig,tests

It allows to select any combination of information and recipient, and
seems a bit more flexible.

But this can be done later. However, I'd really like to have separate
commits for the separate things.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com