pkg-stats json output improvements

From: Heiko Thiery <[email protected]>

This patch changes the type of the latest_version variable to a dict. This is for better readability/usability of the data. With this the json output is more descriptive in later processing of the json output.

Signed-off-by Heiko Thiery <[email protected]>

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 33 +++++++++++++++++----------------
  1 file changed, 17 insertions(+), 16 deletions(-)

support/scripts/pkg-stats +3 -4

index 7721d98459..8cc78f2f66 100755
@@ -71,7 +71,7 @@ class Package:
          self.url_status = None
          self.url_worker = None
          self.cves = list()
-        self.latest_version = (RM_API_STATUS_ERROR, None, None)
+        self.latest_version = {'status': RM_API_STATUS_ERROR, 'version': None, 'id': None}
 
      def pkgvar(self):
          return self.name.upper().replace("-", "_")

@@ -460,9 +460,8 @@ def check_package_latest_version(packages):
      """
      Fills in the .latest_version field of all Package objects
 
-    This field has a special format:
-      (status, version, id)
-    with:
+    This field is a dict and has the following keys:
+
      - status: one of RM_API_STATUS_ERROR,
        RM_API_STATUS_FOUND_BY_DISTRO, RM_API_STATUS_FOUND_BY_PATTERN,
        RM_API_STATUS_NOT_FOUND

Hello Heiko and all,

Quoted 6 lines · Heiko Thiery

From: Heiko Thiery <[email protected]>

This patch changes the type of the latest_version variable to a dict. This is for better readability/usability of the data. With this the json output is more descriptive in later processing of the json output.

Quoted 2 lines

Signed-off-by Heiko Thiery <[email protected]>

Signed-off-by Heiko Thiery <[email protected]>

You signed off with both your personal and work email addresses, was this intended ?

Quoted 30 lines

---
  support/scripts/pkg-stats | 33 +++++++++++++++++----------------
  1 file changed, 17 insertions(+), 16 deletions(-)

support/scripts/pkg-stats +3 -4

index 7721d98459..8cc78f2f66 100755
@@ -71,7 +71,7 @@ class Package:
          self.url_status = None
          self.url_worker = None
          self.cves = list()
-        self.latest_version = (RM_API_STATUS_ERROR, None, None)
+        self.latest_version = {'status': RM_API_STATUS_ERROR, 'version': None, 'id': None}
 
      def pkgvar(self):
          return self.name.upper().replace("-", "_")

@@ -460,9 +460,8 @@ def check_package_latest_version(packages):
      """
      Fills in the .latest_version field of all Package objects
 
-    This field has a special format:
-      (status, version, id)
-    with:
+    This field is a dict and has the following keys:
+
      - status: one of RM_API_STATUS_ERROR,
        RM_API_STATUS_FOUND_BY_DISTRO, RM_API_STATUS_FOUND_BY_PATTERN,
        RM_API_STATUS_NOT_FOUND

support/scripts/pkg-stats def check_package_latest_version(packages): · line 482

Quoted 7 lines

support/scripts/pkg-stats +3 -1

@@ -478,7 +477,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.latest_version['status'] = r[0]
+        pkg.latest_version['version'] = r[1]
+        pkg.latest_version['id'] = r[2]

Bikeshedding, but maybe more elegant:

pkg.latest_version = dict(zip(['status', 'version', 'id'], r))

From: Heiko Thiery <[email protected]>

Signed-off-by Heiko Thiery <[email protected]>

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

support/scripts/pkg-stats +1 -0

index 8cc78f2f66..4c963cef0f 100755
@@ -65,6 +65,7 @@ class Package:
          self.has_license_files = False
          self.has_hash = False
          self.patch_count = 0
+        self.patch_files = []
          self.warnings = 0
          self.current_version = None
          self.url = None

Heiko,

Quoted 4 lines · Heiko Thiery

From: Heiko Thiery <[email protected]>

Signed-off-by Heiko Thiery <[email protected]>

Signed-off-by Heiko Thiery <[email protected]>

Same as the previous patch, signed off with 2 different email addresses.

Quoted 17 lines

---
  support/scripts/pkg-stats | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

support/scripts/pkg-stats +1 -0

