Use simple-git to produce diffs in the code analyzer (#33837)

In code-analyzer, clone the repo locally in a tmp folder and perform git operations on the copy instead of local files.
This commit is contained in:
Sam Seay 2022-07-14 15:39:38 +12:00 committed by GitHub
parent cca2df58b4
commit 5157bcf934
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 195 additions and 38 deletions

View File

@ -17,7 +17,7 @@ jobs:
pnpm build:feature-config --filter=woocommerce pnpm build:feature-config --filter=woocommerce
- name: Run analyzer - name: Run analyzer
id: run id: run
run: ./tools/code-analyzer/bin/dev analyzer "$GITHUB_HEAD_REF" -o github run: ./tools/code-analyzer/bin/dev analyzer "$GITHUB_HEAD_REF"
- name: Print results - name: Print results
id: results id: results
run: echo "::set-output name=results::${{ steps.run.outputs.templates }}${{ steps.run.outputs.wphooks }}${{ steps.run.outputs.schema }}${{ steps.run.outputs.database }}" run: echo "::set-output name=results::${{ steps.run.outputs.templates }}${{ steps.run.outputs.wphooks }}${{ steps.run.outputs.schema }}${{ steps.run.outputs.database }}"

View File

@ -1743,17 +1743,23 @@ importers:
'@oclif/plugin-help': ^5 '@oclif/plugin-help': ^5
'@oclif/plugin-plugins': ^2.0.1 '@oclif/plugin-plugins': ^2.0.1
'@types/node': ^16.9.4 '@types/node': ^16.9.4
'@types/uuid': ^8.3.4
eslint: ^7.32.0 eslint: ^7.32.0
globby: ^11 globby: ^11
oclif: ^2 oclif: ^2
shx: ^0.3.3 shx: ^0.3.3
simple-git: ^3.10.0
ts-node: ^10.2.1 ts-node: ^10.2.1
tslib: ^2.3.1 tslib: ^2.3.1
typescript: ^4.4.3 typescript: ^4.4.3
uuid: ^8.3.2
dependencies: dependencies:
'@oclif/core': 1.3.4 '@oclif/core': 1.3.4
'@oclif/plugin-help': 5.1.11 '@oclif/plugin-help': 5.1.11
'@oclif/plugin-plugins': 2.1.0 '@oclif/plugin-plugins': 2.1.0
'@types/uuid': 8.3.4
simple-git: 3.10.0
uuid: 8.3.2
devDependencies: devDependencies:
'@types/node': 16.10.3 '@types/node': 16.10.3
eslint: 7.32.0 eslint: 7.32.0
@ -8932,11 +8938,9 @@ packages:
debug: 4.3.4 debug: 4.3.4
transitivePeerDependencies: transitivePeerDependencies:
- supports-color - supports-color
dev: true
/@kwsites/promise-deferred/1.1.1: /@kwsites/promise-deferred/1.1.1:
resolution: {integrity: sha512-GaHYm+c0O9MjZRu0ongGBRbinu8gVAMd2UZjji6jVmqKtZluZnptXGWhz1E8j8D2HJ3f/yMxKAUC0b+57wncIw==} resolution: {integrity: sha512-GaHYm+c0O9MjZRu0ongGBRbinu8gVAMd2UZjji6jVmqKtZluZnptXGWhz1E8j8D2HJ3f/yMxKAUC0b+57wncIw==}
dev: true
/@mdx-js/loader/1.6.22: /@mdx-js/loader/1.6.22:
resolution: {integrity: sha512-9CjGwy595NaxAYp0hF9B/A0lH6C8Rms97e2JS9d3jVUtILn6pT5i5IV965ra3lIWc7Rs1GG1tBdVF7dCowYe6Q==} resolution: {integrity: sha512-9CjGwy595NaxAYp0hF9B/A0lH6C8Rms97e2JS9d3jVUtILn6pT5i5IV965ra3lIWc7Rs1GG1tBdVF7dCowYe6Q==}
@ -12739,6 +12743,10 @@ packages:
resolution: {integrity: sha512-PBjIUxZHOuj0R15/xuwJYjFi+KZdNFrehocChv4g5hu6aFroHue8m0lBP0POdK2nKzbw0cgV1mws8+V/JAcEkQ==} resolution: {integrity: sha512-PBjIUxZHOuj0R15/xuwJYjFi+KZdNFrehocChv4g5hu6aFroHue8m0lBP0POdK2nKzbw0cgV1mws8+V/JAcEkQ==}
dev: true dev: true
/@types/uuid/8.3.4:
resolution: {integrity: sha512-c/I8ZRb51j+pYGAu5CrFMRxqZ2ke4y2grEBO5AUjgSkSk+qT2Ea+OdWElz/OiMf5MNpn2b17kuVBwZLQJXzihw==}
dev: false
/@types/vinyl/2.0.6: /@types/vinyl/2.0.6:
resolution: {integrity: sha512-ayJ0iOCDNHnKpKTgBG6Q6JOnHTj9zFta+3j2b8Ejza0e4cvRyMn0ZoLEmbPrTHe5YYRlDYPvPWVdV4cTaRyH7g==} resolution: {integrity: sha512-ayJ0iOCDNHnKpKTgBG6Q6JOnHTj9zFta+3j2b8Ejza0e4cvRyMn0ZoLEmbPrTHe5YYRlDYPvPWVdV4cTaRyH7g==}
dependencies: dependencies:
@ -35359,6 +35367,16 @@ packages:
/signal-exit/3.0.7: /signal-exit/3.0.7:
resolution: {integrity: sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==} resolution: {integrity: sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==}
/simple-git/3.10.0:
resolution: {integrity: sha512-2w35xrS5rVtAW0g67LqtxCZN5cdddz/woQRfS0OJXaljXEoTychZ4jnE+CQgra/wX4ZvHeiChTUMenCwfIYEYw==}
dependencies:
'@kwsites/file-exists': 1.1.1
'@kwsites/promise-deferred': 1.1.1
debug: 4.3.4
transitivePeerDependencies:
- supports-color
dev: false
/simple-git/3.7.1: /simple-git/3.7.1:
resolution: {integrity: sha512-+Osjtsumbtew2y9to0pOYjNzSIr4NkKGBg7Po5SUtjQhaJf2QBmiTX/9E9cv9rmc7oUiSGFIB9e7ys5ibnT9+A==} resolution: {integrity: sha512-+Osjtsumbtew2y9to0pOYjNzSIr4NkKGBg7Po5SUtjQhaJf2QBmiTX/9E9cv9rmc7oUiSGFIB9e7ys5ibnT9+A==}
dependencies: dependencies:

View File

@ -19,7 +19,10 @@
"dependencies": { "dependencies": {
"@oclif/core": "^1", "@oclif/core": "^1",
"@oclif/plugin-help": "^5", "@oclif/plugin-help": "^5",
"@oclif/plugin-plugins": "^2.0.1" "@oclif/plugin-plugins": "^2.0.1",
"@types/uuid": "^8.3.4",
"simple-git": "^3.10.0",
"uuid": "^8.3.2"
}, },
"devDependencies": { "devDependencies": {
"@types/node": "^16.9.4", "@types/node": "^16.9.4",

View File

@ -24,7 +24,7 @@ import {
getHookDescription, getHookDescription,
getHookChangeType, getHookChangeType,
} from '../../utils'; } from '../../utils';
import { generatePatch, generateSchemaDiff } from '../../git'; import { generateDiff, generatePatch, generateSchemaDiff } from '../../git';
/** /**
* Analyzer class * Analyzer class
@ -64,8 +64,8 @@ export default class Analyzer extends Command {
} ), } ),
source: Flags.string( { source: Flags.string( {
char: 's', char: 's',
description: 'GitHub organization/repository.', description: 'Git repo url or local path to a git repo.',
default: 'woocommerce/woocommerce', default: process.cwd(),
} ), } ),
plugin: Flags.string( { plugin: Flags.string( {
char: 'p', char: 'p',
@ -73,6 +73,12 @@ export default class Analyzer extends Command {
options: [ 'core', 'admin', 'beta' ], options: [ 'core', 'admin', 'beta' ],
default: 'core', default: 'core',
} ), } ),
'is-woocommerce': Flags.boolean( {
char: 'w',
description:
'Analyzing WooCommerce? (Will scan for DB schema changes).',
default: true,
} ),
}; };
/** /**
@ -81,49 +87,28 @@ export default class Analyzer extends Command {
async run(): Promise< void > { async run(): Promise< void > {
const { args, flags } = await this.parse( Analyzer ); const { args, flags } = await this.parse( Analyzer );
this.validateArgs( flags.source ); const diff = await generateDiff(
const patchContent = generatePatch(
flags.source, flags.source,
args.compare,
flags.base, flags.base,
( e: string ): void => this.error( e ) args.compare,
this.error
); );
const pluginData = this.getPluginData( flags.plugin ); const pluginData = this.getPluginData( flags.plugin );
this.log( `${ pluginData[ 1 ] } Version: ${ pluginData[ 0 ] }` ); this.log( `${ pluginData[ 1 ] } Version: ${ pluginData[ 0 ] }` );
// Run schema diffs only in the monorepo. // Run schema diffs only in the monorepo.
if ( flags.source === 'woocommerce/woocommerce' ) { if ( flags[ 'is-woocommerce' ] ) {
const schemaDiff = await generateSchemaDiff( const schemaDiff = await generateSchemaDiff(
flags.source, 'woocommerce/woocommerce',
args.compare, args.compare,
flags.base, flags.base,
( e: string ): void => this.error( e ) ( e: string ): void => this.error( e )
); );
this.scanChanges( this.scanChanges( diff, pluginData[ 0 ], flags.output, schemaDiff );
patchContent,
pluginData[ 0 ],
flags.output,
schemaDiff
);
} else { } else {
this.scanChanges( patchContent, pluginData[ 0 ], flags.output ); this.scanChanges( diff, pluginData[ 0 ], flags.output );
}
}
/**
* Validates all of the arguments to make sure
*
* @param {string} source The GitHub repository we are merging.
*/
private validateArgs( source: string ): void {
// We only support pulling from GitHub so the format needs to match that.
if ( ! source.match( /^[a-z0-9\-]+\/[a-z0-9\-]+$/ ) ) {
this.error(
'The "source" argument must be in "organization/repository" format'
);
} }
} }

