Add new official build for runtimelab #2563
Conversation
|
FYI for msft employees, internal run ID is 2438214 |
Succeeded, but it did not publish the packages (e.g. https://dev.azure.com/dnceng/public/_artifacts/feed/dotnet-experimental/NuGet/runtime.browser-wasm.Microsoft.DotNet.ILCompiler.LLVM/) |
There was a problem hiding this comment.
Some high level questions.
What is it exactly that is required of the "official" .yml (that wasn't previously)?
Is it required to have a separate .yml instead of using runtimelab.yml with isOfficialBuild: true, or is it a code organization choice? Can/should we delete isOfficialBuild == true paths from runtimelab.yml now (downstream and upstream)?
| extends: | ||
| template: /eng/pipelines/common/templates/pipeline-with-resources.yml@self | ||
| parameters: | ||
| isOfficialBuild: true |
There was a problem hiding this comment.
Below in the file, there are places that check isOfficialBuild instead of hardcoding it. What is the intent behind that?
There was a problem hiding this comment.
It is likely that some are auto-generated from the conversion script, and some were just written by me by hand. Thus you have me hardcoding things to true (because this is the official build) and the script not knowing what to do and pulling the variable.
| templatePath: 'templates-official' | ||
| timeoutInMinutes: 300 | ||
| isOfficialBuild: ${{ variables.isOfficialBuild }} | ||
| testGroup: innerloop |
There was a problem hiding this comment.
testGroup: innerloop
Where is this parameter defined? I don't see it in https://github.com/dotnet/runtimelab/blob/feature/NativeAOT-LLVM/eng/pipelines/common/global-build-job.yml.
| ${{ if eq(variables.isOfficialBuild, true) }}: | ||
| buildArgs: -s clr.aot+libs+nativeaot.packages -c $(_BuildConfig) |
There was a problem hiding this comment.
This is an odd placement for buildArgs. I wonder if the indentation works out correctly.
| jobTemplate: /eng/pipelines/common/global-build-job.yml | ||
| buildConfig: Release | ||
| platforms: | ||
| - windows_arm64 |
There was a problem hiding this comment.
Note that we don't really need windows_arm64 here (and probably don't want it, since it doesn't get tested on PR CI).
Yup, expected -- it's branch gated so we won't get any new packages until this is merged. |
In short, we need to import an internal template that sets policy.
It's possible to share, but hard. The internal template requires certain tasks to be used, and those tasks aren't available in the public template. The current guidance is to split to internal and external.
Yes, although that's not strictly necessary. |
There was a problem hiding this comment.
Thank you for the explanation, it makes sense to me.
I have added a couple suggestions that hardcode the official build parameter and make the code a bit simpler.
Modulo that, LGTM. We can see to the runtimelab.yml changes separately.
I am assuming a version of this will go upstream. Is that correct?
| jobTemplate: /eng/pipelines/common/global-build-job.yml | ||
| buildConfig: Release | ||
| platforms: | ||
| - windows_arm64 |
There was a problem hiding this comment.
Note that we don't really need windows_arm64 here (and probably don't want it, since it doesn't get tested on PR CI).
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
I do not think we should be doing that. It makes official build flakier and the failures in official build are impossible for non-Microsoft people to see and debug. |
|
The current config is running tests, so I can instead just skip testing entirely. |
Yes, that would be better. |
Sorry, my mistake. |
|
np, simple change. Looks like the internal build passed. |
|
This brings in the same 1ES official build templates that are in runtime and are now required for all internal builds.