index 8cc78f2f66..4c963cef0f 100755
@@ -65,6 +65,7 @@ class Package:
          self.has_license_files = False
          self.has_hash = False
          self.patch_count = 0
+        self.patch_files = []
          self.warnings = 0
          self.current_version = None
          self.url = None

support/scripts/pkg-stats class Package: · line 138

Quoted 9 lines

support/scripts/pkg-stats +2 -2

@@ -131,10 +132,10 @@ class Package:
          """
          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'))
+            self.patch_files = fnmatch.filter(os.listdir(subdir), '*.patch')
+            self.patch_count = len(self.patch_files)

We can compute the patch_count only once after looping through all the files, no need to update it at each step.

Maybe we can also entirely remove this patch_count attribute from the Package class, and define it as a property:

class Package:

     # ...
     @property
     def patch_count(self):
         return len(self.patch_files)

Quoted 1 line

Quoted 3 lines

      def set_current_version(self):
          """

Best regards,

Titouan

Use the function 'parse_developers' function from getdeveloperlib that collect the information about the developers and the files they maintain. Then set the maintainer(s) to each package.

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 16 ++++++++++++++++
  1 file changed, 16 insertions(+)

support/scripts/pkg-stats +4 -0

index 4c963cef0f..643272e9d2 100755
@@ -30,10 +30,14 @@ import certifi
  import distutils.version
  import time
  import gzip
+import sys
  from urllib3 import HTTPSConnectionPool
  from urllib3.exceptions import HTTPError
  from multiprocessing import Pool
 
+sys.path.append('utils/')
+from getdeveloperlib import parse_developers
+ >   NVD_START_YEAR = 2002
  NVD_JSON_VERSION = "1.0"
  NVD_BASE_URL = "https://nvd.nist.gov/feeds/json/cve/" + NVD_JSON_VERSION

@@ -169,6 +173,15 @@ class Package:
          """

support/scripts/pkg-stats +4 -0

+
      def __eq__(self, other):
          return self.path == other.path
 

@@ -891,6 +904,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 ...")

@@ -902,6 +917,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 ...")

Heiko, all,

Quoted 31 lines · Heiko Thiery

Use the function 'parse_developers' function from getdeveloperlib that collect the information about the developers and the files they maintain. Then set the maintainer(s) to each package.

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 16 ++++++++++++++++
  1 file changed, 16 insertions(+)

support/scripts/pkg-stats +4 -0

index 4c963cef0f..643272e9d2 100755
@@ -30,10 +30,14 @@ import certifi
  import distutils.version
  import time
  import gzip
+import sys
  from urllib3 import HTTPSConnectionPool
  from urllib3.exceptions import HTTPError
  from multiprocessing import Pool
 
+sys.path.append('utils/')
+from getdeveloperlib import parse_developers
+ >   NVD_START_YEAR = 2002
  NVD_JSON_VERSION = "1.0"
  NVD_BASE_URL = "https://nvd.nist.gov/feeds/json/cve/" + NVD_JSON_VERSION

@@ -169,6 +173,15 @@ class Package:
          """
          return cve in self.all_ignored_cves.get(self.pkgvar(), [])
 

support/scripts/pkg-stats class Package: · line 183

Quoted 8 lines

support/scripts/pkg-stats +8 -0

+    def set_developers(self, developers):
+        """
+        Fills in the .developers field
+        """
+        self.developers = list()
+        for dev in developers:
+            if dev.hasfile(self.path):
+                self.developers.append((dev.name))

Bikeshedding again, but maybe more elegant like this:

self.developers = [
dev.name
for dev in developers
if dev.hasfile(self.path)
]

Quoted 22 lines

support/scripts/pkg-stats +4 -0

+
      def __eq__(self, other):
          return self.path == other.path
 

@@ -891,6 +904,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 ...")

@@ -902,6 +917,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 ...")

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

support/scripts/pkg-stats +3 -1

index 643272e9d2..ebaf04465e 100755
@@ -56,7 +56,7 @@ http_pool = None
 
 
  class Package:
-    all_licenses = list()
+    all_licenses = dict()
      all_license_files = list()
      all_versions = dict()
      all_ignored_cves = dict()

@@ -65,6 +65,7 @@ class Package:
          self.name = name
          self.path = path
          self.infras = None
