diff --git a/action.py b/action.py index 3724393..f55ad6d 100644 --- a/action.py +++ b/action.py @@ -17,7 +17,8 @@ # 3rd party imports go here import requests from github import Github, GithubException -from west.manifest import Manifest, MalformedManifest, ImportFlag +from west.manifest import Manifest, MalformedManifest, ImportFlag, \ + MANIFEST_PROJECT_INDEX NOTE = "\n\n*Note: This message is automatically posted and updated by the " \ "Manifest GitHub Action.* " @@ -34,14 +35,12 @@ def die(s): print(f'ERROR: {s}', file=sys.stderr) sys.exit(1) - -def gh_tuple_split(s): +def gh_pr_split(s): sl = s.split('/') - if len(sl) != 2: - raise RuntimeError("Invalid org or dst format") - - return sl[0], sl[1] + if len(sl) != 3: + raise RuntimeError(f"Invalid pr format: {s}") + return sl[0], sl[1], sl[2] def cmd2str(cmd): # Formats the command-line arguments in the iterable 'cmd' into a string, @@ -172,7 +171,7 @@ def fmt_rev(repo, rev): try: if maybe_sha(rev): - branches = [b.name for b in repo.get_branches() if rev == + branches = [f'`{b.name}`' for b in repo.get_branches() if rev == b.commit.sha] s = repo.get_commit(rev).html_url # commits get formatted nicely by GitHub itself @@ -221,7 +220,7 @@ def _get_manifests_from_gh(token, gh_repo, mpath, new_mfile, base_sha): try: old_mfile = gh_repo.get_contents(mpath, base_sha) except GithubException: - print('Base revision does not contain a valid manifest') + log('Base revision does not contain a valid manifest') exit(0) old_manifest = manifest_from_url(token, old_mfile.download_url) @@ -261,6 +260,59 @@ def manifest_at_rev(sha): return (old_manifest, new_manifest) +def _get_merge_status(len_a, len_r, len_pr, len_meta, impostor_shas, unreachables): + strs = [] + def plural(count): + return 's' if count > 1 else '' + + if len_a: + strs.append(f'{len_a} added project{plural(len_a)}') + if len_r: + strs.append(f'{len_r} removed project{plural(len_r)}') + if len_pr: + strs.append(f'{len_pr} project{plural(len_pr)} with PR revision') + if len_meta: + strs.append(f'{len_meta} project{plural(len_meta)} with metadata changes') + if impostor_shas: + strs.append(f'{impostor_shas} impostor SHA{plural(impostor_shas)}') + if unreachables: + strs.append(f'{unreachables} unreachable repo{plural(unreachables)}') + + if not len(strs): + return False, '\u2705 **All manifest checks OK**' + + n = '\U000026D4 **DNM label due to: ' + for i, s in enumerate(strs): + if i == (len(strs) - 1): + _s = f'and {s}' if len(strs) > 1 else s + else: + _s = f'{s}, ' if (len(strs) - i > 2) else f'{s} ' + n += _s + n += '**' + return True, n + +def _get_sets(old_projs, new_projs): + # Symmetric difference: everything that is not in both + + # 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 + + log(f'rprojs: {rprojs}') + log(f'uprojs: {uprojs}') + log(f'aprojs: {aprojs}') + + return (projs, rprojs, uprojs, aprojs) + + def main(): parser = argparse.ArgumentParser( @@ -270,6 +322,9 @@ def main(): required=True, help='Path to the manifest file.') + parser.add_argument('--pr', default=None, required=True, + help='//') + parser.add_argument('-m', '--message', action='store', required=False, help='Message to post.') @@ -302,7 +357,7 @@ def main(): type=int, default=0, choices=range(0, 2), required=False, help='Verbosity level.') - print(sys.argv) + log(sys.argv) args = parser.parse_args() @@ -313,7 +368,6 @@ def main(): checkout = args.checkout_path if args.checkout_path != 'none' else None use_tree = args.use_tree_checkout != 'false' check_impostor = args.check_impostor_commits != 'false' - impostor_sha = False labels = [x.strip() for x in args.labels.split(',')] \ if args.labels != 'none' else None dnm_labels = [x.strip() for x in args.dnm_labels.split(',')] \ @@ -323,39 +377,25 @@ def main(): if use_tree and not checkout: sys_exit("Cannot use a tree checkout without a checkout path") - # Retrieve main env vars - action = os.environ.get('GITHUB_ACTION', None) - workflow = os.environ.get('GITHUB_WORKFLOW', None) - org_repo = os.environ.get('GITHUB_REPOSITORY', None) - - log(f'Running action {action} from workflow {workflow} in {org_repo}') - - evt_name = os.environ.get('GITHUB_EVENT_NAME', None) - evt_path = os.environ.get('GITHUB_EVENT_PATH', None) - workspace = os.environ.get('GITHUB_WORKSPACE', None) - # Abs path to checked-out tree - checkout = (Path(workspace) / Path(checkout)).resolve() if checkout else None - - log(f'Event {evt_name} in {evt_path} and workspace {workspace}') + workspace = os.environ.get('GITHUB_WORKSPACE', None) + if checkout: + checkout = ((Path(workspace) / Path(checkout)).resolve() if workspace else + Path(checkout).resolve()) + if not checkout.is_dir(): + die(f'checkout repo {checkout} does not exist; check path') + log(f'Checkout path: {checkout}') token = os.environ.get('GITHUB_TOKEN', None) if not token: sys.exit('Github token not set in environment, please set the ' 'GITHUB_TOKEN environment variable and retry.') - if not ("pull_request" in evt_name): - sys.exit(f'Invalid event {evt_name}') - - with open(evt_path, 'r') as f: - evt = json.load(f) - - pr = evt['pull_request'] - gh = Github(token) - gh_repo = gh.get_repo(org_repo) - gh_pr = gh_repo.get_pull(int(pr['number'])) + org_str, repo_str, pr_str = gh_pr_split(args.pr) + gh_repo = gh.get_repo(f'{org_str}/{repo_str}') + gh_pr = gh_repo.get_pull(int(pr_str)) mpath = args.path new_mfile = None @@ -379,31 +419,24 @@ def main(): (old_manifest, new_manifest) = _get_manifests_from_gh(token, gh_repo, mpath, new_mfile, base_sha) + # Ensure we only remove the manifest project + assert(MANIFEST_PROJECT_INDEX == 0) + omp = old_manifest.projects[MANIFEST_PROJECT_INDEX] + nmp = new_manifest.projects[MANIFEST_PROJECT_INDEX] + ops = old_manifest.projects[MANIFEST_PROJECT_INDEX + 1:] + nps = new_manifest.projects[MANIFEST_PROJECT_INDEX + 1:] + + old_projs = set((p.name, p.revision) for p in ops) + new_projs = set((p.name, p.revision) for p in nps) - 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) log(f'old_projs: {old_projs}') log(f'new_projs: {new_projs}') - # Symmetric difference: everything that is not in both + log('Revision sets') + (projs, rprojs, uprojs, aprojs) = _get_sets(old_projs, new_projs) - # 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'rprojs: {rprojs}') - log(f'uprojs: {uprojs}') - log(f'aprojs: {aprojs}') - if not len(projs): log('No projects updated') @@ -415,11 +448,14 @@ def main(): log(f'projs_names: {str(projs_names)}') + impostor_shas = 0 + unreachables = 0 + files = dict() # Link main PR to project PRs strs = list() if message: strs.append(message) - strs.append('The following west manifest projects have been modified in this Pull ' + strs.append('The following west manifest projects have changed revision in this Pull ' 'Request:\n') strs.append('| Name | Old Revision | New Revision | Diff |') strs.append('| ---- | ------------ | ------------ |------|') @@ -430,39 +466,110 @@ def main(): old_rev = None if p in aprojs else next( filter(lambda _p: _p[0] == p[0], old_projs))[1] new_rev = None if p in rprojs else p[1] + or_note = ' (Added)' if not old_rev else '' + nr_note = ' (Removed)' if not new_rev else '' + name_note = ' \U0001F195' if not old_rev else ' \U0000274c ' if \ + not new_rev else '' url = manifest.get_projects([p[0]])[0].url re_url = re.compile(r'https://github\.com/' '([A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)/?') try: repo = gh.get_repo(re_url.match(url)[1]) except (GithubException, TypeError) as error: - print(error) - print(f"Can't get repo for {p[0]}; output will be limited") - strs.append(f'| {p[0]} | {old_rev} | {new_rev} | N/A |') + log(error) + log(f"Can't get repo for {p[0]}; output will be limited") + strs.append(f'| {p[0]}{name_note} | {old_rev}{or_note} | {new_rev}{nr_note} | N/A |') + unreachables += 1 continue - line = f'| {p[0]} | {fmt_rev(repo, old_rev)} ' + line = f'| {p[0]}{name_note} | {fmt_rev(repo, old_rev)}{or_note} ' if p in pr_projs: pr = repo.get_pull(int(re_rev.match(new_rev)[1])) - line += f'| {pr.html_url} ' + line += f'| {pr.html_url}{nr_note} ' line += f'| [{repo.full_name}#{pr.number}/files]' + \ f'({pr.html_url}/files) |' + # Store files changed + files[p[0]] = [f for f in pr.get_files()] else: - if check_impostor and is_impostor(repo, new_rev): - impostor_sha = True - line += f'|\u274c Impostor SHA: {fmt_rev(repo, new_rev)} ' + if check_impostor and new_rev and is_impostor(repo, new_rev): + impostor_shas += 1 + line += f'|\u274c Impostor SHA: {fmt_rev(repo, new_rev)}{nr_note} ' else: - line += f'| {fmt_rev(repo, new_rev)} ' + line += f'| {fmt_rev(repo, new_rev)}{nr_note}' if p in uprojs: line += f'| [{repo.full_name}@{shorten_rev(old_rev)}..' + \ f'{shorten_rev(new_rev)}]' + \ f'({repo.html_url}/compare/{old_rev}..{new_rev}) |' + # Store files changed + try: + c = repo.compare(old_rev, new_rev) + files[p[0]] = [f for f in c.files] + except GithubException as e: + log(e) + log(f"Can't get files changed for {p[0]}") else: line += '| N/A |' strs.append(line) - message = '\n'.join(strs) + NOTE + def _hashable(o): + if isinstance(o, list): + return frozenset(o) + return o + + def _module_changed(p): + if p.name in files: + for f in files[p.name]: + if ('zephyr/module.yml' in f.filename or + 'zephyr/module.yaml' in f.filename): + return True + return False + + # Check additional metadata + meta_op = set((p.name, p.url, _hashable(p.submodules), + _hashable(p.west_commands), False) for p in ops) + meta_np = set((p.name, p.url, _hashable(p.submodules), + _hashable(p.west_commands), _module_changed(p)) for p in nps) + + log('Metadata sets') + (_, meta_rprojs, meta_uprojs, meta_aprojs) = _get_sets(meta_op, meta_np) + meta_projs = meta_uprojs | meta_aprojs + + def _cmp_old_new(p, index, force_change=False): + old = None if p in meta_aprojs else next(filter(lambda _p: _p[0] == p[0], meta_op))[index] + new = next(filter(lambda _p: _p[0] == p[0], meta_np))[index] + log(f'name: {p[0]} index: {index} old: {old} new: {new}') + # Special handling for an added project + if old is None and not new: + return '' + # Select which symbol to show + if not old and new: + return '\U0001F195' if not force_change else '\u270f' # added + elif not new and old: + return '\u274c' # removed + elif new != old: + return '\u270f' # modified + else: + return '' + + if len(meta_uprojs): + strs.append('\n\nAdditional metadata changed:\n') + strs.append('| Name | URL | Submodules | West cmds | `module.yml` | ') + strs.append('| ---- | --- | ---------- | --------- | ------------ | ') + for p in sorted(meta_uprojs, key=lambda _p: _p[0]): + url = _cmp_old_new(p, 1) + subms = _cmp_old_new(p, 2) + wcmds = _cmp_old_new(p, 3) + mys = _cmp_old_new(p, 4, True) + line = f'| {p[0]} | {url} | {subms} | {wcmds} | {mys} |' + strs.append(line) + + # Add a note about the merge status of the manifest PR + dnm, status_note = _get_merge_status(len(aprojs), len(rprojs), len(pr_projs), + len(meta_uprojs), impostor_shas, unreachables) + status_note = f'\n\n{status_note}' + + message = '\n'.join(strs) + status_note + NOTE comment = None for c in gh_pr.get_issue_comments(): if NOTE in c.body: @@ -471,12 +578,12 @@ def main(): if not comment: if len(projs): - print('Creating comment') + log('Creating comment') comment = gh_pr.create_issue_comment(message) else: - print('Skipping comment creation, no manifest changes') + log('Skipping comment creation, no manifest changes') else: - print('Updating comment') + log('Updating comment') comment.edit(message) if not comment: @@ -503,7 +610,7 @@ def get_relevant_labels(label_list): log(f'removing label {l}') gh_pr.remove_from_labels(l) except GithubException: - print(f'Unable to remove label {l}') + log(f'Unable to remove label {l}') if label_prefix: for p in projs: @@ -516,17 +623,17 @@ def get_relevant_labels(label_list): log(f'removing label {l}') gh_pr.remove_from_labels(l) except GithubException: - print(f'Unable to remove prefixed label {l}') + log(f'Unable to remove prefixed label {l}') if dnm_labels: - if not len(aprojs) and not len(pr_projs) and not impostor_sha: + if not dnm: # Remove the DNM labels try: for l in dnm_labels: log(f'removing label {l}') gh_pr.remove_from_labels(l) except GithubException: - print('Unable to remove DNM label') + log('Unable to remove DNM label') else: # Add the DNM labels for l in dnm_labels: diff --git a/action.yml b/action.yml index d76bcca..adb5ad3 100644 --- a/action.yml +++ b/action.yml @@ -7,10 +7,11 @@ inputs: required: false default: '0' manifest-path: - description: 'The path to the manifest file' + description: 'The relative path to the manifest file' required: true checkout-path: - description: 'The path to the checked out Pull Request' + description: 'The path to the checked out PR. Relative or absolute depending on + whether the GITHUB_WORKSPACE env var is defined' default: 'none' required: false use-tree-checkout: @@ -56,6 +57,7 @@ runs: - id: run-python run: | python3 ${{ github.action_path }}/action.py -p "${{ inputs.manifest-path }}" \ + --pr "${{ github.repository }}/${{ github.event.pull_request.number }}" \ --checkout-path "${{ inputs.checkout-path}}" -m "${{ inputs.message }}" \ -l "${{ inputs.labels }}" --label-prefix "${{ inputs.label-prefix }}" \ --dnm-labels "${{ inputs.dnm-labels }}" -v "${{ inputs.verbosity-level }}" \