[Buildroot] [PATCH 00/11] pkg-stats json output improvements

[Buildroot] [PATCH 00/11] pkg-stats json output improvements

[Heiko Thiery]

This patches add several improvements for the json output of the
pkg-stats script.

- add developers information to the packages
- add supported defconfigs to json
- add license information to json
- add package dependencies to json
- add more generic status field to the packages

Heiko Thiery (11):
  support/scripts/pkg-stats: store latest version in a hash
  support/scripts/pkg-stats: store pkg dir path
  support/scripts/pkg-stats: store patch info in a hash
  support/scripts/pkg-stats: do not exclued pkg name in json output
  support/scripts/pkg-stats: parse and set developers info
  support/scripts/pkg-stats: store licences of package
  support/scripts/pkg-stats: store dependencies of package
  support/scripts/pkg-stats: add generic package status field
  support/scripts/pkg-stats; use url status from dict for check
  support/scripts/pkg-stats: add package count to stats
  support/scripts/pkg-stats: create and store defconfig information

 support/scripts/pkg-stats | 252 +++++++++++++++++++++++++++++++-------
 1 file changed, 207 insertions(+), 45 deletions(-)

-- 
2.20.1

[Buildroot] [PATCH 01/11] support/scripts/pkg-stats: store latest version in a hash

[Heiko Thiery]

This patch changes the name and the variable from latest_verion of type list
to rm_status of type hash. This is for better readability/usability of the
data. With this the json output is more descriptive.

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index e477828f7b..8c64993aaf 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -61,7 +61,7 @@ class Package:
         self.url = None
         self.url_status = None
         self.url_worker = None
-        self.latest_version = (RM_API_STATUS_ERROR, None, None)
+        self.rm_version = {'status': RM_API_STATUS_ERROR, 'version': None, 'id': None}
 
     def pkgvar(self):
         return self.name.upper().replace("-", "_")
@@ -331,11 +331,10 @@ def check_package_latest_version_worker(name):
 
 def check_package_latest_version(packages):
     """
-    Fills in the .latest_version field of all Package objects
+    Fills in the .rm_version field of all Package objects
+
+    This field is a dict and has the following keys:
 
-    This field has a special format:
-      (status, version, id)
-    with:
     - status: one of RM_API_STATUS_ERROR,
       RM_API_STATUS_FOUND_BY_DISTRO, RM_API_STATUS_FOUND_BY_PATTERN,
       RM_API_STATUS_NOT_FOUND
@@ -351,7 +350,9 @@ def check_package_latest_version(packages):
     worker_pool = Pool(processes=64)
     results = worker_pool.map(check_package_latest_version_worker, (pkg.name for pkg in packages))
     for pkg, r in zip(packages, results):
-        pkg.latest_version = r
+        pkg.rm_version['status'] = r[0]
+        pkg.rm_version['version'] = r[1]
+        pkg.rm_version['id'] = r[2]
     del http_pool
 
 
@@ -379,13 +380,13 @@ def calculate_stats(packages):
             stats["hash"] += 1
         else:
             stats["no-hash"] += 1
-        if pkg.latest_version[0] == RM_API_STATUS_FOUND_BY_DISTRO:
+        if pkg.rm_version['status'] == RM_API_STATUS_FOUND_BY_DISTRO:
             stats["rmo-mapping"] += 1
         else:
             stats["rmo-no-mapping"] += 1
-        if not pkg.latest_version[1]:
+        if not pkg.rm_version['version']:
             stats["version-unknown"] += 1
-        elif pkg.latest_version[1] == pkg.current_version:
+        elif pkg.rm_version['version'] == pkg.current_version:
             stats["version-uptodate"] += 1
         else:
             stats["version-not-uptodate"] += 1
@@ -548,29 +549,29 @@ def dump_html_pkg(f, pkg):
     f.write("  <td class=\"centered\">%s</td>\n" % current_version)
 
     # Latest version
-    if pkg.latest_version[0] == RM_API_STATUS_ERROR:
+    if pkg.rm_version['status'] == RM_API_STATUS_ERROR:
         td_class.append("version-error")
-    if pkg.latest_version[1] is None:
+    if pkg.rm_version['version'] is None:
         td_class.append("version-unknown")
-    elif pkg.latest_version[1] != pkg.current_version:
+    elif pkg.rm_version['version'] != pkg.current_version:
         td_class.append("version-needs-update")
     else:
         td_class.append("version-good")
 
-    if pkg.latest_version[0] == RM_API_STATUS_ERROR:
+    if pkg.rm_version['status'] == RM_API_STATUS_ERROR:
         latest_version_text = "<b>Error</b>"
-    elif pkg.latest_version[0] == RM_API_STATUS_NOT_FOUND:
+    elif pkg.rm_version['status'] == RM_API_STATUS_NOT_FOUND:
         latest_version_text = "<b>Not found</b>"
     else:
-        if pkg.latest_version[1] is None:
+        if pkg.rm_version['version'] is None:
             latest_version_text = "<b>Found, but no version</b>"
         else:
             latest_version_text = "<a href=\"https://release-monitoring.org/project/%s\"><b>%s</b></a>" % \
-                (pkg.latest_version[2], str(pkg.latest_version[1]))
+                (pkg.rm_version['id'], str(pkg.rm_version['version']))
 
         latest_version_text += "<br/>"
 
-        if pkg.latest_version[0] == RM_API_STATUS_FOUND_BY_DISTRO:
+        if pkg.rm_version['status'] == RM_API_STATUS_FOUND_BY_DISTRO:
             latest_version_text += "found by <a href=\"https://release-monitoring.org/distro/Buildroot/\">distro</a>"
         else:
             latest_version_text += "found by guess"
-- 
2.20.1

[Buildroot] [PATCH 02/11] support/scripts/pkg-stats: store pkg dir path

[Heiko Thiery]

Use this value where it make sense.

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index 8c64993aaf..9cfbcf1acc 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -51,6 +51,7 @@ class Package:
     def __init__(self, name, path):
         self.name = name
         self.path = path
+        self.pkg_path = os.path.dirname(path)
         self.infras = None
         self.has_license = False
         self.has_license_files = False
@@ -71,9 +72,9 @@ class Package:
         Fills in the .url field
         """
         self.url_status = "No Config.in"
-        for filename in os.listdir(os.path.dirname(self.path)):
+        for filename in os.listdir(self.pkg_path):
             if fnmatch.fnmatch(filename, 'Config.*'):
-                fp = open(os.path.join(os.path.dirname(self.path), filename), "r")
+                fp = open(os.path.join(self.pkg_path, filename), "r")
                 for config_line in fp:
                     if URL_RE.match(config_line):
                         self.url = config_line.strip()
@@ -88,7 +89,7 @@ class Package:
         Fills in the .infras field
         """
         self.infras = list()
-        with open(self.path, 'r') as f:
+        with open(os.path.join(self.pkg_path, self.name + '.mk'), 'r') as f:
             lines = f.readlines()
             for l in lines:
                 match = INFRA_RE.match(l)
@@ -139,8 +140,7 @@ class Package:
         Fills in the .warnings field
         """
         cmd = ["./utils/check-package"]
-        pkgdir = os.path.dirname(self.path)
-        for root, dirs, files in os.walk(pkgdir):
+        for root, dirs, files in os.walk(self.pkg_path):
             for f in files:
                 if f.endswith(".mk") or f.endswith(".hash") or f == "Config.in" or f == "Config.in.host":
                     cmd.append(os.path.join(root, f))
@@ -732,6 +732,8 @@ def __main__():
                                       'HEAD']).splitlines()[0]
     print("Build package list ...")
     packages = get_pkglist(args.npackages, package_list)