+        self.license = None
          self.has_license = False
          self.has_license_files = False
          self.has_hash = False

@@ -122,6 +123,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
 

Unify the status check information. The status is stored in a tuple. The first entry is the status that can be 'ok', 'warning' or 'error'. The second entry is a verbose message.

The following checks are performed:
- url: status of the URL check
- license: status of the license presence check
- license-files: status of the license file check
- hash: status of the hash file presence check
- patches: status of the patches count check
- pkg-check: status of the check-package script result
- developers: status if a package has developers in the DEVELOPERS file
- version: status of the version check

With that status information the following variables are replaced:
has_license, has_license_files, has_hash, url_status

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 104 +++++++++++++++++++++++++-------------
  1 file changed, 69 insertions(+), 35 deletions(-)

support/scripts/pkg-stats +2 -5

index ebaf04465e..7c8cc81cf2 100755
@@ -66,18 +66,15 @@ class Package:
          self.path = path
          self.infras = None
          self.license = None
-        self.has_license = False
-        self.has_license_files = False
-        self.has_hash = False
          self.patch_count = 0
          self.patch_files = []
          self.warnings = 0
          self.current_version = None
          self.url = None
-        self.url_status = None
          self.url_worker = None
          self.cves = list()
          self.latest_version = {'status': RM_API_STATUS_ERROR, 'version': None, 'id': None}
+        self.status = {}
 
      def pkgvar(self):
          return self.name.upper().replace("-", "_")

@@ -86,17 +83,17 @@ class Package:
          """
          Fills in the .url field
          """
-        self.url_status = "No Config.in"
+        self.status['url'] = ("warning", "no Config.in")
          for filename in os.listdir(os.path.dirname(self.path)):
              if fnmatch.fnmatch(filename, 'Config.*'):

support/scripts/pkg-stats +0 -0

@@ -496,6 +517,18 @@ def check_package_latest_version(packages):
          pkg.latest_version['status'] = r[0]
          pkg.latest_version['version'] = r[1]
          pkg.latest_version['id'] = r[2]

Heiko, all,

First of all, I really like this idea, as it makes the status checks quite easier to extend !

Quoted 58 lines · Heiko Thiery

Unify the status check information. The status is stored in a tuple. The first entry is the status that can be 'ok', 'warning' or 'error'. The second entry is a verbose message.

The following checks are performed:
- url: status of the URL check
- license: status of the license presence check
- license-files: status of the license file check
- hash: status of the hash file presence check
- patches: status of the patches count check
- pkg-check: status of the check-package script result
- developers: status if a package has developers in the DEVELOPERS file
- version: status of the version check

With that status information the following variables are replaced:
has_license, has_license_files, has_hash, url_status

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 104 +++++++++++++++++++++++++-------------
  1 file changed, 69 insertions(+), 35 deletions(-)

support/scripts/pkg-stats +2 -5

index ebaf04465e..7c8cc81cf2 100755
@@ -66,18 +66,15 @@ class Package:
          self.path = path
          self.infras = None
          self.license = None
-        self.has_license = False
-        self.has_license_files = False
-        self.has_hash = False
          self.patch_count = 0
          self.patch_files = []
          self.warnings = 0
          self.current_version = None
          self.url = None
-        self.url_status = None
          self.url_worker = None
          self.cves = list()
          self.latest_version = {'status': RM_API_STATUS_ERROR, 'version': None, 'id': None}
+        self.status = {}
 
      def pkgvar(self):
          return self.name.upper().replace("-", "_")

@@ -86,17 +83,17 @@ class Package:
          """
          Fills in the .url field
          """
-        self.url_status = "No Config.in"
+        self.status['url'] = ("warning", "no Config.in")
          for filename in os.listdir(os.path.dirname(self.path)):
              if fnmatch.fnmatch(filename, 'Config.*'):
                  fp = open(os.path.join(os.path.dirname(self.path), filename), "r")
                  for config_line in fp:
                      if URL_RE.match(config_line):
                          self.url = config_line.strip()

support/scripts/pkg-stats class Package: · line 96

Quoted 6 lines

support/scripts/pkg-stats +2 -2

-                        self.url_status = "Found"
+                        self.status['url'] = ("ok", "found")
                          fp.close()
                          return
