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.

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

Add a Nightly build pipeline for the Canary branding #15869

Merged
merged 6 commits into from
Aug 24, 2023

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented Aug 22, 2023

To make this happen, I moved most of release.yml into a shared pipeline template (which is larger than a steps or jobs template). Most of the diffs are due to that move.

If you compare main:build/pipelines/release.yml against
dev/duhowett/nightly-build:build/pipelines/templates-v2/pipeline-full-release-build.yml,
you will see that the changes are much more minimal than they look. (#15869 (comment))

I also added a parameter to configure how long symbols will be kept. It defaults to 36530 days (which is the default for the PublishSymbols task! Yes, 100 years!) but nightly builds will get 15 days.

The coolest part of this is how little I had to rewrite to do it.
`extends` is so cool!
@DHowett

This comment was marked as resolved.

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

fuckin yea

@avih

This comment was marked as resolved.

@zadjii-msft

This comment was marked as resolved.

@DHowett

This comment was marked as off-topic.

zadjii-msft

This comment was marked as resolved.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 23, 2023
@avih

This comment was marked as resolved.

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 23, 2023
@DHowett

This comment was marked as resolved.

@DHowett
Copy link
Member Author

DHowett commented Aug 23, 2023

@zadjii-msft okay, this is a much bigger diff now but only because I had to rename release.yml to templates-v2/xxxxxx and pull out some things that made it illegal to have in a template. I had to keep a different release.yml around though.

If you git diff origin/main:build/pipelines/release.yml origin/dev/duhowett/nightly-build:build/pipelines/templates-v2/pipeline-full-release-build.yml, you get this.

diff --git a/build/pipelines/release.yml b/build/pipelines/templates-v2/pipeline-full-release-build.yml
index c177634ba..42c5c6250 100644
--- a/build/pipelines/release.yml
+++ b/build/pipelines/templates-v2/pipeline-full-release-build.yml
@@ -1,35 +1,20 @@
 # This build should never run as CI or against a pull request.
 trigger: none
-pr: none
-
-pool:
-  name: SHINE-INT-S # By default, send jobs to the small agent pool.
-  demands: ImageOverride -equals SHINE-VS17-Latest
 
 parameters:
   - name: branding
-    displayName: "Branding (Build Type)"
     type: string
     default: Release
-    values:
-      - Release
-      - Preview
-      - Canary
-      - Dev
   - name: buildTerminal
-    displayName: "Build Windows Terminal MSIX"
     type: boolean
     default: true
   - name: buildConPTY
-    displayName: "Build ConPTY NuGet"
     type: boolean
     default: false
   - name: buildWPF
-    displayName: "Build Terminal WPF Control"
     type: boolean
     default: false
   - name: pgoBuildMode
-    displayName: "PGO Build Mode"
     type: string
     default: Optimize
     values:
@@ -37,39 +22,41 @@ parameters:
       - Instrument
       - None
   - name: buildConfigurations
-    displayName: "Build Configurations"
     type: object
     default:
       - Release
   - name: buildPlatforms
-    displayName: "Build Platforms"
     type: object
     default:
       - x64
       - x86
       - arm64
   - name: codeSign
-    displayName: "Sign all build outputs"
     type: boolean
     default: true
   - name: generateSbom
-    displayName: "Generate a Bill of Materials"
     type: boolean
     default: true
   - name: terminalInternalPackageVersion
-    displayName: "Terminal Internal Package Version"
     type: string
     default: '0.0.8'
 
   - name: publishSymbolsToPublic
-    displayName: "Publish Symbols to MSDL"
     type: boolean
     default: true
+  - name: symbolExpiryTime
+    type: string
+    default: 36530 # This is the default from PublishSymbols@2
   - name: publishVpackToWindows
-    displayName: "Publish VPack to Windows"
     type: boolean
     default: false
 
+  - name: pool
+    type: object
+    default:
+      name: SHINE-INT-S # By default, send jobs to the small agent pool.
+      demands: ImageOverride -equals SHINE-VS17-Latest
+
 variables:
   # If we are building a branch called "release-*", change the NuGet suffix
   # to "preview". If we don't do that, XES will set the suffix to "release1"
@@ -94,8 +81,6 @@ variables:
   ${{ elseif eq(variables['Build.SourceBranchName'], 'main') }}:
     NuGetPackBetaVersion: experimental
 
-name: $(BuildDefinitionName)_$(date:yyMM).$(date:dd)$(rev:rrr)
-
 resources:
   repositories:
   - repository: self
@@ -105,9 +90,10 @@ resources:
 stages:
   - stage: Build
     displayName: Build
+    pool: ${{ parameters.pool }}
     dependsOn: []
     jobs:
-      - template: ./templates-v2/job-build-project.yml
+      - template: ./job-build-project.yml
         parameters:
           pool:
             name: SHINE-INT-L # Run the compilation on the large agent pool, rather than the default small one.
@@ -134,13 +120,13 @@ stages:
                 packageListDownload: e82d490c-af86-4733-9dc4-07b772033204
                 versionListDownload: ${{ parameters.terminalInternalPackageVersion }}
 
-            - template: ./templates-v2/steps-fetch-and-prepare-localizations.yml
+            - template: ./steps-fetch-and-prepare-localizations.yml
               parameters:
                 includePseudoLoc: true
 
       - ${{ if eq(parameters.buildWPF, true) }}:
         # Add an Any CPU build flavor for the WPF control bits
-        - template: ./templates-v2/job-build-project.yml
+        - template: ./job-build-project.yml
           parameters:
             # This job is allowed to run on the default small pool.
             jobName: BuildWPF
@@ -161,10 +147,11 @@ stages:
 
   - stage: Package
     displayName: Package
+    pool: ${{ parameters.pool }}
     dependsOn: [Build]
     jobs:
       - ${{ if eq(parameters.buildTerminal, true) }}:
-        - template: ./templates-v2/job-merge-msix-into-bundle.yml
+        - template: ./job-merge-msix-into-bundle.yml
           parameters:
             jobName: Bundle
             branding: ${{ parameters.branding }}
@@ -174,7 +161,7 @@ stages:
             codeSign: ${{ parameters.codeSign }}
 
       - ${{ if eq(parameters.buildConPTY, true) }}:
-        - template: ./templates-v2/job-package-conpty.yml
+        - template: ./job-package-conpty.yml
           parameters:
             buildConfigurations: ${{ parameters.buildConfigurations }}
             buildPlatforms: ${{ parameters.buildPlatforms }}
@@ -182,7 +169,7 @@ stages:
             codeSign: ${{ parameters.codeSign }}
 
       - ${{ if eq(parameters.buildWPF, true) }}:
-        - template: ./templates-v2/job-build-package-wpf.yml
+        - template: ./job-build-package-wpf.yml
           parameters:
             buildConfigurations: ${{ parameters.buildConfigurations }}
             buildPlatforms: ${{ parameters.buildPlatforms }}
@@ -191,17 +178,19 @@ stages:
 
   - stage: Publish
     displayName: Publish
+    pool: ${{ parameters.pool }}
     dependsOn: [Build, Package]
     jobs:
       # We only support the vpack for Release builds that include Terminal
       - ${{ if and(containsValue(parameters.buildConfigurations, 'Release'), parameters.buildTerminal, parameters.publishVpackToWindows) }}:
-        - template: ./templates-v2/job-submit-windows-vpack.yml
+        - template: ./job-submit-windows-vpack.yml
           parameters:
             buildConfiguration: Release
             generateSbom: ${{ parameters.generateSbom }}
 
-      - template: ./templates-v2/job-publish-symbols.yml
+      - template: ./job-publish-symbols.yml
         parameters:
           includePublicSymbolServer: ${{ parameters.publishSymbolsToPublic }}
+          symbolExpiryTime: ${{ parameters.symbolExpiryTime }}

@DHowett
Copy link
Member Author

DHowett commented Aug 23, 2023

Right now, it's failing because it can't find origin/main for PGO purposes. This is not unexpected, but it does limit our ability to test on branches other than main.

@avih

This comment was marked as resolved.

@DHowett
Copy link
Member Author

DHowett commented Aug 23, 2023

Not sure I fully understand, and this does become kind of off topic, but if you mean to make the artifact archive contain only the minimal set of files to run the terminal, i.e. in this example maybe only the content of _unpackaged, then it could surely help in some cases - easier to find what needs to be executed, probably quite smaller archive.

So, at the end of the day, the build artifacts are effectively an internal contract between our build pipelines. They include everything because that's...

  • What we need to run the tests on a different machine than the one we built on
  • What we need to ship an update to the store
  • Where we get the unpackaged builds
  • Where the following steps that produce the WPF and ConPTY nuget packages get their binaries to put in the final nuget packages 😄

However, it would also exclude currently-useful artifacts which are otherwise hard to come by, such as conpty.dll which can be used by some third party terminal emulators (well.. more like wrappers of conpty currently, but still, potentially with a different feature set than the windows console and windows terminal).

Yeah, that's fair. I'm not aware of anybody using those build artifacts, because (1) we just changed them to actually include the conpty bits and (2) they're totally unsupported if they do 😄 (again, internal contract).

So until a newer conpty is available on its own (I think you do have such plans), then it can be useful that it's not excluded from the terminal artifacts IMHO.

Yep! #15065. Note that you need all three architectures of OpenConsole.exe and at least one architecture of conpty.dll and we might break you and they're not going to be code-signed by the nightly CI. 😄

We have plans to make all of the actual usable artifacts a little easier for people to find.

@avih
Copy link

avih commented Aug 23, 2023

Yeah, that's fair. I'm not aware of anybody using those build artifacts

wezterm distributes a copy conpty.dll and OpenConsole.exe (which were built by forking this repo and building it), but it's not necessarily up to date, and personally I prefer an upstream artifact where possible.

alacritty (master) can load/use such conpty.dll at the binary dir (it seems to work with the file I got from the artifacts here), but they don't distribute it.

I don't know whether vscode terminal would use it if I place it together with OpenConsole.exe at binary dir. Hopefully it would.

One main advantage a newer conpty.dll has for me over the native one (win10), other than hopefully general improvements, is that it supports mouse (wheel?) events, unlike the native one, which can be useful for instance with less for Windows, but possibly elsewhere too, and for me that's the main reason I have use for this dll.

For instance, alacritty will not mouse-wheel scroll in less-for-windows without a new compty dll.

New conpty (conhost?) also seems a lot faster in processing (unicode?) input compared to the native one - about 4x faster in some quick test, and while the use cases for fast output are lesser than mouse wheel support, IMHO, it's still great.

because (1) we just changed them to actually include the conpty bits and (2) they're totally unsupported if they do

Right, today was the 1st time I downloaded the artifacts here, and conpty.dll was there, I didn't realize it's a new thing. Obviously one should not expect any guarantees with such use cases, but if it works - and it seems to, then great :)

Thanks again for the continual improvements!

@zadjii-msft
Copy link
Member

@avih oh you'd be very interested in #15065, which is tracking producing specifically an official ConPTY package

@DHowett DHowett enabled auto-merge (squash) August 24, 2023 19:11
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

unblocking you for the weekend

@DHowett DHowett disabled auto-merge August 24, 2023 21:54
@DHowett DHowett enabled auto-merge (squash) August 24, 2023 21:54
@DHowett DHowett disabled auto-merge August 24, 2023 21:54
@DHowett DHowett enabled auto-merge (squash) August 24, 2023 21:54
lhecker
lhecker approved these changes Aug 24, 2023
Comment on lines +174 to +177
buildConfigurations: ${{ parameters.buildConfigurations }}
buildPlatforms: ${{ parameters.buildPlatforms }}
generateSbom: ${{ parameters.generateSbom }}
codeSign: ${{ parameters.codeSign }}
Copy link
Member

Choose a reason for hiding this comment

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

Pretty boilerplaty.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea unfortunately :(

@DHowett DHowett disabled auto-merge August 24, 2023 21:56
@DHowett DHowett enabled auto-merge (squash) August 24, 2023 21:56
@DHowett DHowett disabled auto-merge August 24, 2023 21:56
@DHowett DHowett enabled auto-merge (squash) August 24, 2023 21:57
@DHowett DHowett merged commit 5651f08 into main Aug 24, 2023
15 of 17 checks passed
@DHowett DHowett deleted the dev/duhowett/nightly-build branch August 24, 2023 22:15
@DHowett
Copy link
Member Author

DHowett commented Aug 30, 2023

I have to backport this because it is foundational work for future build system improvements. Bah.

DHowett added a commit that referenced this pull request Aug 30, 2023
To make this happen, I moved most of `release.yml` into a shared
_pipeline_ template (which is larger than a steps or jobs template).
Most of the diffs are due to that move.

If you compare main:build/pipelines/release.yml against
dev/duhowett/nightly-build:build/pipelines/templates-v2/pipeline-full-release-build.yml,
you will see that the changes are much more minimal than they look.

I also added a parameter to configure how long symbols will be kept. It
defaults to 36530 days (which is the default for the PublishSymbols
task! Yes, 100 years!) but nightly builds will get 15 days.

(cherry picked from commit 5651f08)
Service-Card-Id: 90368683
Service-Version: 1.17
DHowett added a commit that referenced this pull request Aug 30, 2023
To make this happen, I moved most of `release.yml` into a shared
_pipeline_ template (which is larger than a steps or jobs template).
Most of the diffs are due to that move.

If you compare main:build/pipelines/release.yml against
dev/duhowett/nightly-build:build/pipelines/templates-v2/pipeline-full-release-build.yml,
you will see that the changes are much more minimal than they look.

I also added a parameter to configure how long symbols will be kept. It
defaults to 36530 days (which is the default for the PublishSymbols
task! Yes, 100 years!) but nightly builds will get 15 days.

(cherry picked from commit 5651f08)
Service-Card-Id: 90368684
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Cherry Picked
Development

Successfully merging this pull request may close these issues.

4 participants