+    print("Getting developers...")
+    developers = parse_developers()
     print("Getting package make info ...")
     package_init_make_info()
     print("Getting package details ...")
-- 
2.20.1

[Buildroot] [PATCH 03/11] support/scripts/pkg-stats: store patch info in a hash

[Heiko Thiery]

This patch changes the variable to store the patch count to a hash. This
variable holds the patch count of the packages as well a list of the patch
file names.

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index 9cfbcf1acc..92fc01d655 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -56,7 +56,7 @@ class Package:
         self.has_license = False
         self.has_license_files = False
         self.has_hash = False
-        self.patch_count = 0
+        self.patches = {'count':0, 'files': None}
         self.warnings = 0
         self.current_version = None
         self.url = None
@@ -115,17 +115,16 @@ class Package:
         """
         Fills in the .has_hash field
         """
-        hashpath = self.path.replace(".mk", ".hash")
+        hashpath = os.path.join(self.pkg_path, self.name + '.hash')
         self.has_hash = os.path.exists(hashpath)
 
     def set_patch_count(self):
         """
         Fills in the .patch_count field
         """
-        self.patch_count = 0
-        pkgdir = os.path.dirname(self.path)
-        for subdir, _, _ in os.walk(pkgdir):
-            self.patch_count += len(fnmatch.filter(os.listdir(subdir), '*.patch'))
+        for subdir, _, _ in os.walk(self.pkg_path):
+            self.patches['files'] = fnmatch.filter(os.listdir(subdir), '*.patch')
+            self.patches['count'] = len(self.patches['files'])
 
     def set_current_version(self):
         """
@@ -160,7 +159,7 @@ class Package:
 
     def __str__(self):
         return "%s (path='%s', license='%s', license_files='%s', hash='%s', patches=%d)" % \
-            (self.name, self.path, self.has_license, self.has_license_files, self.has_hash, self.patch_count)
+            (self.name, self.path, self.has_license, self.has_license_files, self.has_hash, self.patches['count'])
 
 
 def get_pkglist(npackages, package_list):
@@ -390,7 +389,7 @@ def calculate_stats(packages):
             stats["version-uptodate"] += 1
         else:
             stats["version-not-uptodate"] += 1
-        stats["patches"] += pkg.patch_count
+        stats["patches"] += pkg.patches['count']
     return stats
 
 
@@ -495,14 +494,14 @@ def dump_html_pkg(f, pkg):
 
     # Patch count
     td_class = ["centered"]
-    if pkg.patch_count == 0:
+    if pkg.patches['count'] == 0:
         td_class.append("nopatches")
-    elif pkg.patch_count < 5:
+    elif pkg.patches['count'] < 5:
         td_class.append("somepatches")
     else:
         td_class.append("lotsofpatches")
     f.write("  <td class=\"%s\">%s</td>\n" %
-            (" ".join(td_class), str(pkg.patch_count)))
+            (" ".join(td_class), str(pkg.patches['count'])))
 
     # Infrastructure
     infra = infra_str(pkg.infras)
-- 
2.20.1

[Buildroot] [PATCH 04/11] support/scripts/pkg-stats: do not exclued pkg name in json output

[Heiko Thiery]

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index 92fc01d655..4021aacceb 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -676,7 +676,7 @@ def dump_html(packages, stats, date, commit, output):
 def dump_json(packages, stats, date, commit, output):
     # Format packages as a dictionnary instead of a list
     # Exclude local field that does not contains real date
-    excluded_fields = ['url_worker', 'name']
+    excluded_fields = ['url_worker']
     pkgs = {
         pkg.name: {
             k: v
-- 
2.20.1

[Buildroot] [PATCH 05/11] support/scripts/pkg-stats: parse and set developers info

[Heiko Thiery]

This patch collects the developers information and stores developers to
the Pakckage instance.

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 59 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index 4021aacceb..d520da6807 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -20,6 +20,7 @@ import argparse
 import datetime
 import fnmatch
 import os
+import glob
 from collections import defaultdict
 import re
 import subprocess
@@ -42,6 +43,50 @@ RM_API_STATUS_NOT_FOUND = 4
 # because it's used by sub-processes.
 http_pool = None
 
+class Developer:
+    def __init__(self, name, files):
+        self.name = name
+        self.files = files
+
+def parse_developers(basepath=None):
+    """Parse the DEVELOPERS file and return a list of Developer objects."""
+    developers = []
+    linen = 0
+    if basepath is None:
+        basepath = os.getcwd()
+    with open(os.path.join(basepath, "DEVELOPERS"), "r") as f:
+        files = []
+        name = None
+        for line in f:
+            line = line.strip()
+            if line.startswith("#"):
+                continue
+            elif line.startswith("N:"):
+                if name is not None or len(files) != 0:
+                    print("Syntax error in DEVELOPERS file, line %d" % linen)
+                name = line[2:].strip()
+            elif line.startswith("F:"):
+                fname = line[2:].strip()
+                #dev_files = glob.glob(os.path.join(basepath, fname))
+                dev_files = glob.glob(fname)
+                if len(dev_files) == 0:
+                    print("WARNING: '%s' doesn't match any file" % fname)
+                files += dev_files
+            elif line == "":
+                if not name:
+                    continue
+                developers.append(Developer(name, files))
+                files = []
+                name = None
+            else:
+                print("Syntax error in DEVELOPERS file, line %d: '%s'" % (linen, line))
+
+                return None
+            linen += 1
+    # handle last developer
+    if name is not None:
+        developers.append(Developer(name, files))
+    return developers
 
 class Package:
     all_licenses = list()
@@ -56,6 +101,7 @@ class Package:
         self.has_license = False
         self.has_license_files = False
         self.has_hash = False
+        self.developers = None
         self.patches = {'count':0, 'files': None}
         self.warnings = 0
         self.current_version = None
@@ -151,6 +197,16 @@ class Package:
                 self.warnings = int(m.group(1))
                 return
 
+    def set_developers(self, developers):
+        """
+        Fills in the .developers field
+        """
+        self.developers = list()
+        for dev in developers:
+            for f in dev.files:
+                if self.pkg_path[2:] == f[:-1]:
+                    self.developers.append((dev.name))
+
     def __eq__(self, other):
         return self.path == other.path
 
@@ -729,6 +785,8 @@ def __main__():
     date = datetime.datetime.utcnow()
     commit = subprocess.check_output(['git', 'rev-parse',
                                       'HEAD']).splitlines()[0]
+    print("Getting developers...")
+    developers = parse_developers()
     print("Build package list ...")
     packages = get_pkglist(args.npackages, package_list)
     print("Getting developers...")
@@ -744,6 +802,7 @@ def __main__():
         pkg.set_check_package_warnings()
         pkg.set_current_version()
         pkg.set_url()
+        pkg.set_developers(developers)
     print("Checking URL status")
     check_package_urls(packages)
     print("Getting latest versions ...")
-- 
2.20.1

[Buildroot] [PATCH 06/11] support/scripts/pkg-stats: store licences of package

[Heiko Thiery]

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index d520da6807..a5e87a7167 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -89,7 +89,7 @@ def parse_developers(basepath=None):
     return developers
 
 class Package:
-    all_licenses = list()
+    all_licenses = dict()
     all_license_files = list()
     all_versions = dict()
 
@@ -98,6 +98,7 @@ class Package:
         self.path = path
         self.pkg_path = os.path.dirname(path)
         self.infras = None
+        self.license = None
         self.has_license = False
         self.has_license_files = False
         self.has_hash = False
@@ -154,6 +155,7 @@ class Package:
         var = self.pkgvar()
         if var in self.all_licenses:
             self.has_license = True
+            self.license = self.all_licenses[var]
         if var in self.all_license_files:
             self.has_license_files = True
 
@@ -302,7 +304,7 @@ def package_init_make_info():
             if value == "unknown":
                 continue
             pkgvar = pkgvar[:-8]
-            Package.all_licenses.append(pkgvar)
+            Package.all_licenses[pkgvar] = value
 
         elif pkgvar.endswith("_LICENSE_FILES"):
             if pkgvar.endswith("_MANIFEST_LICENSE_FILES"):
@@ -785,8 +787,6 @@ def __main__():
     date = datetime.datetime.utcnow()
     commit = subprocess.check_output(['git', 'rev-parse',
                                       'HEAD']).splitlines()[0]
-    print("Getting developers...")
-    developers = parse_developers()
     print("Build package list ...")
     packages = get_pkglist(args.npackages, package_list)
     print("Getting developers...")
-- 
2.20.1

[Buildroot] [PATCH 07/11] support/scripts/pkg-stats: store dependencies of package

[Heiko Thiery]

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index a5e87a7167..324700adbf 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -92,6 +92,7 @@ class Package:
     all_licenses = dict()
     all_license_files = list()
     all_versions = dict()
+    all_dependencies = dict()
 
     def __init__(self, name, path):
         self.name = name
@@ -106,6 +107,7 @@ class Package:
         self.patches = {'count':0, 'files': None}
         self.warnings = 0
         self.current_version = None
+        self.dependencies = None
         self.url = None
         self.url_status = None
         self.url_worker = None
@@ -182,6 +184,14 @@ class Package:
         if var in self.all_versions:
             self.current_version = self.all_versions[var]
 
+    def set_dependencies(self):
+        """
+        Fills in the .dependencies field
+        """
+        var = self.pkgvar()
+        if var in self.all_dependencies:
+            self.dependencies = self.all_dependencies[var]
+
     def set_check_package_warnings(self):
         """
         Fills in the .warnings field