-                self.url_status = "Missing"
+                self.status['url'] = ("warning", "missing")

I would set ("error", "missing") for consistency with what is done elsewhere

support/scripts/pkg-stats class Package: · line 99

Quoted 4 lines

support/scripts/pkg-stats +0 -0

                  fp.close()
 
      def set_infra(self):

[-- SNIP --]

Quoted 12 lines

support/scripts/pkg-stats +8 -0

@@ -496,6 +517,18 @@ def check_package_latest_version(packages):
          pkg.latest_version['status'] = r[0]
          pkg.latest_version['version'] = r[1]
          pkg.latest_version['id'] = r[2]
+
+        if pkg.latest_version['status'] == RM_API_STATUS_ERROR:
+            pkg.status['version'] = ('warning', 'RM API error')
+        elif pkg.latest_version['status'] == RM_API_STATUS_NOT_FOUND:
+            pkg.status['version'] = ('warning', 'RM package not found')
+
+        if pkg.latest_version['version'] is None:
+            pkg.status['version'] = ('warning', 'no upstream version available')

support/scripts/pkg-stats def check_package_latest_version(packages): · line 529

Quoted 2 lines

support/scripts/pkg-stats +2 -0

+        elif pkg.latest_version['version'] != pkg.current_version:
+            pkg.status['version'] = ('error', 'package needs update')

We can make the messages a bit more explicit:
* "Release Monitoring API error"
* "Package not found on Release Monitoring"
* "The newer version {} is available
upstream".format(pkg.latest_version['version'])

support/scripts/pkg-stats def check_package_latest_version(packages): · line 534

Quoted 5 lines

support/scripts/pkg-stats +2 -0

+        else:
+            pkg.status['version'] = ('ok', 'up-to-date')
      del http_pool
 
 

[-- SNIP --]

support/scripts/pkg-stats def dump_html_pkg(f, pkg): · line 786

Quoted 11 lines

support/scripts/pkg-stats +4 -2

@@ -746,12 +779,13 @@ def dump_html_pkg(f, pkg):
 
      # URL status
      td_class = ["centered"]
-    url_str = pkg.url_status
-    if pkg.url_status == "Missing" or pkg.url_status == "No Config.in":
+    url_str = pkg.status['url'][1]
+    if pkg.status['url'][0] == "warning":
+        td_class.append("missing_url")
+    elif pkg.status['url'][0] == "error":
          td_class.append("missing_url")

The above 2 conditions can be simplified like:

support/scripts/pkg-stats def dump_html_pkg(f, pkg): · line 791

Quoted 8 lines

support/scripts/pkg-stats +1 -2

-    elif pkg.url_status.startswith("Invalid"):
          td_class.append("invalid_url")
-        url_str = "<a href=%s>%s</a>" % (pkg.url, pkg.url_status)
+        url_str = "<a href=%s>%s</a>" % (pkg.url, pkg.status['url'][1])
      else:
          td_class.append("good_url")
          url_str = "<a href=%s>Link</a>" % pkg.url

Best regards,

Titouan

Signed-off-by Heiko Thiery <heiko.thiery at gmail.

---
  support/scripts/pkg-stats | 1 +
  1 file changed, 1 insertion(+)

support/scripts/pkg-stats +1 -0

index 7c8cc81cf2..90afcc2332 100755
@@ -544,6 +544,7 @@ def check_package_cves(nvd_path, packages):
 
  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

This value can be used for later processing.

In the buildroot-stats application this is used to create links pointing to the git repo of buildroot.

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 1 +
  1 file changed, 1 insertion(+)

support/scripts/pkg-stats +0 -0

index 90afcc2332..36b33586ef 100755

Hello Heiko,

Quoted 6 lines · Heiko Thiery

This value can be used for later processing.

In the buildroot-stats application this is used to create links pointing to the git repo of buildroot.

Signed-off-by Heiko Thiery <[email protected]>

Reviewed-by Titouan Christophe <[email protected]>

Tested-by Titouan Christophe <[email protected]>

Quoted 10 lines

---
  support/scripts/pkg-stats | 1 +
  1 file changed, 1 insertion(+)

support/scripts/pkg-stats +0 -0

index 90afcc2332..36b33586ef 100755

support/scripts/pkg-stats class Package: · line 70

