From 300612ca4edf16bbb2ad88d2bc74290b6b195f5a Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Wed, 23 Mar 2022 16:59:07 -0700 Subject: [PATCH] Sync eng/common directory with azure-sdk-tools for PR 2861 (#3425) * Use common script for git diff changes * remove extra files * suppress spell check * suppress false positive cspelling * Address comments * make changes to cspell * Suppress to right values * More fix Co-authored-by: sima-zhu --- .../pipelines/templates/steps/credscan.yml | 11 ++--- .../steps/eng-common-workflow-enforcer.yml | 18 +++---- eng/common/scripts/Detect-Api-Changes.ps1 | 8 ++-- eng/common/scripts/Helpers/git-helpers.ps1 | 38 +++++++++++++++ .../check-spelling-in-changed-files.ps1 | 11 ++--- eng/common/scripts/common.ps1 | 3 ++ eng/common/scripts/get-changedfiles.ps1 | 40 ++++++++++++++++ .../get-markdown-files-from-changed-files.ps1 | 48 +++---------------- eng/common/scripts/git-diff-changes.ps1 | 36 -------------- 9 files changed, 110 insertions(+), 103 deletions(-) create mode 100644 eng/common/scripts/Helpers/git-helpers.ps1 create mode 100644 eng/common/scripts/get-changedfiles.ps1 delete mode 100644 eng/common/scripts/git-diff-changes.ps1 diff --git a/eng/common/pipelines/templates/steps/credscan.yml b/eng/common/pipelines/templates/steps/credscan.yml index 3f61883b6..2d472a970 100644 --- a/eng/common/pipelines/templates/steps/credscan.yml +++ b/eng/common/pipelines/templates/steps/credscan.yml @@ -1,3 +1,7 @@ +# cSpell:ignore changedfiles +# cSpell:ignore credscan +# cSpell:ignore securedevelopmentteam +# cSpell:ignore postanalysis parameters: SuppressionFilePath: 'eng/CredScanSuppression.json' BaselineFilePath: '' @@ -7,12 +11,7 @@ parameters: steps: - pwsh: | if ("$(Build.Reason)" -eq 'PullRequest') { - $targetBranch = "origin/$(System.PullRequest.TargetBranch)" -replace "refs/heads/" - - # Add config to disable the quote and encoding on file name. - # Ref: https://github.com/msysgit/msysgit/wiki/Git-for-Windows-Unicode-Support#disable-quoted-file-names - # Ref: https://github.com/msysgit/msysgit/wiki/Git-for-Windows-Unicode-Support#disable-commit-message-transcoding - $changedFiles = git -c core.quotepath=off -c i18n.logoutputencoding=utf-8 diff $targetBranch HEAD --name-only --diff-filter=d + $changedFiles = & "eng/common/scripts/get-changedfiles.ps1" $changedFiles | ForEach-Object { Add-Content -Path "${{ parameters.SourceDirectory }}/credscan.tsv" -Value "${{ parameters.SourceDirectory }}/$_"} } else { diff --git a/eng/common/pipelines/templates/steps/eng-common-workflow-enforcer.yml b/eng/common/pipelines/templates/steps/eng-common-workflow-enforcer.yml index 58a8b5b48..6857710ec 100644 --- a/eng/common/pipelines/templates/steps/eng-common-workflow-enforcer.yml +++ b/eng/common/pipelines/templates/steps/eng-common-workflow-enforcer.yml @@ -1,18 +1,18 @@ +# cSpell:ignore changedfiles +# cSpell:ignore Committish +# cSpell:ignore LASTEXITCODE + steps: + - template: /eng/common/pipelines/templates/steps/set-default-branch.yml - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}: - pwsh: | - Set-PsDebug -Trace 1 - # Find the default branch of the repo - $defaultBranch = (git remote show origin | Out-String) -replace "(?ms).*HEAD branch: (\w+).*", '$1' + # Find the default branch of the repo. The variable value sets in build step. + Write-Host "Default Branch: $(DefaultBranch)" $targetBranch = "$(System.PullRequest.TargetBranch)" -replace "refs/heads/" - Write-Host "Default Branch: ${defaultBranch}" - - if ((!"$(System.PullRequest.SourceBranch)".StartsWith("sync-eng/common")) -and $targetBranch -eq $defaultBranch) + if ((!"$(System.PullRequest.SourceBranch)".StartsWith("sync-eng/common")) -and $targetBranch -eq "$(DefaultBranch)") { - - $filesInCommonDir = git diff "origin/${targetBranch}" HEAD --name-only -- 'eng/common/*' - + $filesInCommonDir = & "eng/common/scripts/get-changedfiles.ps1" -TargetCommittish $targetBranch -DiffPath 'eng/common/*' if (($LASTEXITCODE -eq 0) -and ($filesInCommonDir.Count -gt 0)) { Write-Host "##vso[task.LogIssue type=error;]Changes to files under 'eng/common' directory should not be made in this Repo`n${filesInCommonDir}" diff --git a/eng/common/scripts/Detect-Api-Changes.ps1 b/eng/common/scripts/Detect-Api-Changes.ps1 index fc55e672b..1c9cdf696 100644 --- a/eng/common/scripts/Detect-Api-Changes.ps1 +++ b/eng/common/scripts/Detect-Api-Changes.ps1 @@ -1,3 +1,5 @@ +# cSpell:ignore PULLREQUEST +# cSpell:ignore TARGETBRANCH [CmdletBinding()] Param ( [Parameter(Mandatory=$True)] @@ -16,6 +18,8 @@ Param ( [string] $TargetBranch = ("origin/${env:SYSTEM_PULLREQUEST_TARGETBRANCH}" -replace "refs/heads/") ) +. (Join-Path $PSScriptRoot common.ps1) + # Submit API review request and return status whether current revision is approved or pending or failed to create review function Submit-Request($filePath, $packageName) { @@ -61,8 +65,7 @@ function Should-Process-Package($pkgPath, $packageName) # Get package info from json file created before updating version to daily dev $pkgInfo = Get-Content $pkgPropPath | ConvertFrom-Json $packagePath = $pkgInfo.DirectoryPath - $modifiedFiles = git diff --name-only --relative $TargetBranch HEAD - $modifiedFiles = $modifiedFiles.Where({$_.startswith($packagePath)}) + $modifiedFiles = Get-ChangedFiles -DiffPath "$packagePath/*" -DiffFilterType '' $filteredFileCount = $modifiedFiles.Count Write-Host "Number of modified files for package: $filteredFileCount" return ($filteredFileCount -gt 0 -and $pkgInfo.IsNewSdk) @@ -80,7 +83,6 @@ function Log-Input-Params() Write-Host "Package Name: $($PackageName)" } -. (Join-Path $PSScriptRoot common.ps1) Log-Input-Params if (!($FindArtifactForApiReviewFn -and (Test-Path "Function:$FindArtifactForApiReviewFn"))) diff --git a/eng/common/scripts/Helpers/git-helpers.ps1 b/eng/common/scripts/Helpers/git-helpers.ps1 new file mode 100644 index 000000000..8c72542a0 --- /dev/null +++ b/eng/common/scripts/Helpers/git-helpers.ps1 @@ -0,0 +1,38 @@ +# cSpell:ignore Committish +# cSpell:ignore PULLREQUEST +# cSpell:ignore TARGETBRANCH +# cSpell:ignore SOURCECOMMITID +function Get-ChangedFiles { + param ( + [string]$SourceCommittish= "${env:SYSTEM_PULLREQUEST_SOURCECOMMITID}", + [string]$TargetCommittish = ("origin/${env:SYSTEM_PULLREQUEST_TARGETBRANCH}" -replace "refs/heads/"), + [string]$DiffPath, + [string]$DiffFilterType = "d" + ) + # If ${env:SYSTEM_PULLREQUEST_TARGETBRANCH} is empty, then return empty. + if ($TargetCommittish -eq "origin/") { + Write-Host "There is no target branch passed in. " + return "" + } + + # Add config to disable the quote and encoding on file name. + # Ref: https://github.com/msysgit/msysgit/wiki/Git-for-Windows-Unicode-Support#disable-quoted-file-names + # Ref: https://github.com/msysgit/msysgit/wiki/Git-for-Windows-Unicode-Support#disable-commit-message-transcoding + # Git PR diff: https://docs.github.com/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-comparing-branches-in-pull-requests#three-dot-and-two-dot-git-diff-comparisons + $command = "git -c core.quotepath=off -c i18n.logoutputencoding=utf-8 diff `"$TargetCommittish...$SourceCommittish`" --name-only --diff-filter=$DiffFilterType" + if ($DiffPath) { + $command = $command + " -- `'$DiffPath`'" + } + Write-Host $command + $changedFiles = Invoke-Expression -Command $command + if(!$changedFiles) { + Write-Host "No changed files in git diff between $TargetCommittish and $SourceCommittish" + } + else { + Write-Host "Here are the diff files:" + foreach ($file in $changedFiles) { + Write-Host " $file" + } + } + return $changedFiles +} diff --git a/eng/common/scripts/check-spelling-in-changed-files.ps1 b/eng/common/scripts/check-spelling-in-changed-files.ps1 index 1e179b4e4..50b4f71ba 100644 --- a/eng/common/scripts/check-spelling-in-changed-files.ps1 +++ b/eng/common/scripts/check-spelling-in-changed-files.ps1 @@ -1,3 +1,6 @@ +# cSpell:ignore LASTEXITCODE +# cSpell:ignore errrrrorrrrr +# cSpell:ignore sepleing <# .SYNOPSIS Uses cspell (from NPM) to check spelling of recently changed files @@ -286,13 +289,7 @@ if (!(Test-Path $CspellConfigPath)) { # Lists names of files that were in some way changed between the # current $SourceBranch and $TargetBranch. Excludes files that were deleted to # prevent errors in Resolve-Path -Write-Host "git diff --diff-filter=d --name-only --relative $TargetBranch $SourceBranch" -$changedFilesList = git diff ` - --diff-filter=d ` - --name-only ` - --relative ` - $TargetBranch ` - $SourceBranch +$changedFilesList = Get-ChangedFiles $changedFiles = @() foreach ($file in $changedFilesList) { diff --git a/eng/common/scripts/common.ps1 b/eng/common/scripts/common.ps1 index c01f9c9f5..6951c5a9a 100644 --- a/eng/common/scripts/common.ps1 +++ b/eng/common/scripts/common.ps1 @@ -1,3 +1,5 @@ +# cSpell:ignore Apireview +# cSpell:ignore Onboarded $RepoRoot = Resolve-Path "${PSScriptRoot}..\..\..\.." $EngDir = Join-Path $RepoRoot "eng" $EngCommonDir = Join-Path $EngDir "common" @@ -12,6 +14,7 @@ $EngScriptsDir = Join-Path $EngDir "scripts" . (Join-Path $EngCommonScriptsDir Invoke-GitHubAPI.ps1) . (Join-Path $EngCommonScriptsDir Invoke-DevOpsAPI.ps1) . (Join-Path $EngCommonScriptsDir artifact-metadata-parsing.ps1) +. (Join-Path $EngCommonScriptsDir "Helpers" git-helpers.ps1) # Setting expected from common languages settings $Language = "Unknown" diff --git a/eng/common/scripts/get-changedfiles.ps1 b/eng/common/scripts/get-changedfiles.ps1 new file mode 100644 index 000000000..df5542090 --- /dev/null +++ b/eng/common/scripts/get-changedfiles.ps1 @@ -0,0 +1,40 @@ + +# cSpell:ignore Committish +# cSpell:ignore committish +# cSpell:ignore PULLREQUEST +# cSpell:ignore TARGETBRANCH +# cSpell:ignore SOURCECOMMITID +# cSpell:ignore elete +# cSpell:ignore ename +<# + .SYNOPSIS + Returns git diff changes in pull request. + .DESCRIPTION + The script is to return diff changes in pull request. + .PARAMETER SourceCommittish + The branch committish PR merges from. + Definition of committish: https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-aiddefcommit-ishacommit-ishalsocommittish + .PARAMETER TargetCommittish + The branch committish PR targets to merge into. + .PARAMETER DiffPath + The files which git diff to scan against. Support regex match. E.g. "eng/common/*", "*.md" + .PARAMETER DiffFilterType + The filter type A(a)dd, D(d)elete, R(r)ename, U(u)pate. + E.g. 'ad' means filter out the newly added file and deleted file + E.g. '' means no filter on file mode. +#> +[CmdletBinding()] +param ( + [string] $SourceCommittish = "${env:SYSTEM_PULLREQUEST_SOURCECOMMITID}", + [string] $TargetCommittish = ("origin/${env:SYSTEM_PULLREQUEST_TARGETBRANCH}" -replace "refs/heads/"), + [string] $DiffPath = "", + [string] $DiffFilterType = 'd' +) + +Set-StrictMode -Version 3 +. (Join-Path $PSScriptRoot common.ps1) + +return Get-ChangedFiles -SourceCommittish $SourceCommittish ` +-TargetCommittish $TargetCommittish ` +-DiffPath $DiffPath ` +-DiffFilterType $DiffFilterType diff --git a/eng/common/scripts/get-markdown-files-from-changed-files.ps1 b/eng/common/scripts/get-markdown-files-from-changed-files.ps1 index 4e0d48bb8..f59f10391 100644 --- a/eng/common/scripts/get-markdown-files-from-changed-files.ps1 +++ b/eng/common/scripts/get-markdown-files-from-changed-files.ps1 @@ -1,49 +1,13 @@ +# cSpell:ignore Committish +# cSpell:ignore PULLREQUEST +# cSpell:ignore TARGETBRANCH param ( - # The root repo we scaned with. + # The root repo we scanned with. [string] $RootRepo = '$PSScriptRoot/../../..', # The target branch to compare with. [string] $targetBranch = ("origin/${env:SYSTEM_PULLREQUEST_TARGETBRANCH}" -replace "/refs/heads/") ) -$deletedFiles = (git diff $targetBranch HEAD --name-only --diff-filter=D) -$renamedFiles = (git diff $targetBranch HEAD --diff-filter=R) -$changedMarkdowns = (git diff $targetBranch HEAD --name-only -- '*.md') -$beforeRenameFiles = @() -# Retrieve the 'renamed from' files. Git command only returns back the files after rename. -# In order to have the files path before rename, it has to do some regex checking. -# It is better to be replaced by more reliable commands if any. -foreach ($file in $renamedFiles) { - if ($file -match "^rename from (.*)$") { - $beforeRenameFiles += $file -replace "^rename from (.*)$", '$1' - } -} -# A combined list of deleted and renamed files. -$relativePathLinks = ($deletedFiles + $beforeRenameFiles) -# Removed the deleted markdowns. -$changedMarkdowns = $changedMarkdowns | Where-Object { $deletedFiles -notcontains $_ } -# Scan all markdowns and find if it contains the deleted or renamed files. -$markdownContainLinks = @() -$allMarkdownFiles = Get-ChildItem -Path $RootRepo -Recurse -Include *.md -foreach ($f in $allMarkdownFiles) { - $filePath = $f.FullName - $content = Get-Content -Path $filePath -Raw - foreach ($l in $relativePathLinks) { - if ($content -match $l) { - $markdownContainLinks += $filePath - break - } - } -} +. (Join-Path $PSScriptRoot common.ps1) -# Convert markdowns path of the PR to absolute path. -$adjustedReadmes = $changedMarkdowns | Foreach-Object { Resolve-Path $_ } -$markdownContainLinks += $adjustedReadmes - -# Get rid of any duplicated ones. -$allMarkdowns = [string[]]($markdownContainLinks | Sort-Object | Get-Unique) - -Write-Host "Here are all markdown files we need to check based on the changed files:" -foreach ($file in $allMarkdowns) { - Write-Host " $file" -} -return $allMarkdowns +return Get-ChangedFiles -TargetCommittish $targetBranch -DiffPath '*.md' diff --git a/eng/common/scripts/git-diff-changes.ps1 b/eng/common/scripts/git-diff-changes.ps1 deleted file mode 100644 index c2f4befa5..000000000 --- a/eng/common/scripts/git-diff-changes.ps1 +++ /dev/null @@ -1,36 +0,0 @@ -<# - .SYNOPSIS - Returns git diff changes in pull request. - - .DESCRIPTION - The script is to return diff changes in pull request. - - .PARAMETER SourceCommittish - The branch committish PR merges from. - Definition of committish: https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-aiddefcommit-ishacommit-ishalsocommittish - - .PARAMETER TargetCommittish - The branch committish PR targets to merge into. -#> -[CmdletBinding()] -param ( - [string] $SourceCommittish = "${env:BUILD_SOURCEVERSION}", - [string] $TargetCommittish = ("origin/${env:SYSTEM_PULLREQUEST_TARGETBRANCH}" -replace "refs/heads/") -) - -# If ${env:SYSTEM_PULLREQUEST_TARGETBRANCH} is empty, then return empty. -if ($TargetCommittish -eq "origin/") { - Write-Host "There is no target branch passed in. " - return "" -} -# Git PR diff: https://docs.github.com/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-comparing-branches-in-pull-requests#three-dot-and-two-dot-git-diff-comparisons -Write-Host "git -c core.quotepath=off -c i18n.logoutputencoding=utf-8 diff $TargetCommittish...$SourceCommittish --name-only --diff-filter=d" -$changedFiles = (git -c core.quotepath=off -c i18n.logoutputencoding=utf-8 diff "$TargetCommittish...$SourceCommittish" --name-only --diff-filter=d) -if(!$changedFiles) { - Write-Host "No changed files in git diff between $TargetCommittish and $SourceCommittish" -} -Write-Host "Here are the diff files:" -foreach ($file in $changedFiles) { - Write-Host " $file" -} -return $changedFiles \ No newline at end of file