From 58f4254fba04abbd0876bb3d6318d43ca1f5c3f1 Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Tue, 10 Mar 2020 12:33:18 -0700 Subject: [PATCH] fix(dev-infra): use @angular/dev-infra-private package for pullapprove verification (#35996) Adds devDependency on @angular/dev-infra-private and removes the verify script from tools, relying instead on the script from ng-dev. PR Close #35996 --- .circleci/config.yml | 2 +- .pullapprove.yml | 1 - package.json | 1 + tools/pullapprove/verify.js | 206 ------------------------------------ yarn.lock | 4 + 5 files changed, 6 insertions(+), 208 deletions(-) delete mode 100644 tools/pullapprove/verify.js diff --git a/.circleci/config.yml b/.circleci/config.yml index 3a31a29a46..2a73417bdc 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -280,7 +280,7 @@ jobs: - run: yarn lint - run: yarn ts-circular-deps:check - - run: node tools/pullapprove/verify.js + - run: yarn -s ng-dev pullapprove:verify test: executor: diff --git a/.pullapprove.yml b/.pullapprove.yml index 480e472bab..1f63de6385 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -958,7 +958,6 @@ groups: 'tools/ngcontainer/**', 'tools/npm/**', 'tools/npm_integration_test/**', - 'tools/pullapprove/**', 'tools/rxjs/**', 'tools/saucelabs/**', 'tools/size-tracking/**', diff --git a/package.json b/package.json index 0a20382a83..5a831ab9b6 100644 --- a/package.json +++ b/package.json @@ -147,6 +147,7 @@ "// 2": "devDependencies are not used under Bazel. Many can be removed after test.sh is deleted.", "devDependencies": { "@angular/cli": "9.0.3", + "@angular/dev-infra-private": "angular/dev-infra-private-builds#3724a71", "@bazel/bazelisk": "^1.3.0", "@bazel/buildifier": "^0.29.0", "@bazel/ibazel": "^0.12.3", diff --git a/tools/pullapprove/verify.js b/tools/pullapprove/verify.js deleted file mode 100644 index 35b3c54010..0000000000 --- a/tools/pullapprove/verify.js +++ /dev/null @@ -1,206 +0,0 @@ -#!/usr/bin/env node -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ -/* tslint:disable:no-console */ -const parseYaml = require('yaml').parse; -const readFileSync = require('fs').readFileSync; -const Minimatch = require('minimatch').Minimatch; -const {exec, set, cd} = require('shelljs'); -const path = require('path'); - -// Exit early on shelljs errors -set('-e'); - -// Regex Matcher for contains_any_globs conditions -const CONTAINS_ANY_GLOBS_REGEX = /^'([^']+)',?$/; - -// Full path of the angular project directory -const ANGULAR_PROJECT_DIR = path.resolve(__dirname, '../..'); -// Change to the Angular project directory -cd(ANGULAR_PROJECT_DIR); - -// Whether to log verbosely -const VERBOSE_MODE = process.argv.includes('-v'); -// Full path to PullApprove config file -const PULL_APPROVE_YAML_PATH = path.resolve(ANGULAR_PROJECT_DIR, '.pullapprove.yml'); -// All relative path file names in the git repo, this is retrieved using git rather -// that a glob so that we only get files that are checked in, ignoring things like -// node_modules, .bazelrc.user, etc -const ALL_FILES = exec('git ls-tree --full-tree -r --name-only HEAD', {silent: true}) - .trim() - .split('\n') - .filter(_ => _); -if (!ALL_FILES.length) { - console.error( - `No files were found to be in the git tree, did you run this command from \n` + - `inside the angular repository?`); - process.exit(1); -} - -/** Gets the glob matching information from each group's condition. */ -function getGlobMatchersFromCondition(groupName, condition) { - const trimmedCondition = condition.trim(); - const globMatchers = []; - const badConditionLines = []; - - // If the condition starts with contains_any_globs, evaluate all of the globs - if (trimmedCondition.startsWith('contains_any_globs')) { - trimmedCondition.split('\n') - .slice(1, -1) - .map(glob => { - const trimmedGlob = glob.trim(); - const match = trimmedGlob.match(CONTAINS_ANY_GLOBS_REGEX); - if (!match) { - badConditionLines.push(trimmedGlob); - return; - } - return match[1]; - }) - .filter(globString => !!globString) - .forEach(globString => globMatchers.push({ - group: groupName, - glob: globString, - matcher: new Minimatch(globString, {dot: true}), - matchCount: 0, - })); - } - return [globMatchers, badConditionLines]; -} - -/** Create logs for each review group. */ -function logGroups(groups) { - Array.from(groups.entries()).sort().forEach(([groupName, globs]) => { - console.groupCollapsed(groupName); - Array.from(globs.values()) - .sort((a, b) => b.matchCount - a.matchCount) - .forEach(glob => console.log(`${glob.glob} - ${glob.matchCount}`)); - console.groupEnd(); - }); -} - -/** Logs a header within a text drawn box. */ -function logHeader(...params) { - const totalWidth = 80; - const fillWidth = totalWidth - 2; - const headerText = params.join(' ').substr(0, fillWidth); - const leftSpace = Math.ceil((fillWidth - headerText.length) / 2); - const rightSpace = fillWidth - leftSpace - headerText.length; - const fill = (count, content) => content.repeat(count); - - console.log(`┌${fill(fillWidth, '─')}┐`); - console.log(`│${fill(leftSpace, ' ')}${headerText}${fill(rightSpace, ' ')}│`); - console.log(`└${fill(fillWidth, '─')}┘`); -} - -/** Runs the pull approve verification check on provided files. */ -function runVerification(files) { - // All of the globs created for each group's conditions. - const allGlobs = []; - // The pull approve config file. - const pullApprove = readFileSync(PULL_APPROVE_YAML_PATH, {encoding: 'utf8'}); - // All of the PullApprove groups, parsed from the PullApprove yaml file. - const parsedPullApproveGroups = parseYaml(pullApprove).groups; - // All files which were found to match a condition in PullApprove. - const matchedFiles = new Set(); - // All files which were not found to match a condition in PullApprove. - const unmatchedFiles = new Set(); - // All PullApprove groups which matched at least one file. - const matchedGroups = new Map(); - // All PullApprove groups which did not match at least one file. - const unmatchedGroups = new Map(); - // All condition lines which were not able to be correctly parsed, by group. - const badConditionLinesByGroup = new Map(); - // Total number of condition lines which were not able to be correctly parsed. - let badConditionLineCount = 0; - - // Get all of the globs from the PullApprove group conditions. - Object.entries(parsedPullApproveGroups).map(([groupName, group]) => { - for (const condition of group.conditions) { - const [matchers, badConditions] = getGlobMatchersFromCondition(groupName, condition); - if (badConditions.length) { - badConditionLinesByGroup.set(groupName, badConditions); - badConditionLineCount += badConditions.length; - } - allGlobs.push(...matchers); - } - }); - - if (badConditionLineCount) { - console.log(`Discovered ${badConditionLineCount} parsing errors in PullApprove conditions`); - console.log(`Attempted parsing using: ${CONTAINS_ANY_GLOBS_REGEX}`); - console.log(); - console.log(`Unable to properly parse the following line(s) by group:`); - for (const [groupName, badConditionLines] of badConditionLinesByGroup.entries()) { - console.log(`- ${groupName}:`); - badConditionLines.forEach(line => console.log(` ${line}`)); - } - console.log(); - console.log( - `Please correct the invalid conditions, before PullApprove verification can be completed`); - process.exit(1); - } - - // Check each file for if it is matched by a PullApprove condition. - for (let file of files) { - const matched = allGlobs.filter(glob => glob.matcher.match(file)); - matched.length ? matchedFiles.add(file) : unmatchedFiles.add(file); - matched.forEach(glob => glob.matchCount++); - } - - // Add each glob for each group to a map either matched or unmatched. - allGlobs.forEach(glob => { - const groups = glob.matchCount ? matchedGroups : unmatchedGroups; - const globs = groups.get(glob.group) || new Map(); - // Set the globs map in the groups map - groups.set(glob.group, globs); - // Set the glob in the globs map - globs.set(glob.glob, glob); - }); - - // PullApprove is considered verified if no files or groups are found to be unsed. - const verificationSucceeded = !(unmatchedFiles.size || unmatchedGroups.size); - - /** - * Overall result - */ - logHeader('Result'); - if (verificationSucceeded) { - console.log('PullApprove verification succeeded!'); - } else { - console.log(`PullApprove verification failed.\n`); - console.log(`Please update '.pullapprove.yml' to ensure that all necessary`); - console.log(`files/directories have owners and all patterns that appear in`); - console.log(`the file correspond to actual files/directories in the repo.`); - } - /** - * File by file Summary - */ - logHeader('PullApprove file match results'); - console.groupCollapsed(`Matched Files (${matchedFiles.size} files)`); - VERBOSE_MODE && matchedFiles.forEach(file => console.log(file)); - console.groupEnd(); - console.groupCollapsed(`Unmatched Files (${unmatchedFiles.size} files)`); - unmatchedFiles.forEach(file => console.log(file)); - console.groupEnd(); - - /** - * Group by group Summary - */ - logHeader('PullApprove group matches'); - console.groupCollapsed(`Matched Groups (${matchedGroups.size} groups)`); - VERBOSE_MODE && logGroups(matchedGroups); - console.groupEnd(); - console.groupCollapsed(`Unmatched Groups (${unmatchedGroups.size} groups)`); - logGroups(unmatchedGroups); - console.groupEnd(); - - // Provide correct exit code based on verification success. - process.exit(verificationSucceeded ? 0 : 1); -} - -runVerification(ALL_FILES); diff --git a/yarn.lock b/yarn.lock index 832618fa93..19c11e9b38 100644 --- a/yarn.lock +++ b/yarn.lock @@ -153,6 +153,10 @@ universal-analytics "^0.4.20" uuid "^3.3.2" +"@angular/dev-infra-private@angular/dev-infra-private-builds#3724a71": + version "0.0.0" + resolved "https://codeload.github.com/angular/dev-infra-private-builds/tar.gz/3724a71047361d85f4131d990f00a5aecdbc3ddc" + "@babel/code-frame@^7.0.0": version "7.0.0" resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.0.0.tgz#06e2ab19bdb535385559aabb5ba59729482800f8"