View File

@ -5,13 +5,165 @@ import { CliUx } from '@oclif/core';
import { execSync } from 'child_process'; import { execSync } from 'child_process';
import { join } from 'path'; import { join } from 'path';
import { tmpdir } from 'os'; import { tmpdir } from 'os';
import { readFileSync } from 'fs'; import { mkdirSync, readFileSync, rmSync } from 'fs';
import { simpleGit } from 'simple-git';
import { v4 } from 'uuid';
/** /**
* Internal dependencies * Internal dependencies
*/ */
import { startWPEnv, stopWPEnv, isValidCommitHash } from './utils'; import { startWPEnv, stopWPEnv, isValidCommitHash } from './utils';
/**
* Check if a string is a valid url.
*
* @param {string} maybeURL - the URL string to check
* @return {boolean} whether the string is a valid URL or not.
*/
const isUrl = ( maybeURL: string ) => {
try {
new URL( maybeURL );
return true;
} catch ( e ) {
return false;
}
};
/**
* Clone a git repository.
*
* @param {string} repoPath - the path (either URL or file path) to the repo to clone.
* @return {string} the path to the cloned repo.
*/
export const cloneRepo = async ( repoPath: string ) => {
const folderPath = join( tmpdir(), 'code-analyzer-tmp', v4() );
mkdirSync( folderPath, { recursive: true } );
const git = simpleGit( { baseDir: folderPath } );
await git.clone( repoPath, folderPath );
// If this is a local clone then the simplest way to maintain remote settings is to copy git config across
if ( ! isUrl( repoPath ) ) {
execSync( `cp ${ repoPath }/.git/config ${ folderPath }/.git/config` );
}
return folderPath;
};
/**
* Do a git diff of 2 commit hashes (or branches)
*
* @param {string} baseDir - baseDir that the repo is in
* @param {string} hashA - either a git commit hash or a git branch
* @param {string} hashB - either a git commit hash or a git branch
* @return {string} - diff of the changfiles between the 2 hashes
*/
export const diffHashes = ( baseDir: string, hashA: string, hashB: string ) => {
const git = simpleGit( { baseDir } );
return git.diff( [ `${ hashA }..${ hashB }` ] );
};
/**
* Determines if a string is a commit hash or not.
*
* @param {string} ref - the ref to check
* @return {boolean} whether the ref is a commit hash or not.
*/
const refIsHash = ( ref: string ) => {
return /^[0-9a-f]{7,40}$/i.test( ref );
};
/**
* Get the commit hash for a ref (either branch or commit hash). If a validly
* formed hash is provided it is returned unmodified.
*
* @param {string} baseDir - the dir of the git repo to get the hash from.
* @param {string} ref - Either a commit hash or a branch name.
* @return {string} - the commit hash of the ref.
*/
export const getCommitHash = async ( baseDir: string, ref: string ) => {
const isHash = refIsHash( ref );
// check if its in history, if its not an error will be thrown
try {
await simpleGit( { baseDir } ).show( ref );
} catch ( e ) {
throw new Error(
`${ ref } is not a valid commit hash or branch name that exists in git history`
);
}
// If its not a hash we assume its a branch
if ( ! isHash ) {
return simpleGit( { baseDir } ).revparse( [ ref ] );
}
// Its a hash already
return ref;
};
/**
* generateDiff generates a diff for a given repo and 2 hashes or branch names.
*
* @param {string} repoPath - the url or filepath of the repo to clone.
* @param {string} hashA - commit hash or branch name.
* @param {string} hashB - commit hash or branch name.
* @param {Function} onError - the handler to call when an error occurs.
*/
export const generateDiff = async (
repoPath: string,
hashA: string,
hashB: string,
onError: ( error: string ) => void
) => {
try {
const tmpRepoPath = await cloneRepo( repoPath );
const git = simpleGit( { baseDir: tmpRepoPath } );
await git.fetch();
const validBranches = [ hashA, hashB ].filter(
( hash ) => ! refIsHash( hash )
);
// checking out any branches will automatically track remote branches.
for ( const validBranch of validBranches ) {
// Note you can't do checkouts in parallel otherwise the git binary will crash
await git.checkout( [ validBranch ] );
}
// turn both hashes into commit hashes if they are not already.
const commitHashA = await getCommitHash( tmpRepoPath, hashA );
const commitHashB = await getCommitHash( tmpRepoPath, hashB );
const isRepo = await simpleGit( {
baseDir: tmpRepoPath,
} ).checkIsRepo();
if ( ! isRepo ) {
throw new Error( 'Not a git repository' );
}
const diff = await diffHashes( tmpRepoPath, commitHashA, commitHashB );
// time to clean up
rmSync( tmpRepoPath, { force: true, recursive: true } );
return diff;
} catch ( e ) {
if ( e instanceof Error ) {
onError(
`Unable to create diff. Check that git repo, base hash, and compare hash all exist.\n Error: ${ e.message }`
);
} else {
onError(
'Unable to create diff. Check that git repo, base hash, and compare hash all exist.'
);
}
return '';
}
};
/** /**
* Fetch branches from origin. * Fetch branches from origin.
* *
@ -203,8 +355,7 @@ export const generateSchemaDiff = async (
description: 'OrdersTableDataStore Schema', description: 'OrdersTableDataStore Schema',
base: baseSchema.OrdersTableDataStore, base: baseSchema.OrdersTableDataStore,
compare: compareSchema.OrdersTableDataStore, compare: compareSchema.OrdersTableDataStore,
method: method: 'Automattic\\WooCommerce\\Internal\\DataStores\\Orders\\OrdersTableDataStore->get_database_schema',
'Automattic\\WooCommerce\\Internal\\DataStores\\Orders\\OrdersTableDataStore->get_database_schema',
areEqual: areEqual:
baseSchema.OrdersTableDataStore === baseSchema.OrdersTableDataStore ===
compareSchema.OrdersTableDataStore, compareSchema.OrdersTableDataStore,