scripts/set_assignees.py: set assignee on manifest changes

Parse manifest for changes and set assignees for any manifest entries
that have changed.

Other changes:
- Do not assign to meta area when additional areas are being changed
- Cleanup of unused code
- Comment where comments are needed.

Signed-off-by: Anas Nashif <anas.nashif@intel.com>

Author: nashif

.github/workflows/assigner.yml

@@ -24,23 +24,32 @@ jobs:
     if: github.event.pull_request.draft == false
     runs-on: ubuntu-24.04
     permissions:
-      pull-requests: write # to add assignees to pull requests
       issues: write        # to add assignees to issues
 
     steps:
     - name: Check out source code
       uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
+      with:
+        fetch-depth: 0
+        persist-credentials: false
 
     - name: Set up Python
-      uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0
+      uses: zephyrproject-rtos/action-python-env@ace91a63fd503cd618ff1eb83fbcf302dabd7d44 # main
       with:
         python-version: 3.12
-        cache: pip
-        cache-dependency-path: scripts/requirements-actions.txt
 
-    - name: Install Python packages
+    - name: Fetch west.yml from pull request
+      run: |
+        git fetch origin pull/${{ github.event.pull_request.number }}/head
+        git show FETCH_HEAD:west.yml > pr_west.yml
+
+    - name: west setup
+      if: >
+        github.event_name == 'pull_request_target'
       run: |
-        pip install -r scripts/requirements-actions.txt --require-hashes
+        git config --global user.email "you@example.com"
+        git config --global user.name "Your Name"
+        west init -l . || true
 
     - name: Run assignment script
       env:
@@ -51,14 +60,13 @@ jobs:
         FLAGS+=" -r ${{ github.event.repository.name }}"
         FLAGS+=" -M MAINTAINERS.yml"
         if [ "${{ github.event_name }}" = "pull_request_target" ]; then
-            FLAGS+=" -P ${{ github.event.pull_request.number }}"
+          FLAGS+=" -P ${{ github.event.pull_request.number }} --updated-manifest pr_west.yml"
         elif [ "${{ github.event_name }}" = "issues" ]; then
-            FLAGS+=" -I ${{ github.event.issue.number }}"
+          FLAGS+=" -I ${{ github.event.issue.number }}"
         elif [ "${{ github.event_name }}" = "schedule" ]; then
-            FLAGS+=" --modules"
+          FLAGS+=" --modules"
         else
           echo "Unknown event: ${{ github.event_name }}"
           exit 1
         fi
-
         python3 scripts/set_assignees.py $FLAGS

scripts/set_assignees.py

@@ -4,20 +4,22 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import argparse
-import sys
+import datetime
 import os
+import sys
 import time
-import datetime
+from collections import defaultdict
+
 from github import Github, GithubException
 from github.GithubException import UnknownObjectException
-from collections import defaultdict
-from west.manifest import Manifest
-from west.manifest import ManifestProject
+from west.manifest import Manifest, ManifestProject
 
 TOP_DIR = os.path.join(os.path.dirname(__file__))
 sys.path.insert(0, os.path.join(TOP_DIR, "scripts"))
 from get_maintainer import Maintainers
 
+zephyr_base = os.getenv('ZEPHYR_BASE', os.path.join(TOP_DIR, '..'))
+
 def log(s):
     if args.verbose > 0:
         print(s, file=sys.stdout)
@@ -50,11 +52,45 @@ def parse_args():
     parser.add_argument("-r", "--repo", default="zephyr",
                         help="Github repository")
 
+    parser.add_argument( "--updated-manifest", default=None,
+                        help="Updated manifest file to compare against current west.yml")
+
     parser.add_argument("-v", "--verbose", action="count", default=0,
                         help="Verbose Output")
 
     args = parser.parse_args()
 
