Rewrite metrics-gathering scripts

Rewrite the scripts that gather the metrics to be more generic.

Extract the metrics repository cloning out so that we don't have to
repeatedly clone it.

Make the scripts parse their arguments using getopt and be more specific
about what they're passed.  In particular, this means that for the patch
review run we pass the _repository_ that we're scanning so we can do git
operations on it, and the base of the _layers_ (either a layer, or a
directory containing layers) so we know what to scan.

Be more clever when identifying what commits we need to analyse for
patch review: instead of iterating through a set randomly, we can keep
the revision list sorted and the checkout operations are a lot faster.

Remove the commit/file count metric addition as patchreview itself does
that now.

Add an explicit --push option so it's easy to test the scripts in
isolation without pushing.

Signed-off-by: Ross Burton <ross.burton@arm.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

config.json

@@ -1208,12 +1208,18 @@
                 "BB_SERVER_TIMEOUT = '0'"
             ],
             "step1" : {
-                "shortname" : "Generating patch metrics",
+                "shortname" : "Fetching metrics repositories",
-                "EXTRACMDS" : ["../../yocto-autobuilder-helper/scripts/run-patchmetrics ../ ../meta/ ${HELPERRESULTSDIR}/../../patchmetrics . ${HELPERBRANCHNAME}"]
+                "EXTRAPLAINCMDS" : [
+                    "git clone ssh://git@push.yoctoproject.org/yocto-metrics"
+                ]
             },
             "step2" : {
-                "shortname" : "Running CVE checks",
+                "shortname" : "Generating patch metrics for Poky",
-                "EXTRACMDS" : ["../../yocto-autobuilder-helper/scripts/run-cvecheck ../ ../meta/ ${HELPERRESULTSDIR}/../../patchmetrics . ${HELPERBRANCHNAME}"]
+                "EXTRACMDS" : ["${SCRIPTSDIR}/run-patchmetrics --poky ../ --metrics ../yocto-metrics --repo ../ --layer ../meta --branch ${HELPERBRANCHNAME} --results ${HELPERRESULTSDIR}/../../patchmetrics --push"]
+            },
+            "step3" : {
+                "shortname" : "Running CVE checks for Poky",
+                "EXTRACMDS" : ["${SCRIPTSDIR}/run-cvecheck --metrics ../yocto-metrics --branch ${HELPERBRANCHNAME} --results ${HELPERRESULTSDIR}/../../patchmetrics --push"]
             }
 
         },

scripts/cve-generate-chartdata

@@ -11,8 +11,12 @@ args.add_argument("-j", "--json", help="JSON data file to use")
 args.add_argument("-r", "--resultsdir", help="results directory to parse")
 args = args.parse_args()
 
-with open(args.json) as f:
+try:
-    counts = json.load(f)
+    with open(args.json) as f:
+        counts = json.load(f)
+except FileNotFoundError:
+    # if the file does not exist, start with an empty database.
+    counts = {}
 
 lastyear = {}
 

scripts/patchmetrics-update

@@ -1,65 +1,76 @@
 #!/usr/bin/env python3
-import json, os.path, collections
+
-import sys
+import json
+import pathlib
 import argparse
 import subprocess
 import tempfile
+import sys
+
+# Given a git repository and a base to search for layers (either a layer
+# directly, or a directory containing layers), run the patchscript and update
+# the specified JSON file.
 
 args = argparse.ArgumentParser(description="Update Patch Metrics")
-args.add_argument("-j", "--json", help="update JSON")
+args.add_argument("-j", "--json", required=True, type=pathlib.Path, help="update specified JSON file")
-args.add_argument("-m", "--metadata", help="metadata directry to scan")
+args.add_argument("-s", "--patchscript", required=True, type=pathlib.Path, help="patchreview script to run")
-args.add_argument("-s", "--patchscript", help="review script to use")
+args.add_argument("-r", "--repo", required=True, type=pathlib.Path, help="repository to use (e.g. path/to/poky)")
-args.add_argument("-r", "--repo", help="repository to use")
+args.add_argument("-l", "--layer", type=pathlib.Path, help="layer/repository to scan")
 args = args.parse_args()
 
-status_values = ["accepted", "pending", "inappropriate", "backport", "submitted", "denied", "inactive-upstream", "malformed-upstream-status", "malformed-sob"]
+if not args.repo.is_dir():
+    print(f"{args.repo} is not a directory")
+    sys.exit(1)
+
+if not args.layer.is_dir():
+    print(f"{args.layer} is not a directory")
+    sys.exit(1)
+
+if not args.patchscript.is_file():
+    print(f"{args.patchscript} is not a file")
+
 
 print("Running patchmetrics-update")
 
