Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Many build.ps1 improvements #15135

Merged
merged 14 commits into from
Jul 27, 2021
Merged

Many build.ps1 improvements #15135

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
838f9aa
[build.ps1] Add config filename option
chamons Jul 23, 2021
73ae0e4
[build.ps1] Add outputFolder option
chamons Jul 23, 2021
601e4e2
[build.ps1] Remove unncessary indirection
chamons Jul 23, 2021
4f081bc
Fix inconsistent spacing
chamons Jul 23, 2021
fcf7db4
[build.ps1] Use finally to always return to current directory
chamons Jul 23, 2021
98870a0
[build.ps1] Refactor structure
chamons Jul 23, 2021
e607697
Fix unrelated change
chamons Jul 23, 2021
adebe93
Make get_module_dirs have a top level function and invoke it from bui…
chamons Jul 23, 2021
7bfe8ba
Update eng/scripts/build.ps1
chamons Jul 26, 2021
1238b3e
Update eng/scripts/get_module_dirs.ps1
chamons Jul 26, 2021
428d589
Update eng/scripts/build.ps1
chamons Jul 26, 2021
3c8dbd7
Update eng/scripts/get_module_dirs.ps1
chamons Jul 26, 2021
ef524e0
Convert all existing powershell code to 4 space standard
chamons Jul 26, 2021
fd6ab1d
Merge branch 'main' into build_improvements
chamons Jul 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 46 additions & 39 deletions eng/scripts/build.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,67 +1,74 @@
#Requires -Version 7.0
param($filter, [switch]$clean, [switch]$vet, [switch]$generate, [switch]$skipBuild)
param([string]$filter, [switch]$clean, [switch]$vet, [switch]$generate, [switch]$skipBuild, [string]$config = "autorest.md", [string]$outputFolder)

. $PSScriptRoot/meta_generation.ps1
. $PSScriptRoot/get_module_dirs.ps1

$startingDirectory = Get-Location
$root = Resolve-Path ($PSScriptRoot + "/../..")
Set-Location $root
$sdks = @{};

foreach ($sdk in (./eng/scripts/get_module_dirs.ps1 -serviceDir 'sdk/...')) {
$name = $sdk | split-path -leaf
$sdks[$name] = @{
'path' = $sdk;
'clean' = $clean;
'vet' = $vet;
'generate' = $generate;
'skipBuild' = $skipBuild;
'root' = $root;
}
}

$keys = $sdks.Keys | Sort-Object;
if (![string]::IsNullOrWhiteSpace($filter)) {
Write-Host "Using filter: $filter"
$keys = $keys.Where( { $_ -match $filter })
}

$keys | ForEach-Object { $sdks[$_] } | ForEach-Object {
Push-Location $_.path

if ($_.clean) {
Write-Host "##[command]Executing go clean -v ./... in " $_.path
function Process-Sdk ($path) {
if ($clean) {
Write-Host "##[command]Executing go clean -v ./... in " $path
go clean -v ./...
}

if ($_.generate) {
Write-Host "##[command]Executing autorest.go in " $_.path
$autorestPath = $_.path + "/autorest.md"
if ($generate) {
Write-Host "##[command]Executing autorest.go in " $path
$autorestPath = $path + "/" + $config

if (ShouldGenerate-AutorestConfig $autorestPath) {
Generate-AutorestConfig $autorestPath
$removeAutorestFile = $true
}

$autorestVersion = "@autorest/go@4.0.0-preview.23"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we look the version up from a file or pass via argument? This string is likely to get out of date

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My idea was that this was the single place we'd bump the autorest version.

I could move it out to another file, but in any case there will be one place to rule them all.

$outputFolder = $_.path
$root = $_.root
if ($outputFolder -eq '') {
$outputFolder = $path
}
autorest --use=$autorestVersion --go --track2 --go-sdk-folder=$root --output-folder=$outputFolder --file-prefix="zz_generated_" --clear-output-folder=false $autorestPath
if ($removeAutorestFile) {
Remove-Item $autorestPath
}
}
if (!$_.skipBuild) {
Write-Host "##[command]Executing go build -v ./... in " $_.path

if (!$skipBuild) {
Write-Host "##[command]Executing go build -x -v ./... in " $path
go build -x -v ./...
Write-Host "##[command]Build Complete!"

}
if ($_.vet) {
Write-Host "##[command]Executing go vet ./... in " $_.path

if ($vet) {
Write-Host "##[command]Executing go vet ./... in " $path
go vet ./...
}
Pop-Location

}

Set-Location $startingDirectory
$startingDirectory = Get-Location
$root = Resolve-Path ($PSScriptRoot + "/../..")
Set-Location $root
$sdks = @{};

foreach ($sdk in (Get-ModuleDirs 'sdk/...')) {
$name = $sdk | split-path -leaf
$sdks[$name] = @{
'path' = $sdk;
}
}

$keys = $sdks.Keys | Sort-Object;
if (![string]::IsNullOrWhiteSpace($filter)) {
Write-Host "Using filter: $filter"
$keys = $keys.Where( { $_ -match $filter })
}

try {
$keys | ForEach-Object { $sdks[$_] } | ForEach-Object {
Push-Location $_.path
Process-Sdk $_.path
Pop-Location
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I'm looking at my suggestion, I'm not convinced it's any better, but since I already wrote it I might as well show you.

If you default the filter value to .* then you can avoid all the extra data manipulation with one pipeline:

try {
    $sdks.GetEnumerator()
    | Where-Object {
      Write-Host "Using filter: $filter"
      $_.key -match $filter
    }
    | Sort-Object -Property key
    | ForEach-Object {
        Push-Location $_.value.path
        Process-Sdk $_.value.path
        Pop-Location
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure I like yours better, but that is very interesting. I didn't realize you could do that in a one liner.

finally {
Set-Location $startingDirectory
}
22 changes: 14 additions & 8 deletions eng/scripts/get_module_dirs.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@ Param(
[string] $serviceDir
)

$modDirs = [Collections.Generic.List[String]]@()
function Get-ModuleDirs ([string] $serviceDir) {
$modDirs = [Collections.Generic.List[String]]@()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another nit, can we keep whitespacing consistent, this looks like two space tabs, but build.ps1 looks like four space tabs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pre-existing ps1 code was all 2 spaces, but that is not my preferred spacing at all. Hey @benbp is there a standard spacing for powershell code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only source I can find. I also prefer four spaces in general.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using the built in code formatter on save in VSCode, and had some trouble convincing it to do what I wanted:

        "editor.tabSize": 4,
        "editor.detectIndentation": false

did the job.


# find each module directory under $serviceDir
Get-Childitem -recurse -path $serviceDir -filter go.mod | foreach-object {
$cdir = $_.Directory
Write-Host "Adding $cdir to list of module paths"
$modDirs.Add($cdir)
# find each module directory under $serviceDir
Get-Childitem -recurse -path $serviceDir -filter go.mod | foreach-object {
$cdir = $_.Directory
Write-Host "Adding $cdir to list of module paths"
$modDirs.Add($cdir)
}

# return the list of module directories
return $modDirs
}

# return the list of module directories
return $modDirs
if ($MyInvocation.InvocationName -ne ".") {
Get-ModuleDirs $serviceDir
}