Quoted 7 lines

support/scripts/pkg-stats +1 -0

@@ -64,6 +64,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.license = None
          self.patch_count = 0

Regards,

Titouan

Scan configs directory and create Defconfig objects.

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 42 +++++++++++++++++++++++++++++++++++++--
  1 file changed, 40 insertions(+), 2 deletions(-)

support/scripts/pkg-stats +4 -0

index 36b33586ef..ae70f90485 100755
@@ -54,6 +54,33 @@ RM_API_STATUS_NOT_FOUND = 4
  # because it's used by sub-processes.
  http_pool = None
 
+class Defconfig:
+    def __init__(self, name, path):
+        self.name = name
+        self.path = path

support/scripts/pkg-stats +14 -2

+
 
  class Package:
      all_licenses = dict()

@@ -882,7 +909,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', 'name']

@@ -893,6 +920,12 @@ 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

@@ -903,6 +936,7 @@ def dump_json(packages, stats, date, commit, output):
      # The actual structure to dump, add commit and date to it
      final = {'packages': pkgs,
               'stats': statistics,
+             'defconfigs': defconfigs,
               'commit': commit,
               'date': str(date)}
 

@@ -944,6 +978,10 @@ def __main__():
      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("Getting package make info ...")
      package_init_make_info()
      print("Getting package details ...")

@@ -970,7 +1008,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__()

Store the names of license files and check if they are in the hash file.

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 16 ++++++++++++++--
  1 file changed, 14 insertions(+), 2 deletions(-)

support/scripts/pkg-stats +14 -2

index ae70f90485..e954dd125e 100755
@@ -84,7 +84,7 @@ def get_defconfig_list():
 
  class Package:
      all_licenses = dict()
-    all_license_files = list()
+    all_license_files = dict()
      all_versions = dict()
      all_ignored_cves = dict()
 

@@ -94,6 +94,7 @@ class Package:
          self.pkg_path = os.path.dirname(path)
          self.infras = None
          self.license = None
+        self.license_files = None
          self.patch_count = 0
          self.patch_files = []
          self.warnings = 0

@@ -152,6 +153,7 @@ class Package:
              self.license = self.all_licenses[var]
              self.status['license'] = ("ok", "found")
          if var in self.all_license_files:
+            self.license_files = self.all_license_files[var].split(' ')
              self.status['license-files'] = ("ok", "found")
 
      def set_hash_info(self):

@@ -159,8 +161,18 @@ class Package:
          Fills in the .status['hash'] field
          """
          hashpath = self.path.replace(".mk", ".hash")
+        self.status['hash-license'] = ("na", "no hash file")
          if os.path.exists(hashpath):
              self.status['hash'] = ("ok", "found")
+            self.status['hash-license'] = ("error", "no license in hash file")
+            # check if license files are in hash file
+            if self.license_files is not None:
+                self.status['hash-license'] = ("ok", "found")
+                with open(hashpath) as f:
+                    content = f.read()
+                    for license in self.license_files:
+                        if content.find(license) == -1:
+                            self.status['hash-license'] = ("error", "license missing in hash file")
          else:
              self.status['hash'] = ("error", "missing")
 

@@ -441,7 +453,7 @@ def package_init_make_info():
              if pkgvar.endswith("_MANIFEST_LICENSE_FILES"):
                  continue
              pkgvar = pkgvar[:-14]
-            Package.all_license_files.append(pkgvar)
+            Package.all_license_files[pkgvar] = value
 
          elif pkgvar.endswith("_VERSION"):
              if pkgvar.endswith("_DL_VERSION"):

If there is no infra set or infra is virtual the status is set to 'na'.

This is done for the follwing checks:
- license
- license-files
- hash
- hash-license
- patches
- version

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 26 ++++++++++++++++++++++++++
  1 file changed, 26 insertions(+)

support/scripts/pkg-stats +0 -0

index e954dd125e..be3b6d7e71 100755

Hello Heiko and all,

Quoted 20 lines · Heiko Thiery

If there is no infra set or infra is virtual the status is set to 'na'.

This is done for the follwing checks:
- license
- license-files
- hash
- hash-license
- patches
- version

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 26 ++++++++++++++++++++++++++
  1 file changed, 26 insertions(+)

support/scripts/pkg-stats +0 -0

index e954dd125e..be3b6d7e71 100755

support/scripts/pkg-stats class Package: · line 134

Quoted 10 lines

support/scripts/pkg-stats +7 -0

@@ -125,6 +125,14 @@ class Package:
                  self.status['url'] = ("warning", "missing")
                  fp.close()
 
+    def is_valid_infra(self):
+        try:
+            if self.infras[0][1] == 'virtual':
+                return False
+        except IndexError:
+                return False
+        return True

You might want to make a property of this (like CVE.identifier). I would also suggest has_valid_infra, because a package is not an infrastructure :-)

support/scripts/pkg-stats class Package: · line 157

Quoted 9 lines

support/scripts/pkg-stats +2 -0

+
      def set_infra(self):
          """
          Fills in the .infras field