@@ -284,7 +294,7 @@ def get_pkglist(npackages, package_list):
 def package_init_make_info():
     # Fetch all variables at once
     variables = subprocess.check_output(["make", "BR2_HAVE_DOT_CONFIG=y", "-s", "printvars",
-                                         "VARS=%_LICENSE %_LICENSE_FILES %_VERSION"])
+                                         "VARS=%_LICENSE %_LICENSE_FILES %_VERSION %_DEPENDENCIES"])
     variable_list = variables.splitlines()
 
     # We process first the host package VERSION, and then the target
@@ -318,6 +328,11 @@ def package_init_make_info():
             pkgvar = pkgvar[:-8]
             Package.all_versions[pkgvar] = value
 
+        elif pkgvar.endswith("_FINAL_DEPENDENCIES"):
+            value = [v for v in value.split(' ') if not v.startswith('host-')]
+            pkgvar = pkgvar[:-19]
+            Package.all_dependencies[pkgvar] = value
+
 
 def check_url_status_worker(url, url_status):
     if url_status != "Missing" and url_status != "No Config.in":
@@ -801,6 +816,7 @@ def __main__():
         pkg.set_patch_count()
         pkg.set_check_package_warnings()
         pkg.set_current_version()
+        pkg.set_dependencies()
         pkg.set_url()
         pkg.set_developers(developers)
     print("Checking URL status")
-- 
2.20.1

[Buildroot] [PATCH 08/11] support/scripts/pkg-stats: add generic package status field

[Heiko Thiery]

Each check stores the status as a tuple: (status, message). The status
value should be one of: 'ok', 'warning', 'error' and the message holds
more verbose information.

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 42 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index 324700adbf..b0c2db2b93 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -112,6 +112,7 @@ class Package:
         self.url_status = None
         self.url_worker = None
         self.rm_version = {'status': RM_API_STATUS_ERROR, 'version': None, 'id': None}
+        self.status = {}
 
     def pkgvar(self):
         return self.name.upper().replace("-", "_")
@@ -121,6 +122,7 @@ class Package:
         Fills in the .url field
         """
         self.url_status = "No Config.in"
+        self.status['url'] = ("error", "no Config.in")
         for filename in os.listdir(self.pkg_path):
             if fnmatch.fnmatch(filename, 'Config.*'):
                 fp = open(os.path.join(self.pkg_path, filename), "r")
@@ -128,9 +130,11 @@ class Package:
                     if URL_RE.match(config_line):
                         self.url = config_line.strip()
                         self.url_status = "Found"
+                        self.status['url'] = ("ok", "found")
                         fp.close()
                         return
                 self.url_status = "Missing"
+                self.status['url'] = ("error", "missing")
                 fp.close()
 
     def set_infra(self):
@@ -155,11 +159,16 @@ class Package:
         Fills in the .has_license and .has_license_files fields
         """
         var = self.pkgvar()
+        self.status['license'] = ("error", "missing")
+        self.status['license-files'] = ("error", "missing")
         if var in self.all_licenses:
             self.has_license = True
             self.license = self.all_licenses[var]
+            self.status['license'] = ("ok", "found")
+
         if var in self.all_license_files:
             self.has_license_files = True
+            self.status['license-files'] = ("ok", "found")
 
     def set_hash_info(self):
         """
@@ -167,6 +176,10 @@ class Package:
         """
         hashpath = os.path.join(self.pkg_path, self.name + '.hash')
         self.has_hash = os.path.exists(hashpath)
+        if self.has_hash:
+            self.status['hash'] = ("ok", "found")
+        else:
+            self.status['hash'] = ("error", "missing")
 
     def set_patch_count(self):
         """
@@ -176,6 +189,13 @@ class Package:
             self.patches['files'] = fnmatch.filter(os.listdir(subdir), '*.patch')
             self.patches['count'] = len(self.patches['files'])
 
+            if self.patches['count'] == 0:
+                self.status['patches'] = ("ok", "no patches")
+            elif self.patches['count'] < 5:
+                self.status['patches'] = ("warning", "some patches")
+            else:
+                self.status['patches'] = ("error", "lots of patches")
+
     def set_current_version(self):
         """
         Fills in the .current_version field
@@ -197,6 +217,7 @@ class Package:
         Fills in the .warnings field
         """
         cmd = ["./utils/check-package"]
+        self.status['pkg-check'] = ("error", "missing")
         for root, dirs, files in os.walk(self.pkg_path):
             for f in files:
                 if f.endswith(".mk") or f.endswith(".hash") or f == "Config.in" or f == "Config.in.host":
@@ -207,6 +228,10 @@ class Package:
             m = re.match("^([0-9]*) warnings generated", line)
             if m:
                 self.warnings = int(m.group(1))
+                if self.warnings == 0:
+                    self.status['pkg-check'] = ("ok", "no warnings")
+                else:
+                    self.status['pkg-check'] = ("error", "{} warnings".format(self.warnings))
                 return
 
     def set_developers(self, developers):
@@ -214,11 +239,15 @@ class Package:
         Fills in the .developers field
         """
         self.developers = list()
+        self.status['developers'] = ("warning", "no developers")
         for dev in developers:
             for f in dev.files:
                 if self.pkg_path[2:] == f[:-1]:
                     self.developers.append((dev.name))
 
+        if self.developers:
+            self.status['developers'] = ("ok", "{} developers".format(len(self.developers)))
+
     def __eq__(self, other):
         return self.path == other.path
 
@@ -425,6 +454,19 @@ def check_package_latest_version(packages):
         pkg.rm_version['status'] = r[0]
         pkg.rm_version['version'] = r[1]
         pkg.rm_version['id'] = r[2]