-with tempfile.TemporaryDirectory(prefix="patchmetrics-") as tempdir:
+if "meta-openembedded" in args.layer.name:
-    subprocess.check_call(["git", "clone", args.repo, tempdir])
+    # 2023-01-01, arbitarily chosen
-    repo_revisions = subprocess.check_output(["git", "rev-list", "--since", "1274625106", "origin/master"], universal_newlines=True, cwd=tempdir).strip().split()
+    epoch = "1672531200"
+else:
+    # 2011-04-25, Yocto 1.0 release.
+    epoch = "1301074853"
 
-    with open(args.json) as f:
+with tempfile.TemporaryDirectory(prefix="patchmetrics-") as tempname:
-        data = json.load(f)
+    tempdir = pathlib.Path(tempname)
+    layerdir = tempdir / args.layer.relative_to(args.repo)
 
-    seen = set()
+    # Create a temporary clone of the repository as we'll be checking out different revisions
-    for i in data:
+    print(f"Making a temporary clone of {args.repo} to {tempdir}")
-        if "commit" in i:
+    subprocess.check_call(["git", "clone", "--quiet", args.repo, tempdir])
-            seen.add(i["commit"])
 
-    needed_revisions = set(repo_revisions) - seen
+    # Identify what revisions need to be analysed. If we reverse the list, keep
+    # order, and remove in-place, then iterating through a large number of
+    # commits is faster.
+    repo_revisions = subprocess.check_output(["git", "rev-list", "--reverse", "--since", epoch, "origin/master"], universal_newlines=True, cwd=tempdir).strip().split()
+    revision_count = len(repo_revisions)
 
-    print("Need to scan revisions %s" % str(needed_revisions))
+    if args.json.exists():
+        with open(args.json) as f:
+            data = json.load(f)
 
-    print("Needed %s revisions (%s and %s in data sets)" % (len(needed_revisions), len(seen), len(repo_revisions)))
+        seen = set()
+        for i in data:
+            try:
+                repo_revisions.remove(i["commit"])
+            except ValueError:
+                pass
 
-    for rev in needed_revisions:
+    new_count = len(repo_revisions)
+    print("Found %s, need to scan %d revisions:\n%s" % (revision_count - new_count, new_count, str(repo_revisions)))
+
+    # Run the patchreview script for every revision
+    for rev in repo_revisions:
         print("Processing %s" % rev)
-        subprocess.check_output(["git", "reset", "--hard", rev], universal_newlines=True, cwd=tempdir).strip()
+        subprocess.check_call(["git", "checkout", "--detach", rev], cwd=tempdir)
-        subprocess.check_call([args.patchscript, os.path.join(tempdir, os.path.relpath(args.metadata, args.repo)), "-j", args.json])
+        subprocess.check_call([args.patchscript, "--json", args.json, layerdir])
-
-    newdata = []
-    with open(args.json) as f:
-        data = json.load(f)
-
-    for i in data:
-        if "commit" not in i:
-            print("Ignoring data with no commit %s" % str(i))
-            continue
-
-        if "commit_count" not in i:
-            i['commit_count'] = subprocess.check_output(["git", "rev-list", "--count", i['commit']], universal_newlines=True, cwd=tempdir).strip()
-        if "recipe_count" not in i:
-            subprocess.check_output(["git", "reset", "--hard", i['commit']], universal_newlines=True, cwd=tempdir).strip()
-            i['recipe_count'] = subprocess.check_output(["find meta -type f -name *.bb  | wc -l"], shell=True, universal_newlines=True, cwd=tempdir).strip()
-
-        #print(i['commit_count'] + " " + i['recipe_count'])
-
-        newdata.append(i)
-
-    with open(args.json, "w") as f:
-        json.dump(newdata, f, sort_keys=True, indent="\t")
 
 print("Finished patchmetrics-update")
-

scripts/run-cvecheck

@@ -2,48 +2,102 @@
 #
 # SPDX-License-Identifier: GPL-2.0-only
 #