@@ -146,6 +154,11 @@ class Package:
          """
          Fills in the .status['license'] and .status['license-files'] fields
          """
+        if self.is_valid_infra() == False:

if not self.is_valid_infra():

(or using a property, if not self.has_valid_infra:)

support/scripts/pkg-stats class Package: · line 176

Quoted 12 lines

support/scripts/pkg-stats +5 -0

+            self.status['license'] = ("na", "no valid package infra")
+            self.status['license-files'] = ("na", "no valid package infra")
+            return
+
          var = self.pkgvar()
          self.status['license'] = ("error", "missing")
          self.status['license-files'] = ("error", "missing")

@@ -160,6 +173,11 @@ class Package:
          """
          Fills in the .status['hash'] field
          """
+        if self.is_valid_infra() == False:

same

support/scripts/pkg-stats class Package: · line 201

Quoted 12 lines

support/scripts/pkg-stats +5 -0

+            self.status['hash'] = ("na", "no valid package infra")
+            self.status['hash-license'] = ("na", "no valid package infra")
+            return
+
          hashpath = self.path.replace(".mk", ".hash")
          self.status['hash-license'] = ("na", "no hash file")
          if os.path.exists(hashpath):

@@ -180,6 +198,10 @@ class Package:
          """
          Fills in the .patch_count, .patch_files and .status['patches'] fields
          """
+        if self.is_valid_infra() == False:

same

Quoted 10 lines

support/scripts/pkg-stats +3 -0

+            self.status['patches'] = ("na", "no valid package infra")
+            return
+
          pkgdir = os.path.dirname(self.path)
          for subdir, _, _ in os.walk(pkgdir):
              self.patch_files = fnmatch.filter(os.listdir(subdir), '*.patch')

@@ -558,6 +580,10 @@ def check_package_latest_version(packages):
          pkg.latest_version['version'] = r[1]
          pkg.latest_version['id'] = r[2]
 

support/scripts/pkg-stats def check_package_latest_version(packages): · line 583

Quoted 1 line

support/scripts/pkg-stats +1 -0

+        if pkg.is_valid_infra() == False:

same

Quoted 7 lines

support/scripts/pkg-stats +3 -0

+            pkg.status['version'] = ("na", "no valid package infra")
+            continue
+
          if pkg.latest_version['status'] == RM_API_STATUS_ERROR:
              pkg.status['version'] = ('warning', 'RM API error')
          elif pkg.latest_version['status'] == RM_API_STATUS_NOT_FOUND:

Regards,

Titouan

Also the list of status checks is added to the json output.

Signed-off-by Heiko Thiery <[email protected]> > ---

  support/scripts/pkg-stats | 14 ++++++++++++++
  1 file changed, 14 insertions(+)

support/scripts/pkg-stats +5 -0

index be3b6d7e71..ed22f6b650 100755
@@ -87,6 +87,11 @@ class Package:
      all_license_files = dict()
      all_versions = dict()
      all_ignored_cves = dict()
+    # This is the list of all possible checks. Add new checks to this list so
+    # a tool that post-processeds the json output knows the checks before
+    # iterating over the packages.
+    status_checks = ['cve', 'developers', 'hash', 'hash-license', 'license',
+                     'license-files', 'patches', 'pkg-check', 'url', 'version']
 
      def __init__(self, name, path):
          self.name = name

@@ -103,7 +108,15 @@ class Package:
          self.url_worker = None
          self.cves = list()

