Props to Yoann Congal for an early version of this work, review, and fixes.
Ross
> On 31 Oct 2023, at 14:49, Ross Burton via lists.yoctoproject.org
> <ross.burton=arm....@lists.yoctoproject.org> wrote:
>
> From: Ross Burton <ross.bur...@arm.com>
>
> 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.bur...@arm.com>
> ---
> config.json | 14 +++--
> scripts/cve-generate-chartdata | 8 ++-
> scripts/patchmetrics-update | 109 +++++++++++++++++---------------
> scripts/run-cvecheck | 106 +++++++++++++++++++++++--------
> scripts/run-patchmetrics | 110 ++++++++++++++++++++++++++++-----
> 5 files changed, 251 insertions(+), 96 deletions(-)
>
> diff --git a/config.json b/config.json
> index 918cf08..f3a4ee7 100644
> --- a/config.json
> +++ b/config.json
> @@ -1208,12 +1208,18 @@
> "BB_SERVER_TIMEOUT = '0'"
> ],
> "step1" : {
> - "shortname" : "Generating patch metrics",
> - "EXTRACMDS" :
> ["../../yocto-autobuilder-helper/scripts/run-patchmetrics ../ ../meta/
> ${HELPERRESULTSDIR}/../../patchmetrics . ${HELPERBRANCHNAME}"]
> + "shortname" : "Fetching metrics repositories",
> + "EXTRAPLAINCMDS" : [
> + "git clone
> ssh://g...@push.yoctoproject.org/yocto-metrics"
> + ]
> },
> "step2" : {
> - "shortname" : "Running CVE checks",
> - "EXTRACMDS" :
> ["../../yocto-autobuilder-helper/scripts/run-cvecheck ../ ../meta/
> ${HELPERRESULTSDIR}/../../patchmetrics . ${HELPERBRANCHNAME}"]
> + "shortname" : "Generating patch metrics for Poky",
> + "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"]
> }
>
> },
> diff --git a/scripts/cve-generate-chartdata b/scripts/cve-generate-chartdata
> index 95d12ff..dbbbe82 100755
> --- a/scripts/cve-generate-chartdata
> +++ b/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:
> - counts = json.load(f)
> +try:
> + 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 = {}
>
> diff --git a/scripts/patchmetrics-update b/scripts/patchmetrics-update
> index 65d351e..2fa7e4a 100755
> --- a/scripts/patchmetrics-update
> +++ b/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("-m", "--metadata", help="metadata directry to scan")
> -args.add_argument("-s", "--patchscript", help="review script to use")
> -args.add_argument("-r", "--repo", help="repository to use")
> +args.add_argument("-j", "--json", required=True, type=pathlib.Path,
> help="update specified JSON file")
> +args.add_argument("-s", "--patchscript", required=True, type=pathlib.Path,
> help="patchreview script to run")
> +args.add_argument("-r", "--repo", required=True, type=pathlib.Path,
> help="repository to use (e.g. path/to/poky)")
> +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"]
> -
> -print("Running patchmetrics-update")
> -
> -with tempfile.TemporaryDirectory(prefix="patchmetrics-") as tempdir:
> - subprocess.check_call(["git", "clone", args.repo, tempdir])
> - repo_revisions = subprocess.check_output(["git", "rev-list", "--since",
> "1274625106", "origin/master"], universal_newlines=True,
> cwd=tempdir).strip().split()
> -
> - with open(args.json) as f:
> - data = json.load(f)
> +if not args.repo.is_dir():
> + print(f"{args.repo} is not a directory")
> + sys.exit(1)
>
> - seen = set()
> - for i in data:
> - if "commit" in i:
> - seen.add(i["commit"])
> +if not args.layer.is_dir():
> + print(f"{args.layer} is not a directory")
> + sys.exit(1)
>
> - needed_revisions = set(repo_revisions) - seen
> +if not args.patchscript.is_file():
> + print(f"{args.patchscript} is not a file")
>
> - print("Need to scan revisions %s" % str(needed_revisions))
>
> - print("Needed %s revisions (%s and %s in data sets)" %
> (len(needed_revisions), len(seen), len(repo_revisions)))
> +print("Running patchmetrics-update")
>
> - for rev in needed_revisions:
> +if "meta-openembedded" in args.layer.name:
> + # 2023-01-01, arbitarily chosen
> + epoch = "1672531200"
> +else:
> + # 2011-04-25, Yocto 1.0 release.
> + epoch = "1301074853"
> +
> +with tempfile.TemporaryDirectory(prefix="patchmetrics-") as tempname:
> + tempdir = pathlib.Path(tempname)
> + layerdir = tempdir / args.layer.relative_to(args.repo)
> +
> + # Create a temporary clone of the repository as we'll be checking out
> different revisions
> + print(f"Making a temporary clone of {args.repo} to {tempdir}")
> + subprocess.check_call(["git", "clone", "--quiet", args.repo, tempdir])
> +
> + # 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)
> +
> + if args.json.exists():
> + with open(args.json) as f:
> + data = json.load(f)
> +
> + seen = set()
> + for i in data:
> + try:
> + repo_revisions.remove(i["commit"])
> + except ValueError:
> + pass
> +
> + 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([args.patchscript, os.path.join(tempdir,
> os.path.relpath(args.metadata, args.repo)), "-j", args.json])
> -
> - 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")
> + subprocess.check_call(["git", "checkout", "--detach", rev],
> cwd=tempdir)
> + subprocess.check_call([args.patchscript, "--json", args.json,
> layerdir])
>
> print("Finished patchmetrics-update")
> -
> diff --git a/scripts/run-cvecheck b/scripts/run-cvecheck
> index 6294fe6..711bec2 100755
> --- a/scripts/run-cvecheck
> +++ b/scripts/run-cvecheck
> @@ -2,48 +2,102 @@
> #
> # 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`
> +
> +set -eu
> +
> +ARGS=$(getopt -o '' --long 'metrics:,branch:,results:,push' -n
> 'run-cvecheck' -- "$@")
> +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)
> +# 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://g...@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
> - mkdir -p $PARENTDIR/yocto-metrics/cve-check/$BRANCH
> - cp tmp/log/cve/cve-summary.json
> $PARENTDIR/yocto-metrics/cve-check/$BRANCH/$TIMESTAMP.json
> - git -C $PARENTDIR/yocto-metrics add cve-check/$BRANCH/$TIMESTAMP.json
> - git -C $PARENTDIR/yocto-metrics commit -asm "Autobuilder adding new CVE
> data for branch $BRANCH"
> - git -C $PARENTDIR/yocto-metrics push
> + git -C $METRICSDIR rm --ignore-unmatch cve-check/$BRANCH/*.json
> + mkdir -p $METRICSDIR/cve-check/$BRANCH/
> + cp tmp/log/cve/cve-summary.json
> $METRICSDIR/cve-check/$BRANCH/$TIMESTAMP.json
> + git -C $METRICSDIR add cve-check/$BRANCH/$TIMESTAMP.json
> + git -C $METRICSDIR commit -asm "Autobuilder adding new CVE data for
> branch $BRANCH" || true
> + 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/
> - $OURDIR/cve-generate-chartdata --json
> $PARENTDIR/yocto-metrics/cve-count-byday.json --resultsdir
> $PARENTDIR/yocto-metrics/cve-check/
> - git -C $PARENTDIR/yocto-metrics add cve-count-byday.json
> - git -C $PARENTDIR/yocto-metrics commit -asm "Autobuilder updating CVE
> counts"
> - git -C $PARENTDIR/yocto-metrics push
> -
> - cp $PARENTDIR/yocto-metrics/cve-count-byday.json $RESULTSDIR
> - cp $PARENTDIR/yocto-metrics/cve-count-byday-lastyear.json $RESULTSDIR
> -fi
> + mkdir -p $METRICSDIR/cve-check/$BRANCH/
> + $OURDIR/cve-generate-chartdata --json $METRICSDIR/cve-count-byday.json
> --resultsdir $METRICSDIR/cve-check/
> + git -C $METRICSDIR add cve-count-byday.json
> + git -C $METRICSDIR commit -asm "Autobuilder updating CVE counts" || true
> + if [ "$PUSH" = "1" ]; then
> + git -C $METRICSDIR push
> + fi
>
> + cp $METRICSDIR/cve-count-byday.json $RESULTSDIR
> + cp $METRICSDIR/cve-count-byday-lastyear.json $RESULTSDIR
> +fi
> diff --git a/scripts/run-patchmetrics b/scripts/run-patchmetrics
> index 391ac45..a75cf52 100755
> --- a/scripts/run-patchmetrics
> +++ b/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://g...@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
> -cp $PARENTDIR/yocto-metrics/patch-status.json $RESULTSDIR
> -cp $PARENTDIR/yocto-metrics/patch-status/* $RESULTSDIR
> +$OURDIR/patchmetrics-generate-chartdata --json $METRICSDIR/patch-status.json
> --outputdir $RESULTSDIR
> +cp $METRICSDIR/patch-status.json $RESULTSDIR
> +cp $METRICSDIR/patch-status/* $RESULTSDIR
> --
> 2.34.1
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#61534): https://lists.yoctoproject.org/g/yocto/message/61534
Mute This Topic: https://lists.yoctoproject.org/mt/102298518/21656
Group Owner: yocto+ow...@lists.yoctoproject.org
Unsubscribe:
https://lists.yoctoproject.org/g/yocto/leave/6691583/21656/737036229/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