+
+def process_manifest(old_manifest_file):
+    log("Processing manifest changes")
+    if not os.path.isfile("west.yml") or not os.path.isfile(old_manifest_file):
+        log("No west.yml found, skipping...")
+        return []
+    old_manifest = Manifest.from_file(old_manifest_file)
+    new_manifest = Manifest.from_file("west.yml")
+    old_projs = set((p.name, p.revision) for p in old_manifest.projects)
+    new_projs = set((p.name, p.revision) for p in new_manifest.projects)
+    # Removed projects
+    rprojs = set(filter(lambda p: p[0] not in list(p[0] for p in new_projs),
+        old_projs - new_projs))
+    # Updated projects
+    uprojs = set(filter(lambda p: p[0] in list(p[0] for p in old_projs),
+        new_projs - old_projs))
+    # Added projects
+    aprojs = new_projs - old_projs - uprojs
+
+    # All projs
+    projs = rprojs | uprojs | aprojs
+    projs_names = [name for name, rev in projs]
+
+    log(f"found modified projects: {projs_names}")
+    areas = []
+    for p in projs_names:
+        areas.append(f'West project: {p}')
+
+    log(f'manifest areas: {areas}')
+    return areas
+
 def process_pr(gh, maintainer_file, number):
 
     gh_repo = gh.get_repo(f"{args.org}/{args.repo}")
@@ -67,35 +103,59 @@ def process_pr(gh, maintainer_file, number):
     found_maintainers = defaultdict(int)
 
     num_files = 0
-    all_areas = set()
     fn = list(pr.get_files())
 
-    for changed_file in fn:
-        if changed_file.filename in ['west.yml','submanifests/optional.yaml']:
-            break
-
     if pr.commits == 1 and (pr.additions <= 1 and pr.deletions <= 1):
         labels = {'size: XS'}
 
     if len(fn) > 500:
         log(f"Too many files changed ({len(fn)}), skipping....")
         return
 
+    # areas where assignment happens if only area is affected
+    meta_areas = [
+            'Release Notes',
+            'Documentation',
+            'Samples'
+            ]
+
     for changed_file in fn:
+
         num_files += 1
         log(f"file: {changed_file.filename}")
-        areas = maintainer_file.path2areas(changed_file.filename)
+
+        areas = []
+        if changed_file.filename in ['west.yml','submanifests/optional.yaml']:
+            if not args.updated_manifest:
+                log("No updated manifest file provided, cannot process west.yml changes, skipping...")
+                continue
+            parsed_areas = process_manifest(old_manifest_file=args.updated_manifest)
+            for _area in parsed_areas:
+                area_match = maintainer_file.name2areas(_area)
+                if area_match:
+                    areas.extend(area_match)
+        else:
+            areas = maintainer_file.path2areas(changed_file.filename)
+
+        log(f"areas for {changed_file}: {areas}")
 
         if not areas:
             continue
 
-        all_areas.update(areas)
+        # instance of an area, for example a driver or a board, not APIs or subsys code.
         is_instance = False
         sorted_areas = sorted(areas, key=lambda x: 'Platform' in x.name, reverse=True)
         for area in sorted_areas:
-            c = 1 if not is_instance else 0
+            # do not count cmake file changes, i.e. when there are changes to
+            # instances of an area listed in both the subsystem and the
+            # platform implementing it
+            if 'CMakeLists.txt' in changed_file.filename or area.name in meta_areas:
+                c = 0
+            else:
+                c = 1 if not is_instance else 0
 
             area_counter[area] += c
+            log(f"area counter: {area_counter}")
             labels.update(area.labels)
             # FIXME: Here we count the same file multiple times if it exists in
             # multiple areas with same maintainer
@@ -122,22 +182,26 @@ def process_pr(gh, maintainer_file, number):
     log(f"Submitted by: {pr.user.login}")
     log(f"candidate maintainers: {_all_maintainers}")
 
-    assignees = []
-    tmp_assignees = []
+    ranked_assignees = []
+    assignees = None
 
     # we start with areas with most files changed and pick the maintainer from the first one.
     # if the first area is an implementation, i.e. driver or platform, we
     # continue searching for any other areas involved
     for area, count in area_counter.items():
-        if count == 0:
+        # if only meta area is affected, assign one of the maintainers of that area
+        if area.name in meta_areas and len(area_counter) == 1:
+            assignees = area.maintainers
+            break
+        # if no maintainers, skip
+        if count == 0 or len(area.maintainers) == 0:
             continue