support/scripts/pkg-stats +1 -0

      def pkgvar(self):
          return self.name.upper().replace("-", "_")

@@ -975,6 +988,7 @@ def dump_json(packages, defconfigs, stats, date, commit, output):
      final = {'packages': pkgs,
               'stats': statistics,
               'defconfigs': defconfigs,
+             'package_status_checks': Package.status_checks,
               'commit': commit,
               'date': str(date)}
 

Heiko, all,

Quoted 25 lines · Heiko Thiery

Also the list of status checks is added to the json output.

Signed-off-by Heiko Thiery <[email protected]> > ---

  support/scripts/pkg-stats | 14 ++++++++++++++
  1 file changed, 14 insertions(+)

support/scripts/pkg-stats +5 -0

index be3b6d7e71..ed22f6b650 100755
@@ -87,6 +87,11 @@ class Package:
      all_license_files = dict()
      all_versions = dict()
      all_ignored_cves = dict()
+    # This is the list of all possible checks. Add new checks to this list so
+    # a tool that post-processeds the json output knows the checks before
+    # iterating over the packages.
+    status_checks = ['cve', 'developers', 'hash', 'hash-license', 'license',
+                     'license-files', 'patches', 'pkg-check', 'url', 'version']
 
      def __init__(self, name, path):
          self.name = name

@@ -103,7 +108,15 @@ class Package:
          self.url_worker = None
          self.cves = list()

support/scripts/pkg-stats class Package: · line 119

Quoted 10 lines

support/scripts/pkg-stats +8 -0

          self.latest_version = {'status': RM_API_STATUS_ERROR, 'version': None, 'id': None}
+        self.init_check_status()
+
+    def init_check_status(self):
+        """
+        Set the status of all check to 'na'.
+        """
          self.status = {}
+        for check in self.status_checks:
+            self.status[check] = ("na", "no status check done")

In my opinion, it would be better to leave this dictionary empty at initialization.

Because we know the list of status checks that have been run (from the 'package_status_checks' list you introduce below), we can tell that a particular status check has not been done if its name is missing from the status dictionary. This would make the outputted json a tad smaller.

support/scripts/pkg-stats def dump_json(packages, defconfigs, stats, date, commit, output): · line 994

Quoted 12 lines

support/scripts/pkg-stats +1 -0

      def pkgvar(self):
          return self.name.upper().replace("-", "_")

@@ -975,6 +988,7 @@ def dump_json(packages, defconfigs, stats, date, commit, output):
      final = {'packages': pkgs,
               'stats': statistics,
               'defconfigs': defconfigs,
+             'package_status_checks': Package.status_checks,
               'commit': commit,
               'date': str(date)}
 

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)

support/scripts/pkg-stats +1 -2

index ed22f6b650..4efff8624f 100755
@@ -617,8 +617,14 @@ def check_package_cves(nvd_path, packages):
 
      for cve in CVE.read_nvd_dir(nvd_path):
          for pkg_name in cve.pkg_names:
-            if pkg_name in packages and cve.affects(packages[pkg_name]):
-                packages[pkg_name].cves.append(cve.identifier)
+            if pkg_name in packages:

Heiko, all,

Quoted 17 lines · Heiko Thiery

Signed-off-by Heiko Thiery <[email protected]>

---
  support/scripts/pkg-stats | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)

support/scripts/pkg-stats +1 -2

index ed22f6b650..4efff8624f 100755
@@ -617,8 +617,14 @@ def check_package_cves(nvd_path, packages):
 
      for cve in CVE.read_nvd_dir(nvd_path):
          for pkg_name in cve.pkg_names:
-            if pkg_name in packages and cve.affects(packages[pkg_name]):
-                packages[pkg_name].cves.append(cve.identifier)
+            if pkg_name in packages:

support/scripts/pkg-stats def check_package_cves(nvd_path, packages): · line 630

Quoted 10 lines

support/scripts/pkg-stats +7 -0

+                if cve.affects(packages[pkg_name]):
+                    packages[pkg_name].cves.append(cve.identifier)
+                if len(packages[pkg_name].cves):
+                    packages[pkg_name].status['cve'] = ('error', 'affected by cve')
+                else:
+                    packages[pkg_name].status['cve'] = ('ok', 'no cve found')
+
 
 
  def calculate_stats(packages):