-PARENTDIR=`realpath $1`
+
-TARGETDIR=`realpath $2`
+set -eu
-RESULTSDIR=`realpath -m $3`
+
-BUILDDIR=`realpath $4`
+ARGS=$(getopt -o '' --long 'metrics:,branch:,results:,push' -n 'run-cvecheck' -- "$@")
-BRANCH=$5
+if [ $? -ne 0 ]; then
-OURDIR=`dirname $0`
+    echo 'Cannot parse arguments...' >&2
+    exit 1
+fi
+eval set -- "$ARGS"
+unset ARGS
+
+# Location of the yocto-autobuilder-helper scripts
+OURDIR=$(dirname $0)
+# The metrics repository to use
+METRICSDIR=""
+# Where to copy results to
+RESULTSDIR=""
+# The branch we're building
+BRANCH=""
+# Whether to push the metrics
+PUSH=0
+
+while true; do
+    case "$1" in
+        '--metrics')
+            METRICSDIR=$(realpath $2)
+            shift 2
+            continue
+        ;;
+        '--branch')
+            BRANCH=$2
+            shift 2
+            continue
+        ;;
+        '--results')
+            RESULTSDIR=$(realpath -m $2)
+            shift 2
+            continue
+        ;;
+        '--push')
+            PUSH=1
+            shift
+            continue
+        ;;
+        '--')
+            shift
+            break
+        ;;
+        *)
+            echo "Unexpected value $1" >&2
+            exit 1
+        ;;
+    esac
+done
 
 TIMESTAMP=`date +"%s"`
 
+if ! test "$METRICSDIR" -a "$BRANCH" -a "$RESULTSDIR"; then
+    echo "Not all required options specified"
+    exit 1
+fi
+
 #
 # CVE Checks
 #
-if [ ! -e $PARENTDIR/yocto-metrics ]; then
-    git clone ssh://git@push.yoctoproject.org/yocto-metrics $PARENTDIR/yocto-metrics
-fi
-
 if [ ! -d $RESULTSDIR ]; then
     mkdir $RESULTSDIR
 fi
 
-mkdir -p $PARENTDIR/yocto-metrics/cve-check/$BRANCH/
 cd ..
+set +u
 . oe-init-build-env build
+set -u
 bitbake world --runall cve_check -R conf/distro/include/cve-extra-exclusions.inc
+
 if [ -e tmp/log/cve/cve-summary.json ]; then