+        # if there are maintainers, but no assignees yet, set them
         if len(area.maintainers) > 0:
-            tmp_assignees = area.maintainers
             if pr.user.login in area.maintainers:
-                # submitter = assignee, try to pick next area and
-                # assign someone else other than the submitter
-                # when there also other maintainers for the area
-                # assign them
+                # If submitter = assignee, try to pick next area and assign
+                # someone else other than the submitter, otherwise when there
+                # are other maintainers for the area, assign them.
                 if len(area.maintainers) > 1:
                     assignees = area.maintainers.copy()
                     assignees.remove(pr.user.login)
@@ -146,16 +210,25 @@ def process_pr(gh, maintainer_file, number):
             else:
                 assignees = area.maintainers
 
-            if 'Platform' not in area.name:
-                break
+        # found a non-platform area that was changed, pick assignee from this
+        # area and put them on top of the list, otherwise just append.
+        if 'Platform' not in area.name:
+            ranked_assignees.insert(0, area.maintainers)
+            break
+        else:
+            ranked_assignees.append(area.maintainers)
 
-    if tmp_assignees and not assignees:
-        assignees = tmp_assignees
+    if ranked_assignees:
+        assignees = ranked_assignees[0]
 
     if assignees:
         prop = (found_maintainers[assignees[0]] / num_files) * 100
         log(f"Picked assignees: {assignees} ({prop:.2f}% ownership)")
         log("+++++++++++++++++++++++++")
+    elif len(_all_maintainers) > 0:
+        # if we have maintainers found, but could not pick one based on area,
+        # then pick the one with most changes
+        assignees = [next(iter(_all_maintainers))]
 
     # Set labels
     if labels:
@@ -165,7 +238,7 @@ def process_pr(gh, maintainer_file, number):
                 if not args.dry_run:
                     pr.add_to_labels(l)
         else:
-            log(f"Too many labels to be applied")
+            log("Too many labels to be applied")
 
     if collab:
         reviewers = []
@@ -206,27 +279,30 @@ def process_pr(gh, maintainer_file, number):
         if len(existing_reviewers) < 15:
             reviewer_vacancy = 15 - len(existing_reviewers)
             reviewers = reviewers[:reviewer_vacancy]
-
-            if reviewers:
-                try:
-                    log(f"adding reviewers {reviewers}...")
-                    if not args.dry_run:
-                        pr.create_review_request(reviewers=reviewers)
-                except GithubException:
-                    log("cant add reviewer")
         else:
             log("not adding reviewers because the existing reviewer count is greater than or "
-                "equal to 15")
+                "equal to 15. Adding maintainers of all areas as reviewers instead.")
+            # FIXME: Here we could also add collaborators of the areas most
+            # affected, i.e. the one with the final assigne.
+            reviewers = list(_all_maintainers.keys())
+
+        if reviewers:
+            try:
+                log(f"adding reviewers {reviewers}...")
+                if not args.dry_run:
+                    pr.create_review_request(reviewers=reviewers)
+            except GithubException:
+                log("can't add reviewer")
 
     ms = []
     # assignees
-    if assignees and not pr.assignee:
+    if assignees and (not pr.assignee or args.dry_run):
         try:
             for assignee in assignees:
                 u = gh.get_user(assignee)
                 ms.append(u)
         except GithubException:
-            log(f"Error: Unknown user")
+            log("Error: Unknown user")
 
         for mm in ms:
             log(f"Adding assignee {mm}...")
@@ -280,7 +356,7 @@ def process_issue(gh, maintainer_file, number):
     print(f"Using labels: {issue_labels}")
 
     if issue_labels not in label_to_maintainer:
-        print(f"no match for the label set, not assigning")
+        print("no match for the label set, not assigning")
         return
 
     for maintainer in label_to_maintainer[issue_labels]:
@@ -317,7 +393,7 @@ def process_modules(gh, maintainers_file):
         repo_name = f"{args.org}/{project.name}"
         repos[repo_name] = maintainers_file.areas[area]
 
-    query = f"is:open is:pr no:assignee"
+    query = "is:open is:pr no:assignee"
     for repo in repos:
         query += f" repo:{repo}"