In the current state, that would give:

* If a CVE applies to a br package -> error
* If a CVE does not applies to a br package -> ok
* If no CVE points to a br package -> na (no status check done)

I would argue that the latest case is not correct. The status should be ok, because we ran through the whole list of CVEs from the NVD feed, and we did not find any of them that applies to this package.

I would rather write it like this:

########################

support/scripts/pkg-stats +0 -0

index ed22f6b650..91477d583e 100755

support/scripts/pkg-stats +0 -0

  def calculate_stats(packages):
      stats = defaultdict(int)
 ########################

Best regards,

Titouan

Hi Titouan and all,

Am So., 23. Feb. 2020 um 15:24 Uhr schrieb Titouan Christophe <[email protected]>:

Quoted 5 lines

Heiko, all,

Quoted 1 line · Heiko Thiery

Signed-off-by Heiko Thiery <[email protected]>

Quoted 10 lines

I would rather write it like this:

########################

support/scripts/pkg-stats +0 -0

index ed22f6b650..91477d583e 100755

support/scripts/pkg-stats def check_package_cves(nvd_path, packages): · line 628

Quoted 9 lines

support/scripts/pkg-stats +6 -0

@@ -620,6 +620,12 @@ def check_package_cves(nvd_path, packages):
               if pkg_name in packages and cve.affects(packages[pkg_name]):
                   packages[pkg_name].cves.append(cve.identifier)
 
+    for pkg_name, pkg in packages.items():
+        if len(pkg.cves) > 0:
+            pkg.status['cve'] = ('error', 'affected by CVE(s)')
+        else:
+            pkg.status['cve'] = ('ok', 'no CVE found')
+

Isn't it right that we loop then (depending on the amount of nvd pathes) several thousend times?

e.g. packages ~2600, nvds ~ 20 => 20*2600=52000

On the other hand we loop over the list of packages all over the place ;-/

Quoted 10 lines

support/scripts/pkg-stats +0 -0

   def calculate_stats(packages):
       stats = defaultdict(int)
 ########################

Best regards,

Titouan

Quoted 9 lines

Hi Titouan and all,

Am So., 23. Feb. 2020 um 15:24 Uhr schrieb Titouan Christophe <[email protected]>:

Quoted 5 lines

Heiko, all,

Quoted 1 line · Heiko Thiery

Signed-off-by Heiko Thiery <[email protected]>

Quoted 4 lines

Isn't it right that we loop then (depending on the amount of nvd pathes) several thousend times?

e.g. packages ~2600, nvds ~ 20 => 20*2600=52000

Except that each NVD file contains a few thousands CVEs :).

Quoted 2 lines

On the other hand we loop over the list of packages all over the place ;-/

Looping over all CVEs in a single NVD file yields 5 to 10 more iterations than looping over all packages (for instance year 2018 alone has 16039 CVE items)

Quoted 10 lines

Quoted 10 lines

def calculate_stats(packages):
stats = defaultdict(int)
########################

Best regards,

Titouan

This patchset adds 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 more generic status field to the packages for easier post processing

---
v2 -> v3:
- keep variable latest_release but change format
- add check for license file hashes
- introduce a na status for the checks
- add a list of all possible checks to the json output
- add the cve check status

v1 -> v2:
- cleanup and recreation of patches
- remove pkg name from dumping to json
- use patch_files instead of combine count and files in dict
- include getdevelopers.py to reuse Developers class

---
Heiko Thiery (12):
support/scripts/pkg-stats: store latest version in a dict support/scripts/pkg-stats: store patch files for the package support/scripts/pkg-stats: set developers info support/scripts/pkg-stats: store licences of package support/scripts/pkg-stats: add package status support/scripts/pkg-stats: add package count to stats support/scripts/pkg-stats: store pkg dir path support/scripts/pkg-stats: add defconfig support support/scripts/pkg-stats: add support for license hash check support/scripts/pkg-stats: set status to 'na' for virtual packages support/scripts/pkg-stats: initialize all package status checks support/scripts/pkg-stats: add status for cve check

  support/scripts/pkg-stats | 272 +++++++++++++++++++++++++++++---------
  1 file changed, 212 insertions(+), 60 deletions(-)