+        pkg.rm_version['version'] == pkg.current_version
+
+        if pkg.rm_version['status'] == RM_API_STATUS_ERROR:
+            pkg.status['version'] = ('warning', 'rm api error')
+        elif pkg.rm_version['status'] == RM_API_STATUS_NOT_FOUND:
+            pkg.status['version'] = ('warning', 'rm package not found')
+
+        if pkg.rm_version['version'] is None:
+            pkg.status['version'] = ('warning', 'no upstream version available')
+        elif pkg.rm_version['version'] != pkg.current_version:
+            pkg.status['version'] = ('error', 'package needs update')
+        else:
+            pkg.status['version'] = ('ok', 'up-to-date')
     del http_pool
 
 
-- 
2.20.1

[Buildroot] [PATCH 09/11] support/scripts/pkg-stats; use url status from dict for check

[Heiko Thiery]

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index b0c2db2b93..dd3c6566a0 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -364,23 +364,23 @@ def package_init_make_info():
 
 
 def check_url_status_worker(url, url_status):
-    if url_status != "Missing" and url_status != "No Config.in":
+    if url_status[0] == "ok":
         try:
             url_status_code = requests.head(url, timeout=30).status_code
             if url_status_code >= 400:
-                return "Invalid(%s)" % str(url_status_code)
+                return ('error', 'invalid({})'.format(url_status_code))
         except requests.exceptions.RequestException:
-            return "Invalid(Err)"
-        return "Ok"
+            return ('error', 'invalid(Err)')
+        return ("ok", "Valid")
     return url_status
 
 
 def check_package_urls(packages):
     Package.pool = Pool(processes=64)
     for pkg in packages:
-        pkg.url_worker = pkg.pool.apply_async(check_url_status_worker, (pkg.url, pkg.url_status))
+        pkg.url_worker = pkg.pool.apply_async(check_url_status_worker, (pkg.url, pkg.status['url']))
     for pkg in packages:
-        pkg.url_status = pkg.url_worker.get(timeout=3600)
+        pkg.status['url'] = pkg.url_worker.get(timeout=3600)
 
 
 def release_monitoring_get_latest_version_by_distro(pool, name):
-- 
2.20.1

[Buildroot] [PATCH 10/11] support/scripts/pkg-stats: add package count to stats

[Heiko Thiery]

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index dd3c6566a0..afd9cacafb 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -457,21 +457,22 @@ def check_package_latest_version(packages):
         pkg.rm_version['version'] == pkg.current_version
 
         if pkg.rm_version['status'] == RM_API_STATUS_ERROR:
-            pkg.status['version'] = ('warning', 'rm api error')
+            pkg.status['version'] = ('warning', 'RM API error')
         elif pkg.rm_version['status'] == RM_API_STATUS_NOT_FOUND:
-            pkg.status['version'] = ('warning', 'rm package not found')
+            pkg.status['version'] = ('warning', 'RM package not found')
 
         if pkg.rm_version['version'] is None:
             pkg.status['version'] = ('warning', 'no upstream version available')
         elif pkg.rm_version['version'] != pkg.current_version:
-            pkg.status['version'] = ('error', 'package needs update')
+            pkg.status['version'] = ('error', 'Package needs update')
         else:
-            pkg.status['version'] = ('ok', 'up-to-date')
+            pkg.status['version'] = ('ok', 'update-to-date')
     del http_pool
 
 
 def calculate_stats(packages):
     stats = defaultdict(int)
+    stats["packages"] = len(packages)
     for pkg in packages:
         # If packages have multiple infra, take the first one. For the
         # vast majority of packages, the target and host infra are the
@@ -743,6 +744,8 @@ def dump_html_all_pkgs(f, packages):
 def dump_html_stats(f, stats):
     f.write("<a id=\"results\"></a>\n")
     f.write("<table>\n")
+    f.write(" <tr><td>Total number of packages</td><td>%s</td></tr>\n" %
+            stats["packages"])
     infras = [infra[6:] for infra in stats.keys() if infra.startswith("infra-")]
     for infra in infras:
         f.write(" <tr><td>Packages using the <i>%s</i> infrastructure</td><td>%s</td></tr>\n" %
-- 
2.20.1

[Buildroot] [PATCH 11/11] support/scripts/pkg-stats: create and store defconfig information

[Heiko Thiery]

Collect information about developers and store with defconfig name.

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 48 +++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index afd9cacafb..a85aefadf4 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -88,6 +88,34 @@ def parse_developers(basepath=None):
         developers.append(Developer(name, files))
     return developers
 
+class Defconfig:
+    def __init__(self, name, path):
+        self.name = name
+        self.path = path
+        self.developers = None
+
+    def set_developers(self, developers):
+        """
+        Fills in the .developers field
+        """
+        self.developers = list()
+        for dev in developers:
+            for f in dev.files:
+                if self.path == f:
+                    self.developers.append(dev.name)
+
+def get_defconfig_list():
+    """
+    Builds the list of Buildroot defconfigs, returning a list of Defconfig
+    objects.
+    """
+    defconfigs = list()
+    files = [f for f in os.listdir('configs')]
+    for name in files:
+        d = Defconfig(name[:-10], os.path.join('configs', name))
+        defconfigs.append(d)
+    return defconfigs
+
 class Package:
     all_licenses = dict()
     all_license_files = list()
@@ -791,7 +819,7 @@ def dump_html(packages, stats, date, commit, output):
         f.write(html_footer)
 
 
-def dump_json(packages, stats, date, commit, output):
+def dump_json(packages, defconfigs, stats, date, commit, output):
     # Format packages as a dictionnary instead of a list
     # Exclude local field that does not contains real date
     excluded_fields = ['url_worker']
@@ -802,6 +830,13 @@ def dump_json(packages, stats, date, commit, output):
             if k not in excluded_fields
         } for pkg in packages
     }
+
+    defconfigs = {
+        d.name: {
+            k: v
+            for k, v in d.__dict__.items()
+        } for d in defconfigs
+    }
     # Aggregate infrastructures into a single dict entry
     statistics = {
         k: v
@@ -811,6 +846,7 @@ def dump_json(packages, stats, date, commit, output):
     statistics['infra'] = {k[6:]: v for k, v in stats.items() if k.startswith('infra-')}
     # The actual structure to dump, add commit and date to it
     final = {'packages': pkgs,
+             'defconfigs': defconfigs,
              'stats': statistics,
              'commit': commit,
              'date': str(date)}
@@ -847,10 +883,14 @@ def __main__():
     date = datetime.datetime.utcnow()
     commit = subprocess.check_output(['git', 'rev-parse',
                                       'HEAD']).splitlines()[0]
-    print("Build package list ...")
-    packages = get_pkglist(args.npackages, package_list)
     print("Getting developers...")
     developers = parse_developers()
+    print("Build defconfig list ...")
+    defconfigs = get_defconfig_list()
+    for d in defconfigs:
+        d.set_developers(developers)
+    print("Build package list ...")
+    packages = get_pkglist(args.npackages, package_list)
     print("Getting package make info ...")
     package_init_make_info()
     print("Getting package details ...")
@@ -875,7 +915,7 @@ def __main__():
         dump_html(packages, stats, date, commit, args.html)
     if args.json:
         print("Write JSON")
-        dump_json(packages, stats, date, commit, args.json)
+        dump_json(packages, defconfigs, stats, date, commit, args.json)
 
 
 __main__()
-- 
2.20.1

[Buildroot] [PATCH 02/11] support/scripts/pkg-stats: store pkg dir path

[Thomas Petazzoni]

On Fri,  3 Jan 2020 16:18:39 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> @@ -732,6 +732,8 @@ def __main__():
>                                        'HEAD']).splitlines()[0]
>      print("Build package list ...")
>      packages = get_pkglist(args.npackages, package_list)
> +    print("Getting developers...")
> +    developers = parse_developers()
>      print("Getting package make info ...")
>      package_init_make_info()
>      print("Getting package details ...")

