Switching to Jobs from Phases for YAML and also switch to new pools. #3062

livarcocc · 2019-03-29T20:12:02Z

No description provided.

eng/build.yml

chcosta

For confidence, when I'm making changes like this. I like to manually push to the internal git repo and schedule a build to validate the internal builds are still working before I merge a major yaml change.

chcosta · 2019-03-29T20:51:52Z

eng/build.yml

+  strategy: {}
+
+  # Job timeout
+  timeoutInMinutes: 180


nit, your default is 180, but you're explicitly passing 120 everywhere. You might just set the default to 120 and then not override the default.

chcosta · 2019-03-29T21:00:32Z

eng/build.yml

-                  /p:DotNetSymbolServerTokenSymWeb=$(symweb-symbol-server-pat)
-                  /p:PB_PublishType=$(_PublishType)
-        _SignArgs: /p:DotNetSignType=$(_SignType) /p:TeamName=$(_TeamName)
+      - ${{ insert }}: ${{ parameters.variables }}


I think that this might work, but only because you only have one variable defined in .vsts-ci.yml. As soon as you add another variable, I think yaml will get confused because you'll be trying to insert the yaml variables "object" into the variables "array" and it won't know what to do with that. The way to fix this would be to change .vsts-ci.yml to pass variables as an array...

variables: - _PREVIEW_VSTS_DOCKER_IMAGE: microsoft/dotnet-buildtools-prereqs:ubuntu-16.04-cross-e435274-20180628134544

… and then insert each of those items here like we do in job.yml...

https://github.com/dotnet/arcade/blob/master/eng/common/templates/job/job.yml#L89-L109

chcosta · 2019-03-29T21:05:46Z

eng/build.yml

-phases:
- template: /eng/common/templates/phases/base.yml
+  # Build strategy - matrix
+  strategy: {}


change this to '' so that it aligns properly with the condition below that is checking to see if "strategy" is an empty string or not. I'm not sure if the comparison of {} resolves to '' or not.

chcosta · 2019-03-29T21:06:01Z

eng/build.yml

-        TargetFolder: '$(Build.ArtifactStagingDirectory)'
-      continueOnError: true
-      condition: not(succeeded())
+    - ${{ if and(eq(parameters.enablePublishBuildAssets, true), eq(parameters.runAsPublic, 'false'), ne(variables['System.TeamProject'], 'public'), notin(variables['Build.Reason'], 'PullRequest')) }}:


You should be able to delete this section of code as it is handled automatically by job.yml since you specified enablePublishBuildAssets=true above.

chcosta · 2019-03-29T21:09:02Z

.vsts-ci.yml

    _DotNetPublishToBlobFeed: true

-phases:
+jobs:


The variables above need to be defined using the array syntax. You can either define variables using the object syntax or the array syntax, but you can't mix n' match.

https://github.com/dotnet/arcade/blob/master/Documentation/AzureDevOps/PhaseToJobSchemaChange.md#phase-to-job-schema-transition

We've settled on the array syntax because the object syntax doesn't support variable groups. In its current state, I think all of the above variables will just get silently ignored.

chcosta

One not, one comment, and I think you're aware that the docker thing will probably be broken until addressed.

chcosta · 2019-04-01T22:48:34Z

.vsts-ci.yml

-    PB_PublishBlobFeedKey:
-    PB_PublishBlobFeedUrl:
-    _DotNetPublishToBlobFeed: false
+    - name: PB_PublishBlobFeedUrl


nit: For variables defined using array syntax, you could just define the default and then override with the internal condition. ie.

variables: - name: teamName value: Roslyn-Project-System - name: PB_PublishBlobFeedUrl value: '' - ${{ if ne(variables['System.TeamProject'], 'public') }}: - name: PB_PublishBlobFeedUrl value: https://dotnetfeed.blob.core.windows.net/dotnet-core/index.json - name: _DotNetPublishToBlobFeed value: true

chcosta · 2019-04-01T22:49:39Z

.vsts-ci.yml

-    PB_PublishBlobFeedKey:
-    PB_PublishBlobFeedUrl:
-    _DotNetPublishToBlobFeed: false
+    - name: PB_PublishBlobFeedUrl


The conditions in this variables block have to be treated the same way as the rest of the values in the object. ie, you have to treat them as array entries meaning prefix them with -

ie - ${{ if eq(variables['System.TeamProject'], 'public') }}:

livarcocc · 2019-04-01T23:12:01Z

Ack on the docker issue.

johnbeisner · 2019-04-04T16:16:32Z

closing and re-opening...

johnbeisner · 2019-04-04T17:26:49Z

@chcosta
Why do the legs timeout at 60 minutes when the variable "timeoutInMinutes" is set to "120"?

chcosta · 2019-04-04T19:48:57Z

eng/build.yml

+  strategy: ''
+
+  # Job timeout
+  timeoutInMinutes: 120


It doesn't appear that this property (timeoutInMinutes) is being passed through to the job.yml template.

johnbeisner · 2019-04-04T20:52:44Z

closing PR in favor of: #3078

….2 (dotnet#3062) - Microsoft.DotNet.Cli.Runtime - 3.1.100-preview1.19504.2

Switching to Jobs from Phases for YAML and also switch to new pools.

d2ddb32

livarcocc added this to the 3.0.1xx milestone Mar 29, 2019

livarcocc requested review from a team and chcosta March 29, 2019 20:12

chcosta requested changes Mar 29, 2019

View reviewed changes

eng/build.yml Show resolved Hide resolved

eng/build.yml Outdated Show resolved Hide resolved

chcosta reviewed Mar 29, 2019

View reviewed changes

chcosta reviewed Apr 1, 2019

View reviewed changes

Addressing code review comments.

b130d41

livarcocc force-pushed the change_yaml_to_use_jobs branch from 90cf3ad to b130d41 Compare April 1, 2019 23:11

johnbeisner closed this Apr 4, 2019

johnbeisner reopened this Apr 4, 2019

johnbeisner closed this Apr 4, 2019

johnbeisner reopened this Apr 4, 2019

johnbeisner closed this Apr 4, 2019

johnbeisner reopened this Apr 4, 2019

chcosta reviewed Apr 4, 2019

View reviewed changes

johnbeisner closed this Apr 4, 2019

chcosta mentioned this pull request Apr 11, 2019

Switching to Jobs from Phases for YAML #3078

Merged

dsplaisted pushed a commit to dsplaisted/sdk that referenced this pull request Feb 19, 2020

Update dependencies from https://github.com/dotnet/cli build 20191004…

a5d1be3

….2 (dotnet#3062) - Microsoft.DotNet.Cli.Runtime - 3.1.100-preview1.19504.2

Switching to Jobs from Phases for YAML and also switch to new pools. #3062

Switching to Jobs from Phases for YAML and also switch to new pools. #3062

Uh oh!

Conversation

livarcocc commented Mar 29, 2019

Uh oh!

Uh oh!

Uh oh!

chcosta left a comment

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!

Choose a reason for hiding this comment

Uh oh!

chcosta left a comment

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!

livarcocc commented Apr 1, 2019

Uh oh!

johnbeisner commented Apr 4, 2019

Uh oh!

johnbeisner commented Apr 4, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnbeisner commented Apr 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants