⚡Improve Build Performance by Using Invoke-Build Incremental Builds #3619

JustinGrote · 2021-10-15T20:57:05Z

Improves the InvokeBuild script to use the Incremental Tasks feature to only rebuild components that have changed. Dramatically shortens the inner dev loop when making changes.
https://github.com/nightroman/Invoke-Build/wiki/Incremental-Tasks

PR Summary

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

PR has a meaningful title
Summarized changes
PR has tests
This PR is ready to merge and is not work in progress
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

andyleejordan · 2021-10-15T21:05:10Z

vscode-powershell.build.ps1

 }

 #endregion
 #region Build tasks

-task BuildEditorServices -If (Get-EditorServicesPath) {
+Task BuildEditorServices -Input {


Not sure if this is going to work. This task and build script is specifically setup to allow the build to work when the PSES repo does not exist, but a drop of PSES (like from the signing build) has been placed in the appropriate location.

Shouldn't something like substituting in a separate PSES be explicit? I could add a -UseExistingPSES switch or something to the script and the appropriate same place on the CI for that scenario

JustinGrote · 2021-10-15T21:08:18Z

Not sure if this is going to work. This task and build script is specifically setup to allow the build to work when the PSES repo does not exist, but a drop of PSES (like from the signing build) has been placed in the appropriate location.

I thought that's what specifying the EditorServicesRepoPath via the param is supposed to do...

Wait I kind of see what you are saying, one sec.

andyleejordan

Not sure I understand why these changes need to be made to this build script. As it exists, it simply invokes the build in the PSES repo (if it exists) and copies it to the appropriate location. Improvements to the PSES build script to rebuild itself incrementally would be welcome, but definitely don't belong here, they should be as decoupled as possible in order to support dropping PSES builds manually (or via automation). And that improvement shouldn't require changes here, as the cd PSES && Invoke-Build would automatically become incremental if done there.

andyleejordan · 2021-10-15T21:09:24Z

Don't get me wrong, I appreciate it, but I massaged this build script to hell that it it guarantees a clean and correct build for both dev and release/signing scenarios, with that prioritized over speed (and the slowest thing here is restore Node modules, running TSC to check types, and running the PSES build itself).

JustinGrote · 2021-10-15T21:16:43Z

Not sure I understand why these changes need to be made to this build script. As it exists, it simply invokes the build in the PSES repo (if it exists) and copies it to the appropriate location. Improvements to the PSES build script to rebuild itself incrementally would be welcome, but definitely don't belong here, they should be as decoupled as possible in order to support dropping PSES builds manually (or via automation). And that improvement shouldn't require changes here, as the cd PSES && Invoke-Build would automatically become incremental if done there.

The intent was to enable changes to be made to the PSES repository and be able to just click "run extension" to see the changes, as of today you have to clean and do a full rebuild, so the time between change and testing the change is 30-45 seconds. I have a small separate PR to allow incremental builds in PSES and a further PR to do incremental builds to that script as well.

andyleejordan · 2021-10-18T16:51:29Z

I had an idea last night @JustinGrote: What if we introduced a Link task that instead of copying ../pses/module to the extension build, it symlinked it! It would need to be separate because that would NOT work in a VSIX package, but as a dev we could use the task to set things up. Then the incremental build of PSES will just update the files directly in that folder and the extension just needs to be relaunched.

JustinGrote · 2021-10-18T17:09:37Z

@andschwa seems fine to me, only issue I can think of are really edge case.

andyleejordan · 2021-10-18T18:08:15Z

vscode-powershell.build.ps1

    Write-Host "`n### Running extension tests" -ForegroundColor Green
-    exec { & npm run test }
+    if ($ENV:TERM_PROGRAM -eq 'vscode') {
+        Write-Warning 'E2E Tests cannot be performed from the CLI while vscode is running, please close vscode and run again or use the "Launch Extension Tests" launch config'


Sure they can, that's the whole reason it opens up a copy of Insiders.

Maybe because I was running on insiders, it threw an error for me every time and I couldn't find a way to specific like a "data-only" copy of insiders.

andyleejordan · 2021-10-18T18:08:46Z

vscode-powershell.build.ps1

+    $editorServicesPath = Get-EditorServicesPath
+    $editorServicesBuildScriptPath = Get-EditorServicesBuildScriptPath
+} catch {
+    throw 'Powershell Editor Services repo was not detected. Specify its path via the -EditorServicesRepoPath parameter or ensure it is available in the directory above {0}' -f $PSScriptRoot


Ok so this change should no longer be necessary with #3623.

andyleejordan · 2021-10-18T18:09:36Z

vscode-powershell.build.ps1

+
+$restoreCheckpoint = "$PSScriptRoot/node_modules/restored"
+#This will run when node_modules is empty or when package.json is updated
+Task Restore -Input $PSScriptRoot/package.json -Output { $RestoreCheckpoint } {


Why not just -Input package.json -Output node_modules?

andyleejordan · 2021-10-18T18:13:54Z

vscode-powershell.build.ps1

+    #We use these files as a freshness indicator since they always gets updated on a build
+    #TODO: More granular incremental build on PSES tasks
+    Join-Path $editorServicesPath 'module/PowerShellEditorServices/bin/Core/Microsoft.PowerShell.EditorServices.Hosting.dll'
+} -If $editorServicesBuildScriptPath {


All of this shouldn't be necessary any more.

andyleejordan · 2021-10-18T18:14:07Z

vscode-powershell.build.ps1

    Write-Host "`n### Copying PowerShellEditorServices module files" -ForegroundColor Green
-    Copy-Item -Recurse -Force "$(Split-Path (Get-EditorServicesPath))/module/*" ./modules
+    Copy-Item $PSESModulePath/* -Recurse -Force -Destination (Join-Path $PSScriptRoot 'modules')


andyleejordan · 2021-10-18T18:15:28Z

vscode-powershell.build.ps1

+} -Output {
+    Join-Path $PSScriptRoot 'out/main.js'
+    Get-ChildItem -File -Recurse (Join-Path $PSScriptRoot 'out')
+} CopyEditorServices, Restore, {


Instead of this, since esbuild is soooo quick, what if we moved the calls of tsc all to the lint step and did the lint when running the tests instead? Then rebuilding the extension doesn't need to be incremental, and the tests can be the "slower" part. (Note that esbuild does not build the test files, just the extension.)

andyleejordan · 2021-10-18T18:15:45Z

vscode-powershell.build.ps1

@@ -104,13 +145,21 @@ task UpdateReadme -If { $script:IsPreviewExtension } {
    $readmePath = (Join-Path $PSScriptRoot README.md)

    $readmeContent = Get-Content -Path $readmePath
-    if (!($readmeContent -match "This is the PREVIEW version of the PowerShell extension")) {
+    if (!($readmeContent -match 'This is the PREVIEW version of the PowerShell extension')) {


Can we save changes like this for a different PR?

andyleejordan · 2021-10-18T18:16:32Z

vscode-powershell.build.ps1

+    Get-ChildItem $PSScriptRoot -Exclude '.vscode-test', 'logs', 'sessions', 'node_modules', '.git' |
+        Get-ChildItem -Recurse -File
+} -Output {
+    $vsix = Get-ChildItem $PSScriptRoot/*.vsix
+    if (-not $VSIX) {
+        'No VSIX Created Yet'
+    } else { $vsix }


Not sure why this is necessary. The VSIX is only really created by the release pipeline, and only occasionally by a developer (and doesn't need to be incremental).

andyleejordan · 2022-01-20T22:26:41Z

Gonna close this one Justin as the build has changed a lot (though it was inspirational!)

JustinGrote · 2022-01-20T23:12:43Z

Fair!

⚡Improve Build Performance by Using Invoke-Build Incremental Builds

3392fe8

andyleejordan reviewed Oct 15, 2021

View reviewed changes

andyleejordan mentioned this pull request Oct 18, 2021

Add LinkEditorServices task for developers #3623

Merged

andyleejordan reviewed Oct 18, 2021

View reviewed changes

JustinGrote marked this pull request as draft November 10, 2021 19:42

andyleejordan closed this Jan 20, 2022

JustinGrote deleted the JustinGrote/incrementalBuilds branch January 20, 2022 23:12

⚡Improve Build Performance by Using Invoke-Build Incremental Builds #3619

⚡Improve Build Performance by Using Invoke-Build Incremental Builds #3619

Uh oh!

Conversation

JustinGrote commented Oct 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JustinGrote commented Oct 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andyleejordan left a comment

Choose a reason for hiding this comment

Uh oh!

andyleejordan commented Oct 15, 2021

Uh oh!

JustinGrote commented Oct 15, 2021

Uh oh!

andyleejordan commented Oct 18, 2021

Uh oh!

JustinGrote commented Oct 18, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyleejordan commented Jan 20, 2022

Uh oh!

JustinGrote commented Jan 20, 2022

Uh oh!

Uh oh!

JustinGrote commented Oct 15, 2021 •

edited

Loading

JustinGrote commented Oct 15, 2021 •

edited

Loading