This chunk doesn't seem to be related to this patch.

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

[Buildroot] [PATCH 03/11] support/scripts/pkg-stats: store patch info in a hash

[Thomas Petazzoni]

On Fri,  3 Jan 2020 16:18:40 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> This patch changes the variable to store the patch count to a hash. This
> variable holds the patch count of the packages as well a list of the patch
> file names.
> 
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

Instead of "hash" you should use "dict", which is the actual name used
in the Python world for this data type, if I'm correct. This is also
valid for PATCH 01/11.

> diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> index 9cfbcf1acc..92fc01d655 100755
> --- a/support/scripts/pkg-stats
> +++ b/support/scripts/pkg-stats
> @@ -56,7 +56,7 @@ class Package:
>          self.has_license = False
>          self.has_license_files = False
>          self.has_hash = False
> -        self.patch_count = 0
> +        self.patches = {'count':0, 'files': None}

I am wondering if a dict here is really appropriate, shouldn't we
instead keep a simpler:

	self.patch_count
	self.patch_files

and be done with it ?

Note: I haven't looked at the other patches in your series yet. Maybe
you add more entries to the dict that make it more relevant.

Best regards,

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

[Buildroot] [PATCH 04/11] support/scripts/pkg-stats: do not exclued pkg name in json output

[Thomas Petazzoni]

On Fri,  3 Jan 2020 16:18:41 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

But isn't the package name already present in
http://autobuild.buildroot.net/stats/index.json ? All the subnodes of
/packages/ are nodes named after the packages.

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

[Buildroot] [PATCH 05/11] support/scripts/pkg-stats: parse and set developers info

[Thomas Petazzoni]

Hello,

On Fri,  3 Jan 2020 16:18:42 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> +class Developer:
> +    def __init__(self, name, files):
> +        self.name = name
> +        self.files = files
> +
> +def parse_developers(basepath=None):
> +    """Parse the DEVELOPERS file and return a list of Developer objects."""
> +    developers = []
> +    linen = 0
> +    if basepath is None:
> +        basepath = os.getcwd()
> +    with open(os.path.join(basepath, "DEVELOPERS"), "r") as f:
> +        files = []
> +        name = None
> +        for line in f:
> +            line = line.strip()
> +            if line.startswith("#"):
> +                continue
> +            elif line.startswith("N:"):
> +                if name is not None or len(files) != 0:
> +                    print("Syntax error in DEVELOPERS file, line %d" % linen)
> +                name = line[2:].strip()
> +            elif line.startswith("F:"):
> +                fname = line[2:].strip()
> +                #dev_files = glob.glob(os.path.join(basepath, fname))
> +                dev_files = glob.glob(fname)
> +                if len(dev_files) == 0:
> +                    print("WARNING: '%s' doesn't match any file" % fname)
> +                files += dev_files
> +            elif line == "":
> +                if not name:
> +                    continue
> +                developers.append(Developer(name, files))
> +                files = []
> +                name = None
> +            else:
> +                print("Syntax error in DEVELOPERS file, line %d: '%s'" % (linen, line))
> +
> +                return None
> +            linen += 1
> +    # handle last developer
> +    if name is not None:
> +        developers.append(Developer(name, files))
> +    return developers

You have duplicated this function from utils/getdeveloperlib.py, which
is intended to be used as a Python module. Why don't we try to use it,
instead ?

Thanks,

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

[Buildroot] [PATCH 06/11] support/scripts/pkg-stats: store licences of package

[Thomas Petazzoni]

On Fri,  3 Jan 2020 16:18:43 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

Just curious: how does this gets used in your statistics site ? I see
that you show whether the package has a license or not, but I don't see
the license details being exposed.

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

[Buildroot] [PATCH 07/11] support/scripts/pkg-stats: store dependencies of package

[Thomas Petazzoni]

On Fri,  3 Jan 2020 16:18:44 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

Storing those dependencies do not make sense, because dependencies are
completely different depending on the Buildroot configuration. Packages
have optional dependencies, some dependencies on host packages only
exist depending on the system configuration, etc. So exposing those
dependencies is never going to give anything that is correct.

Best regards,

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

[Buildroot] [PATCH 08/11] support/scripts/pkg-stats: add generic package status field

[Thomas Petazzoni]

Hello,

On Fri,  3 Jan 2020 16:18:45 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> index 324700adbf..b0c2db2b93 100755
> --- a/support/scripts/pkg-stats
> +++ b/support/scripts/pkg-stats
> @@ -112,6 +112,7 @@ class Package:
>          self.url_status = None
>          self.url_worker = None
>          self.rm_version = {'status': RM_API_STATUS_ERROR, 'version': None, 'id': None}
> +        self.status = {}
>  
>      def pkgvar(self):
>          return self.name.upper().replace("-", "_")
> @@ -121,6 +122,7 @@ class Package:
>          Fills in the .url field
>          """
>          self.url_status = "No Config.in"
> +        self.status['url'] = ("error", "no Config.in")

Here it is duplicating the url_status field, I believe we want to avoid
that.

>          for filename in os.listdir(self.pkg_path):
>              if fnmatch.fnmatch(filename, 'Config.*'):
>                  fp = open(os.path.join(self.pkg_path, filename), "r")
> @@ -128,9 +130,11 @@ class Package:
>                      if URL_RE.match(config_line):
>                          self.url = config_line.strip()
>                          self.url_status = "Found"
> +                        self.status['url'] = ("ok", "found")

Ditto.

>                          fp.close()
>                          return
>                  self.url_status = "Missing"
> +                self.status['url'] = ("error", "missing")

Ditto.

>                  fp.close()
>  
>      def set_infra(self):
> @@ -155,11 +159,16 @@ class Package:
>          Fills in the .has_license and .has_license_files fields
>          """
>          var = self.pkgvar()
> +        self.status['license'] = ("error", "missing")
> +        self.status['license-files'] = ("error", "missing")
>          if var in self.all_licenses:
>              self.has_license = True
>              self.license = self.all_licenses[var]
> +            self.status['license'] = ("ok", "found")
> +
>          if var in self.all_license_files:
>              self.has_license_files = True
> +            self.status['license-files'] = ("ok", "found")

Here it's duplicating self.has_license and self.has_license_files
basically.

>      def set_hash_info(self):
>          """
> @@ -167,6 +176,10 @@ class Package:
>          """
>          hashpath = os.path.join(self.pkg_path, self.name + '.hash')
>          self.has_hash = os.path.exists(hashpath)
> +        if self.has_hash:
> +            self.status['hash'] = ("ok", "found")
> +        else:
> +            self.status['hash'] = ("error", "missing")

Same for self.has_hash.

Can we do better ? Maybe the issues is that pkg-stats does both
extracting the data to a JSON format *and* presenting it as HTML. Maybe
it should be separated into two tools:

 - One doing the data extraction and generation of JSON

 - One using the JSON to produce the HTML page, which would be replaced
   by your new site

I'm not sure exactly how to go, but I'm not a big fan of this patch
that duplicates information that already exists (and which doesn't
explain why).

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

[Buildroot] [PATCH 10/11] support/scripts/pkg-stats: add package count to stats

[Thomas Petazzoni]

