correct perf-send-to-helix on windows

[ooooolivia]

Fixing changes in this commit 0956285
Currently perf jobs on windows are running in bash and not being sent to helix.

[trylek]

Looks great, thanks for fixing this!

[trylek]

YAML identifiers should be case insensitive. I think you need to use variables['Agent.Os'] but use a different substitution syntax. I'll look it up after our team meeting, I think it's something like $[[ (instead of ${{) that expands later so that it "waits" until the variable gets populated. @DrewScoggins might remember the syntax off the top of his head.

[ooooolivia]

Is it possible if we directly use this variable as variables['Agent.Os'] in

runtime/eng/pipelines/common/templates/runtimes/send-to-helix-inner-step.yml

Line 10 in 0956285

- ${{ if eq(parameters.osGroup, 'Windows_NT') }}:

instead of passing it as a parameter?

[trylek]

$[, see

https://docs.microsoft.com/en-us/azure/devops/pipelines/process/expressions?view=azure-devops

[trylek]

I think it should be $[ variables('Agent.Os') ], at least I hope so

[trylek]

Well, the problem is that the inner-step is called from three different scripts, we would need to double-check that variables['Agent.Os'] work everywhere, it can likely break for cross-targeting builds.

[ooooolivia]

Thanks for sending the doc! I'll test $[ variables('Agent.Os') ] first

[ooooolivia]

From the log it seems the $[ variables['Agent.Os'] ] is not expanded before evaluating the condition:

Begin evaluating template '/eng/pipelines/common/templates/runtimes/send-to-helix-inner-step.yml'
Evaluating: eq(parameters['osGroup'], 'Windows_NT')
Expanded: eq('$[ variables[''Agent.Os''] ]', 'Windows_NT')
Result: False
Evaluating: ne(parameters['osGroup'], 'Windows_NT')
Expanded: ne('$[ variables[''Agent.Os''] ]', 'Windows_NT')
Result: True

[ooooolivia]

It seems that the problem is.. the variable won't be populated until runtime but the condition evaluation happens at compile time? I was thinking we might want to try using runtime expression here $[ ] instead of compile expression

runtime/eng/pipelines/common/templates/runtimes/send-to-helix-inner-step.yml

Line 10 in 0956285

- ${{ if eq(parameters.osGroup, 'Windows_NT') }}:

@trylek

[ooooolivia]

I found another place that uses this condition

runtime/eng/pipelines/common/xplat-setup.yml

Line 45 in bb921d6

- ${{ if eq(parameters.osGroup, 'Windows_NT') }}:

but in this case it works because the variable's ready at compile time

runtime/eng/pipelines/common/platform-matrix.yml

Line 36 in d992e0c

osGroup: Linux

[DrewScoggins]

Looking at eng\common\templates\phases\publish-build-assets.yml it seems that what we should be doing is $(Agent.Os), or at least that is the syntax that is used in that yml file and seems to work.

- task: PublishBuildArtifacts@1
          displayName: Publish Logs to VSTS
          inputs:
            PathtoPublish: '$(Build.SourcesDirectory)/artifacts/log/$(_BuildConfig)'
            PublishLocation: Container
            ArtifactName: $(Agent.Os)_Asset_Registry_Publish
          continueOnError: true
          condition: always()

[ooooolivia]

Looking at eng\common\templates\phases\publish-build-assets.yml it seems that what we should be doing is $(Agent.Os), or at least that is the syntax that is used in that yml file and seems to work.

- task: PublishBuildArtifacts@1
          displayName: Publish Logs to VSTS
          inputs:
            PathtoPublish: '$(Build.SourcesDirectory)/artifacts/log/$(_BuildConfig)'
            PublishLocation: Container
            ArtifactName: $(Agent.Os)_Asset_Registry_Publish
          continueOnError: true
          condition: always()

We will see whether this will work in the current build. I suppose $(Agent.Os) has value here because it gets expanded in runtime, but the condition that evaluates the parameter OSGroup happens at compile time. @DrewScoggins

[ooooolivia]

Works with parameters.osGroup passed to perf-send-to-helix.yml @DrewScoggins https://dev.azure.com/dnceng/public/_build/results?buildId=817998&view=logs&j=e46328bd-abc1-50e1-2136-d10902647fd9&t=09232069-e64e-5111-d89c-2234e5b017ef

[trylek]

Thanks so much Olivia for your perseverance in pushing this through!

[ooooolivia]

Just noticed arcade is not update with both commits ( 0956285 and this)... will submit both commits to arcade so they won't be overriden.

[akoeplinger]

@ooooolivia @trylek it looks like 0956285 didn't make it to arcade, only this PR.

[ooooolivia]

@ooooolivia @trylek it looks like 0956285 didn't make it to arcade, only this PR.

Missed updating non-perf changes in Arcade; new PR submitted in Arcade dotnet/arcade#6227