-    git -C $PARENTDIR/yocto-metrics rm cve-check/$BRANCH/*.json
+    git -C $METRICSDIR rm --ignore-unmatch cve-check/$BRANCH/*.json
-    mkdir -p $PARENTDIR/yocto-metrics/cve-check/$BRANCH
+    mkdir -p $METRICSDIR/cve-check/$BRANCH/
-    cp tmp/log/cve/cve-summary.json $PARENTDIR/yocto-metrics/cve-check/$BRANCH/$TIMESTAMP.json
+    cp tmp/log/cve/cve-summary.json $METRICSDIR/cve-check/$BRANCH/$TIMESTAMP.json
-    git -C $PARENTDIR/yocto-metrics add cve-check/$BRANCH/$TIMESTAMP.json
+    git -C $METRICSDIR add cve-check/$BRANCH/$TIMESTAMP.json
-    git -C $PARENTDIR/yocto-metrics commit -asm "Autobuilder adding new CVE data for branch $BRANCH"
+    git -C $METRICSDIR commit -asm "Autobuilder adding new CVE data for branch $BRANCH" || true
-    git -C $PARENTDIR/yocto-metrics push
+    if [ "$PUSH" = "1" ]; then
+        git -C $METRICSDIR push
+    fi
     $OURDIR/cve-report.py tmp/log/cve/cve-summary.json > $RESULTSDIR/cve-status-$BRANCH.txt
 fi
 
 if [ "$BRANCH" = "master" ]; then
-    mkdir -p $PARENTDIR/yocto-metrics/cve-check/
+    mkdir -p $METRICSDIR/cve-check/$BRANCH/
-    $OURDIR/cve-generate-chartdata --json $PARENTDIR/yocto-metrics/cve-count-byday.json --resultsdir $PARENTDIR/yocto-metrics/cve-check/
+    $OURDIR/cve-generate-chartdata --json $METRICSDIR/cve-count-byday.json --resultsdir $METRICSDIR/cve-check/
-    git -C $PARENTDIR/yocto-metrics add cve-count-byday.json
+    git -C $METRICSDIR add cve-count-byday.json
-    git -C $PARENTDIR/yocto-metrics commit -asm "Autobuilder updating CVE counts"
+    git -C $METRICSDIR commit -asm "Autobuilder updating CVE counts" || true
-    git -C $PARENTDIR/yocto-metrics push
+    if [ "$PUSH" = "1" ]; then
+        git -C $METRICSDIR push
+    fi
 
-    cp $PARENTDIR/yocto-metrics/cve-count-byday.json $RESULTSDIR
+    cp $METRICSDIR/cve-count-byday.json $RESULTSDIR
-    cp $PARENTDIR/yocto-metrics/cve-count-byday-lastyear.json $RESULTSDIR
+    cp $METRICSDIR/cve-count-byday-lastyear.json $RESULTSDIR
 fi
-

scripts/run-patchmetrics

@@ -2,34 +2,114 @@
 #
 # SPDX-License-Identifier: GPL-2.0-only
 #
-PARENTDIR=`realpath $1`
-TARGETDIR=`realpath $2`
-RESULTSDIR=`realpath -m $3`
-BUILDDIR=`realpath $4`
-BRANCH=$5
-OURDIR=`dirname $0`
 
-TIMESTAMP=`date +"%s"`
+set -eu
+
+ARGS=$(getopt -o '' --long 'poky:,metrics:,repo:,layer:,branch:,results:,push' -n 'run-patchmetrics' -- "$@")
+if [ $? -ne 0 ]; then
+    echo 'Cannot parse arguments...' >&2
+    exit 1
+fi
+eval set -- "$ARGS"
+unset ARGS
+
+# Location of the yocto-autobuilder-helper scripts
+OURDIR=$(dirname $0)
+# Where Poky is (for patchreview.py)
+POKYDIR=""
+# The metrics repository to use
+METRICSDIR=""
+# Where to copy results to
+RESULTSDIR=""
+# The branch we're building
+BRANCH=""
+# The layer/repository to scan
+LAYERDIR=""
+# Whether to push the metrics
+PUSH=0
+
+while true; do
+    case "$1" in
+        '--poky')
+            POKYDIR=$(realpath $2)
+            shift 2
+            continue
+        ;;
+        '--metrics')
+            METRICSDIR=$(realpath $2)
+            shift 2
+            continue
+        ;;
+        '--layer')
+            LAYERDIR=$(realpath $2)
+            shift 2
+            continue
+        ;;
+        '--repo')
+            REPODIR=$(realpath $2)
+            shift 2
+            continue
+        ;;
+        '--branch')
+            BRANCH=$2
+            shift 2
+            continue
+        ;;
+        '--results')
+            RESULTSDIR=$(realpath -m $2)
+            shift 2
+            continue
+        ;;
+        '--push')
+            PUSH=1
+            shift
+            continue
+        ;;
+        '--')
+            shift
+            break
+        ;;
+        *)
+            echo "Unexpected value $1" >&2
+            exit 1
+        ;;
+    esac
+done
+
+if ! test "$POKYDIR" -a "$METRICSDIR" -a "$REPODIR" -a "$LAYERDIR" -a "$BRANCH" -a "$RESULTSDIR"; then
+    echo "Not all required options specified"
+    exit 1
+fi
 
 # We only monitor patch metrics on the master branch
 if [ "$BRANCH" != "master" ]; then
+    echo "Skipping, $BRANCH is not master"
     exit 0
 fi
 
 #
 # Patch Metrics
 #
-if [ ! -e $PARENTDIR/yocto-metrics ]; then
+
-    git clone ssh://git@push.yoctoproject.org/yocto-metrics $PARENTDIR/yocto-metrics
+set -x
+$OURDIR/patchmetrics-update --patchscript $POKYDIR/scripts/contrib/patchreview.py --json $METRICSDIR/patch-status.json --repo $REPODIR --layer $LAYERDIR
+set +x
+
+# Allow the commit to fail if there is nothing to commit
+git -C $METRICSDIR commit -asm "Autobuilder adding new patch stats" || true
+
+if [ "$PUSH" = "1" ]; then
+    git -C $METRICSDIR push
 fi
-$OURDIR/patchmetrics-update --repo $PARENTDIR --patchscript $PARENTDIR/scripts/contrib/patchreview.py --metadata $TARGETDIR --json $PARENTDIR/yocto-metrics/patch-status.json
+
-git -C $PARENTDIR/yocto-metrics commit -asm "Autobuilder adding new patch stats"
+#
-git -C $PARENTDIR/yocto-metrics push
+# Update the results
+#
 
 if [ ! -d $RESULTSDIR ]; then
     mkdir $RESULTSDIR
 fi
 
-$OURDIR/patchmetrics-generate-chartdata --json $PARENTDIR/yocto-metrics/patch-status.json --outputdir $RESULTSDIR
+$OURDIR/patchmetrics-generate-chartdata --json $METRICSDIR/patch-status.json --outputdir $RESULTSDIR
-cp $PARENTDIR/yocto-metrics/patch-status.json $RESULTSDIR
+cp $METRICSDIR/patch-status.json $RESULTSDIR
-cp $PARENTDIR/yocto-metrics/patch-status/* $RESULTSDIR
+cp $METRICSDIR/patch-status/* $RESULTSDIR