On Fri,  3 Jan 2020 16:18:47 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> ---
>  support/scripts/pkg-stats | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> index dd3c6566a0..afd9cacafb 100755
> --- a/support/scripts/pkg-stats
> +++ b/support/scripts/pkg-stats
> @@ -457,21 +457,22 @@ def check_package_latest_version(packages):
>          pkg.rm_version['version'] == pkg.current_version
>  
>          if pkg.rm_version['status'] == RM_API_STATUS_ERROR:
> -            pkg.status['version'] = ('warning', 'rm api error')
> +            pkg.status['version'] = ('warning', 'RM API error')
>          elif pkg.rm_version['status'] == RM_API_STATUS_NOT_FOUND:
> -            pkg.status['version'] = ('warning', 'rm package not found')
> +            pkg.status['version'] = ('warning', 'RM package not found')
>  
>          if pkg.rm_version['version'] is None:
>              pkg.status['version'] = ('warning', 'no upstream version available')
>          elif pkg.rm_version['version'] != pkg.current_version:
> -            pkg.status['version'] = ('error', 'package needs update')
> +            pkg.status['version'] = ('error', 'Package needs update')
>          else:
> -            pkg.status['version'] = ('ok', 'up-to-date')
> +            pkg.status['version'] = ('ok', 'update-to-date')
>      del http_pool

The change up to here is not related to the commit title.

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

[Buildroot] [PATCH 11/11] support/scripts/pkg-stats: create and store defconfig information

[Thomas Petazzoni]

Hello,

On Fri,  3 Jan 2020 16:18:48 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> Collect information about developers and store with defconfig name.
> 
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

Seems good. Two comments below.

> ---
>  support/scripts/pkg-stats | 48 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> index afd9cacafb..a85aefadf4 100755
> --- a/support/scripts/pkg-stats
> +++ b/support/scripts/pkg-stats
> @@ -88,6 +88,34 @@ def parse_developers(basepath=None):
>          developers.append(Developer(name, files))
>      return developers
>  
> +class Defconfig:
> +    def __init__(self, name, path):
> +        self.name = name
> +        self.path = path
> +        self.developers = None
> +
> +    def set_developers(self, developers):
> +        """
> +        Fills in the .developers field
> +        """
> +        self.developers = list()
> +        for dev in developers:
> +            for f in dev.files:
> +                if self.path == f:

Perhaps add a Developer.has_file() method, to simplify this to:

	for dev in developers:
		if dev.has_file(f):
			self.developers.append(dev.name)


>  class Package:
>      all_licenses = dict()
>      all_license_files = list()
> @@ -791,7 +819,7 @@ def dump_html(packages, stats, date, commit, output):
>          f.write(html_footer)
>  
>  
> -def dump_json(packages, stats, date, commit, output):
> +def dump_json(packages, defconfigs, stats, date, commit, output):
>      # Format packages as a dictionnary instead of a list
>      # Exclude local field that does not contains real date
>      excluded_fields = ['url_worker']
> @@ -802,6 +830,13 @@ def dump_json(packages, stats, date, commit, output):
>              if k not in excluded_fields
>          } for pkg in packages
>      }
> +
> +    defconfigs = {
> +        d.name: {
> +            k: v
> +            for k, v in d.__dict__.items()
> +        } for d in defconfigs
> +    }
>      # Aggregate infrastructures into a single dict entry
>      statistics = {
>          k: v
> @@ -811,6 +846,7 @@ def dump_json(packages, stats, date, commit, output):
>      statistics['infra'] = {k[6:]: v for k, v in stats.items() if k.startswith('infra-')}
>      # The actual structure to dump, add commit and date to it
>      final = {'packages': pkgs,
> +             'defconfigs': defconfigs,
>               'stats': statistics,
>               'commit': commit,
>               'date': str(date)}
> @@ -847,10 +883,14 @@ def __main__():
>      date = datetime.datetime.utcnow()
>      commit = subprocess.check_output(['git', 'rev-parse',
>                                        'HEAD']).splitlines()[0]
> -    print("Build package list ...")
> -    packages = get_pkglist(args.npackages, package_list)

Not sure why this is being moved around.

>      print("Getting developers...")
>      developers = parse_developers()
> +    print("Build defconfig list ...")
> +    defconfigs = get_defconfig_list()
> +    for d in defconfigs:
> +        d.set_developers(developers)
> +    print("Build package list ...")
> +    packages = get_pkglist(args.npackages, package_list)
>      print("Getting package make info ...")
>      package_init_make_info()
>      print("Getting package details ...")
> @@ -875,7 +915,7 @@ def __main__():
>          dump_html(packages, stats, date, commit, args.html)
>      if args.json:
>          print("Write JSON")
> -        dump_json(packages, stats, date, commit, args.json)
> +        dump_json(packages, defconfigs, stats, date, commit, args.json)
>  
>  
>  __main__()

Thanks!

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

[Buildroot] [PATCH 03/11] support/scripts/pkg-stats: store patch info in a hash

[Heiko Thiery]

Am Fr., 3. Jan. 2020 um 16:23 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> On Fri,  3 Jan 2020 16:18:40 +0100
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > This patch changes the variable to store the patch count to a hash. This
> > variable holds the patch count of the packages as well a list of the patch
> > file names.
> >
> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
>
> Instead of "hash" you should use "dict", which is the actual name used
> in the Python world for this data type, if I'm correct. This is also
> valid for PATCH 01/11.

you are right

> > diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> > index 9cfbcf1acc..92fc01d655 100755
> > --- a/support/scripts/pkg-stats
> > +++ b/support/scripts/pkg-stats
> > @@ -56,7 +56,7 @@ class Package:
> >          self.has_license = False
> >          self.has_license_files = False
> >          self.has_hash = False
> > -        self.patch_count = 0
> > +        self.patches = {'count':0, 'files': None}
>
> I am wondering if a dict here is really appropriate, shouldn't we
> instead keep a simpler:
>
>         self.patch_count
>         self.patch_files
>
> and be done with it ?
>
> Note: I haven't looked at the other patches in your series yet. Maybe
> you add more entries to the dict that make it more relevant.

No there are no other entries in this dict.
But thinking over that I consider removing the count filed because we
can calculate the count from the length of the list of patches?! What
do you think?

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

[Buildroot] [PATCH 03/11] support/scripts/pkg-stats: store patch info in a hash

[Thomas Petazzoni]

On Fri, 3 Jan 2020 17:23:20 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> > Note: I haven't looked at the other patches in your series yet. Maybe
> > you add more entries to the dict that make it more relevant.  
> 
> No there are no other entries in this dict.
> But thinking over that I consider removing the count filed because we
> can calculate the count from the length of the list of patches?! What
> do you think?

Yes, also thought about this when reviewing. I also saw we calculate
this count in a few places, so was not sure if it was that great to
re-do the count several times. But it's not too many times so indeed,
perhaps it's easier to keep the patch list around.

But in fact, why do you need the list of patches ? Your web site only
shows the patch count anyway.

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

[Buildroot] [PATCH 04/11] support/scripts/pkg-stats: do not exclued pkg name in json output

[Heiko Thiery]

Am Fr., 3. Jan. 2020 um 16:24 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> On Fri,  3 Jan 2020 16:18:41 +0100
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
>
> But isn't the package name already present in
> http://autobuild.buildroot.net/stats/index.json ? All the subnodes of
> /packages/ are nodes named after the packages.

When handling a package node I dont have the knowledge of the name. Or
I have to passing the name also to the render function:
https://github.com/hthiery/buildroot-stats/blob/master/app/routes.py#L67

This is also possible. But I thought it is a good idea to do so.

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

[Buildroot] [PATCH 03/11] support/scripts/pkg-stats: store patch info in a hash

[Heiko Thiery]

Am Fr., 3. Jan. 2020 um 17:26 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> On Fri, 3 Jan 2020 17:23:20 +0100
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > > Note: I haven't looked at the other patches in your series yet. Maybe
> > > you add more entries to the dict that make it more relevant.
> >
> > No there are no other entries in this dict.
> > But thinking over that I consider removing the count filed because we
> > can calculate the count from the length of the list of patches?! What
> > do you think?
>
> Yes, also thought about this when reviewing. I also saw we calculate
> this count in a few places, so was not sure if it was that great to
> re-do the count several times. But it's not too many times so indeed,
> perhaps it's easier to keep the patch list around.
>
> But in fact, why do you need the list of patches ? Your web site only
> shows the patch count anyway.

The patches are listed in package detail ... see a the bottom of the page:
http://packages.buildroot.thiery.it/packages/package/android-tools


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

[Buildroot] [PATCH 05/11] support/scripts/pkg-stats: parse and set developers info

[Heiko Thiery]

Am Fr., 3. Jan. 2020 um 16:26 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> Hello,
>
> On Fri,  3 Jan 2020 16:18:42 +0100
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > +class Developer:
> > +    def __init__(self, name, files):
> > +        self.name = name
> > +        self.files = files
> > +
> > +def parse_developers(basepath=None):
> > +    """Parse the DEVELOPERS file and return a list of Developer objects."""
> > +    developers = []
> > +    linen = 0
> > +    if basepath is None:
> > +        basepath = os.getcwd()
> > +    with open(os.path.join(basepath, "DEVELOPERS"), "r") as f:
> > +        files = []
> > +        name = None
> > +        for line in f:
> > +            line = line.strip()
> > +            if line.startswith("#"):
> > +                continue
> > +            elif line.startswith("N:"):
> > +                if name is not None or len(files) != 0:
> > +                    print("Syntax error in DEVELOPERS file, line %d" % linen)
> > +                name = line[2:].strip()
> > +            elif line.startswith("F:"):
> > +                fname = line[2:].strip()
> > +                #dev_files = glob.glob(os.path.join(basepath, fname))
> > +                dev_files = glob.glob(fname)
> > +                if len(dev_files) == 0:
> > +                    print("WARNING: '%s' doesn't match any file" % fname)
> > +                files += dev_files
> > +            elif line == "":
> > +                if not name:
> > +                    continue
> > +                developers.append(Developer(name, files))
> > +                files = []
> > +                name = None
> > +            else:
> > +                print("Syntax error in DEVELOPERS file, line %d: '%s'" % (linen, line))
> > +
> > +                return None
> > +            linen += 1
> > +    # handle last developer
> > +    if name is not None:
> > +        developers.append(Developer(name, files))
> > +    return developers
>
> You have duplicated this function from utils/getdeveloperlib.py, which
> is intended to be used as a Python module. Why don't we try to use it,
> instead ?

I also thought about that ... should we then move the pkg-stats script
to the utils folder?

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

[Buildroot] [PATCH 06/11] support/scripts/pkg-stats: store licences of package

[Heiko Thiery]

Am Fr., 3. Jan. 2020 um 16:28 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> On Fri,  3 Jan 2020 16:18:43 +0100
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
>
> Just curious: how does this gets used in your statistics site ? I see
> that you show whether the package has a license or not, but I don't see
> the license details being exposed.

The license used by a package is show on the package detail page:
http://packages.buildroot.thiery.it/packages/package/android-tools

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

[Buildroot] [PATCH 05/11] support/scripts/pkg-stats: parse and set developers info

[Thomas Petazzoni]

On Fri, 3 Jan 2020 17:32:42 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> > You have duplicated this function from utils/getdeveloperlib.py, which
> > is intended to be used as a Python module. Why don't we try to use it,
> > instead ?  
> 
> I also thought about that ... should we then move the pkg-stats script
> to the utils folder?

Normally, utils/ is for tools that are directly useful to the Buildroot
user/developer, and pkg-stats is not really such a tool. It's kind of
an internal tool we use to generate this JSON/HTML stuff, but the
regular Buildroot developer does not need that.

One idea is to add ../../utils to the Python path used to search for
modules. I think it can be tweaked from inside a Python script. Not
sure if it's the nicest thing to do, though.

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

[Buildroot] [PATCH 07/11] support/scripts/pkg-stats: store dependencies of package

[Heiko Thiery]

Am Fr., 3. Jan. 2020 um 16:29 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> On Fri,  3 Jan 2020 16:18:44 +0100
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
>
> Storing those dependencies do not make sense, because dependencies are
> completely different depending on the Buildroot configuration. Packages
> have optional dependencies, some dependencies on host packages only
> exist depending on the system configuration, etc. So exposing those
> dependencies is never going to give anything that is correct.

Thought it is a good idea because I saw that on the
https://layers.openembedded.org/layerindex/recipe/45257/. They also
show the dependencies with a note that this can differ dependent on
the real used configuration. But if you think it is too confusing I
can drop that.

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

[Buildroot] [PATCH 10/11] support/scripts/pkg-stats: add package count to stats

[Heiko Thiery]

Am Fr., 3. Jan. 2020 um 16:35 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> On Fri,  3 Jan 2020 16:18:47 +0100
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> > ---
> >  support/scripts/pkg-stats | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> > index dd3c6566a0..afd9cacafb 100755
> > --- a/support/scripts/pkg-stats
> > +++ b/support/scripts/pkg-stats
> > @@ -457,21 +457,22 @@ def check_package_latest_version(packages):
> >          pkg.rm_version['version'] == pkg.current_version
> >
> >          if pkg.rm_version['status'] == RM_API_STATUS_ERROR:
> > -            pkg.status['version'] = ('warning', 'rm api error')
> > +            pkg.status['version'] = ('warning', 'RM API error')
> >          elif pkg.rm_version['status'] == RM_API_STATUS_NOT_FOUND:
> > -            pkg.status['version'] = ('warning', 'rm package not found')
> > +            pkg.status['version'] = ('warning', 'RM package not found')
> >
> >          if pkg.rm_version['version'] is None:
> >              pkg.status['version'] = ('warning', 'no upstream version available')
> >          elif pkg.rm_version['version'] != pkg.current_version:
> > -            pkg.status['version'] = ('error', 'package needs update')
> > +            pkg.status['version'] = ('error', 'Package needs update')
> >          else:
> > -            pkg.status['version'] = ('ok', 'up-to-date')
> > +            pkg.status['version'] = ('ok', 'update-to-date')
> >      del http_pool
>
> The change up to here is not related to the commit title.

there I made a mistake

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

[Buildroot] [PATCH 08/11] support/scripts/pkg-stats: add generic package status field

[Heiko Thiery]

Am Fr., 3. Jan. 2020 um 16:34 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> Hello,
>
> On Fri,  3 Jan 2020 16:18:45 +0100
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> > index 324700adbf..b0c2db2b93 100755
> > --- a/support/scripts/pkg-stats
> > +++ b/support/scripts/pkg-stats
> > @@ -112,6 +112,7 @@ class Package:
> >          self.url_status = None
> >          self.url_worker = None
> >          self.rm_version = {'status': RM_API_STATUS_ERROR, 'version': None, 'id': None}
> > +        self.status = {}
> >
> >      def pkgvar(self):
> >          return self.name.upper().replace("-", "_")
> > @@ -121,6 +122,7 @@ class Package:
> >          Fills in the .url field
> >          """
> >          self.url_status = "No Config.in"
> > +        self.status['url'] = ("error", "no Config.in")
>
> Here it is duplicating the url_status field, I believe we want to avoid
> that.
>
> >          for filename in os.listdir(self.pkg_path):
> >              if fnmatch.fnmatch(filename, 'Config.*'):
> >                  fp = open(os.path.join(self.pkg_path, filename), "r")
> > @@ -128,9 +130,11 @@ class Package:
> >                      if URL_RE.match(config_line):
> >                          self.url = config_line.strip()
> >                          self.url_status = "Found"
> > +                        self.status['url'] = ("ok", "found")
>
> Ditto.
>
> >                          fp.close()
> >                          return
> >                  self.url_status = "Missing"
> > +                self.status['url'] = ("error", "missing")
>
> Ditto.
>
> >                  fp.close()
> >
> >      def set_infra(self):
> > @@ -155,11 +159,16 @@ class Package:
> >          Fills in the .has_license and .has_license_files fields
> >          """
> >          var = self.pkgvar()
> > +        self.status['license'] = ("error", "missing")
> > +        self.status['license-files'] = ("error", "missing")
> >          if var in self.all_licenses:
> >              self.has_license = True
> >              self.license = self.all_licenses[var]
> > +            self.status['license'] = ("ok", "found")
> > +
> >          if var in self.all_license_files:
> >              self.has_license_files = True
> > +            self.status['license-files'] = ("ok", "found")
>
> Here it's duplicating self.has_license and self.has_license_files
> basically.
>
> >      def set_hash_info(self):
> >          """
> > @@ -167,6 +176,10 @@ class Package:
> >          """
> >          hashpath = os.path.join(self.pkg_path, self.name + '.hash')
> >          self.has_hash = os.path.exists(hashpath)
> > +        if self.has_hash:
> > +            self.status['hash'] = ("ok", "found")
> > +        else:
> > +            self.status['hash'] = ("error", "missing")
>
> Same for self.has_hash.
>
> Can we do better ? Maybe the issues is that pkg-stats does both
> extracting the data to a JSON format *and* presenting it as HTML. Maybe
> it should be separated into two tools:

My intention was not to break the existing generation of the html
output. I thought we can remove this duplications later.

>  - One doing the data extraction and generation of JSON
>
>  - One using the JSON to produce the HTML page, which would be replaced
>    by your new site
>
> I'm not sure exactly how to go, but I'm not a big fan of this patch
> that duplicates information that already exists (and which doesn't
> explain why).

So in the first step I will use the "new" data also for the html
generation. And after that we should think about extracting the html
generation to an additional tool/script?

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

[Buildroot] [PATCH 02/11] support/scripts/pkg-stats: store pkg dir path

[Avraham Shukron]

On Fri, Jan 3, 2020 at 5:19 PM Heiko Thiery <heiko.thiery@gmail.com> wrote:

> @@ -88,7 +89,7 @@ class Package:
>          Fills in the .infras field
>          """
>          self.infras = list()
> -        with open(self.path, 'r') as f:
> +        with open(os.path.join(self.pkg_path, self.name + '.mk'), 'r')
> as f:
>              lines = f.readlines()
>              for l in lines:
>                  match = INFRA_RE.match(l)
>

Assuming this code worked before, and that you didn't change the meaning of
`self.path`, I don't see the motivation behind this chunk...
It seems like you are reconstructing `self.path` from `self.pkg_path`. Am
I missing something?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20200103/728a64b8/attachment.html>

[Buildroot] [PATCH 07/11] support/scripts/pkg-stats: store dependencies of package

[Thomas Petazzoni]

On Fri, 3 Jan 2020 17:39:13 +0100
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> Thought it is a good idea because I saw that on the
> https://layers.openembedded.org/layerindex/recipe/45257/. They also
> show the dependencies with a note that this can differ dependent on
> the real used configuration. But if you think it is too confusing I
> can drop that.

Don't get me wrong, it would be nice to have the dependencies of
packages, but it is simply not possible.

Consider a package like this:

FOO_DEPENDENCIES = bar baz

ifeq ($(BR2_PACKAGE_LIBPNG),y)
FOO_DEPENDENCIES += libpng
endif

If you run "make VARS=FOO_DEPENDENCIES printvars" with no Buildroot
configuration defined, or with a Buildroot configuration defined that
has the libpng package disabled, it will return:

FOO_DEPENDENCIES=bar baz

If however, you run the same command with a Buildroot configuration
defined that has the libpng package enabled, it will return:

FOO_DEPENDENCIES=bar baz libpng

So as you can see, there is nothing like "the list of all dependencies
of a package" that you can get with printvars. Lots of dependencies are
optional. Some are even conflicting, like a package can depend on A
*or* B, but not both. Etc.

This is why I think it is not a good idea to show those dependencies,
because they simply cannot be correct "in general": they can only be
correct within the context of a particular Buildroot configuration.

Best regards,

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

[Buildroot] [PATCH 07/11] support/scripts/pkg-stats: store dependencies of package

[Heiko Thiery]

Hi

Am Sa., 4. Jan. 2020 um 10:39 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> On Fri, 3 Jan 2020 17:39:13 +0100
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > Thought it is a good idea because I saw that on the
> > https://layers.openembedded.org/layerindex/recipe/45257/. They also
> > show the dependencies with a note that this can differ dependent on
> > the real used configuration. But if you think it is too confusing I
> > can drop that.
>
> Don't get me wrong, it would be nice to have the dependencies of
> packages, but it is simply not possible.
>
> Consider a package like this:
>
> FOO_DEPENDENCIES = bar baz
>
> ifeq ($(BR2_PACKAGE_LIBPNG),y)
> FOO_DEPENDENCIES += libpng
> endif
>
> If you run "make VARS=FOO_DEPENDENCIES printvars" with no Buildroot
> configuration defined, or with a Buildroot configuration defined that
> has the libpng package disabled, it will return:
>
> FOO_DEPENDENCIES=bar baz
>
> If however, you run the same command with a Buildroot configuration
> defined that has the libpng package enabled, it will return:
>
> FOO_DEPENDENCIES=bar baz libpng
>
> So as you can see, there is nothing like "the list of all dependencies
> of a package" that you can get with printvars. Lots of dependencies are
> optional. Some are even conflicting, like a package can depend on A
> *or* B, but not both. Etc.
>
> This is why I think it is not a good idea to show those dependencies,
> because they simply cannot be correct "in general": they can only be
> correct within the context of a particular Buildroot configuration.

It is no problem for ... Just thought it is a good idea. but I will drop that

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

[Buildroot] [PATCH 05/11] support/scripts/pkg-stats: parse and set developers info

[Arnout Vandecappelle]

On 03/01/2020 17:39, Thomas Petazzoni wrote:
> One idea is to add ../../utils to the Python path used to search for
> modules.

 The reverse would actually make more sense: move the libs to support/scripts
(or support/python) and add that to the path of the stuff in utils.

> I think it can be tweaked from inside a Python script. Not
> sure if it's the nicest thing to do, though.

import sys
sys.path.extend(...)

 I'm not sure though how it deals with relative paths.

 A much simpler approach is to move the python scripts entirely to
support/scripts and symlink them from utils. That works.

 Regards,
 Arnout

[Buildroot] [PATCH 05/11] support/scripts/pkg-stats: parse and set developers info

[Heiko Thiery]

Am So., 5. Jan. 2020 um 20:18 Uhr schrieb Arnout Vandecappelle <arnout@mind.be>:
>
>
>
> On 03/01/2020 17:39, Thomas Petazzoni wrote:
> > One idea is to add ../../utils to the Python path used to search for
> > modules.
>
>  The reverse would actually make more sense: move the libs to support/scripts
> (or support/python) and add that to the path of the stuff in utils.
>
> > I think it can be tweaked from inside a Python script. Not
> > sure if it's the nicest thing to do, though.
>
> import sys
> sys.path.extend(...)

I think with that the script can only be executed from a distinct
location. On the other hand this is already a hard requirement because
the search for packages is done from toplevel.

>  I'm not sure though how it deals with relative paths.
>
>  A much simpler approach is to move the python scripts entirely to
> support/scripts and symlink them from utils. That works.

With doing that with a symlink the above-mentioned could be avoided.

>  Regards,
>  Arnout