Add installer build/test to the runtime.yml pipeline #705

dagood · 2019-12-09T19:52:14Z

Adds installer build and test to runtime.yml, consuming live artifacts. Preserves the old standalone installer build + test pipeline using cached bits for now, but if it breaks with #494 as expected I won't try to repair it with this PR.

Old WIP PR: #529.

/cc @trylek @safern @jashook

eng/pipelines/common/platform-matrix.yml

eng/pipelines/installer/jobs/base-job.yml

safern · 2019-12-09T21:48:19Z

eng/pipelines/installer/jobs/osx-build.yml

-  - ${{ if and(ne(variables['System.TeamProject'], 'public'), notin(variables['Build.Reason'], 'PullRequest')) }}:
-    - task: NuGetAuthenticate@0
+    buildVariables:
+      CommonMSBuildArgs: >-


Just to learn, what does: >- do?

I'm seeing this for the first time too...

> replaces newlines with spaces (vs. | which keeps newlines), - strips the newline at the end. http://yaml-multiline.info/ is a nice resource for this, and it has links to the spec.

Thanks, pretty useful 😄

eng/pipelines/libraries/build-job.yml

eng/pipelines/common/clone-checkout-bundle-step.yml

… minimal changes to the installer jobs. Fix paths to jobs. Fix typos Add platform to parameter lists. Pass platform to all jobs. Cleanup variable usages. Don't pass buildConfig down to installer jobs. They use matrix strategy instead. Fix variable usage in windows build. Fix windows platform names. Add missing platform parameter for Windows_NT_arm Update last condition re parameters.dockerImage. Rename dockerImage parameter to productBuildDockerImage to be more explicit. Use pool and container parameters from platform-matrix in installer build. Pass rootfs dir via crossrootfsDir to bash-build.yml. Fix passing down container in runtime-installer pipeline. Remove unused Linux_x64_raw config.

Includes refactoring the runtime legs to avoid duplicating artifact transfer logic and preserve the runtime standalone build.

$[] becomes empty string when a variable is undefined. $() sticks around literally if the variable doesn't exist, messing up the command. Unfortunately $[] doesn't seem to work in this context so we must use $() and define the variable as empty string.

dagood · 2019-12-10T17:24:31Z

In the last CI run, I somehow got AzDO to give me a good runtime pipeline build of Installer using live bits. All the platforms ran but macOS, which was just stalling due to the ongoing AzDO issues.

(The $[] vs. $() issue I fixed caused some benign error messages in the log, no failure.)

@jkoritzinsky @ViktorHofer @safern @trylek PTAL

eng/pipelines/common/variables.yml

dagood · 2019-12-10T22:50:48Z

CI is all green except runtime-coreclr (CoreCLR Pri0 Test Run R2R Windows_NT x86 checked), which timed out trying to send (get?) Helix results. I hit retry.

Would appreciate more reviews. 🙂

trylek

OMG, sorry for not signing off, your change looks awesome to me!

trylek · 2019-12-10T18:18:31Z

eng/pipelines/common/xplat-setup.yml

-
+        # Set the bash script to display each command, and stop if any command exits nonzero.
+        - name: setScriptToEchoAndFailOnNonZero
+          value: 'set -xe'


Don't we need separate Windows vs. Linux versions here?

We have them, it's just a no-op right now on Win a few lines up:

- name: setScriptToEchoAndFailOnNonZero value: ''

Don't know if it's possible to do this (or necessary) in batch.

trylek · 2019-12-10T18:21:22Z

eng/pipelines/installer/installer-matrix.yml

+    - Windows_NT_arm64
+    jobParameters:
+      ${{ insert }}: ${{ parameters }}
+      skipTests: true


I may be just insufficiently familiar with details of the installer pipeline but wouldn't it be possible to synthesize some of the jobParameters like skipTests or publicRidAgnosticPackages in windows-build so that we could use just one invocation of platform-matrix for all the platform combinations? Non-blocking, just asking.

It might be possible to make skipTests = crossBuild. Based on the current file, that seems correct.

trylek · 2019-12-10T18:25:40Z

eng/pipelines/installer/jobs/base-job.yml

@@ -0,0 +1,173 @@
+parameters:


Non-blocking observation that's moreover unrelated to your change as such. Amusingly enough we managed to put the common build-related templates into a different location for each repo - under "installer/jobs" vs. "libraries" vs. "coreclr/templates". Let's ignore this for now but once the changes settle down, I think some shuffles to make this more regular would be beneficial.

trylek · 2019-12-10T18:31:52Z

eng/pipelines/installer/jobs/base-job.yml

+    value: ''
+
+  - ${{ if ne(parameters.liveCoreClrBuildConfig, '') }}:
+    - name: liveCoreClrLegName


I originally chickened out of Santi's suggestion to put this logic in xplat-setup (and you're not using it, after all) but perhaps we could somehow extract this logic into a separate script to avoid open-coding it in multiple places. I don't yet have a completely clear idea on how exactly this should work and I don't think you should try to address it as part of this change but it might be worth a small design discussion.

I originally chickened out of Santi's suggestion to put this logic in xplat-setup (and you're not using it, after all)

I actually am using xplat-setup, since I use platform-matrix... or am I misunderstanding?

It makes sense to me to have it in xplat-setup, since the bulk of the transfer is setting up variables, and I think this is the only reasonable mechanism we have to share agent-local variable setup.

I originally chickened out of Santi's suggestion to put this logic in xplat-setup (and you're not using it, after all)

I actually am using xplat-setup, since I use platform-matrix... or am I misunderstanding?

We really need to update our docs on how these templates flow. I'll open a tracking issue for that.

The reasoning was to have all this *ArtifactName variables and LegName defined in xplat-setup so that all templates can benefit from it and just use them without having to actually define it in multiple places. Currently each partition defined this on it's base template.

However the change is not trivial as the right parameters need to be flown to the right templates in order to be able to access the values we need on xplat-setup. So this can be a follow up clean-up change.

I still have to do clean-up on libraries template which I left from my previous PR to unblock work, so I can put it as part of my change.

Please let's make sure we don't get deadlocked by these considerations for now - it's probably up to me to apologize for bringing them up in the first place. I'll be the happiest person in the world to clean this up but it's strictly ordered after standing up the official build and producing signing artifacts so please let's not block on that for now. I'll be happy to play with pipeline cleanups over / after the EOY holidays ;-).

jkoritzinsky · 2019-12-10T22:56:56Z

A lot of the Helix queues are very full (the Windows queue that PRs use currently has a queue depth of 2.3k work items), so we might want to wait for the queue to drain before re-running any more if it times out again.

jkoritzinsky · 2019-12-10T23:06:06Z

eng/pipelines/runtime.yml

+  parameters:
+    liveCoreClrBuildConfig: checked
+    liveLibrariesBuildConfig: Release
+    skipPlatforms:


I think we should invert this to instead specify the platforms we want to build for, not the platforms we want to skip. We should do this in a similar manner to the platform-matrix.yml file is done.

Anyway I think it makes sense to add a comment so that whenever we expand the full matrix for all partitions we don't forget adding any platform.

jkoritzinsky · 2019-12-10T23:07:36Z

src/installer/pkg/projects/netcoreapp/src/localnetcoreapp.override.targets

      <ReferenceCopyLocalPaths Remove="@(ReferenceCopyLocalPaths)" Condition="$([System.String]::Copy('%(FileName)').StartsWith('mscordaccore_'))" />
-      <ReferenceCopyLocalPaths Include="@(CoreCLRFiles)" />
+
+      <ReferenceCopyLocalPaths Include="@(CoreCLRFiles)" NuGetPackageId="$(MicrosoftNETCoreRuntimeCoreCLRPackage)" />


Why is the NuGetPackageId metadata required? Is that related to the support in Microsoft.DotNet.Build.Tasks.SharedFramework.Sdk for crossgenning the framework?

I believe it was a batch somewhere that assumed all items had NuGetPackageId defined. I wouldn't expect it to change any concrete results if the assumption were fixed instead, but this was easy and matches how we're adding this data to the CoreFX items.

jkoritzinsky · 2019-12-10T23:10:30Z

src/installer/pkg/projects/netcoreapp/src/localnetcoreapp.override.targets

+  </Target>
+
+  <!-- !!!BEGIN PATCHING Sharedfx SDK target -->
+  <Target Name="GetFilesFromPackageResolve">


What part of this target did you have to override/patch?

See below:

 <RidSpecificFilesToPackage Condition="'%(RidSpecificFilesToPackage.TargetPath)' == ''"> 

Although I guess I neglected mentioning the original: <RidSpecificFilesToPackage>.

The paths without the change should work as-is. Were they not working correctly?

In fact, the paths should have been the same as what they're overridden to here.

The cross-target paths get reset by that, and need to be preserved:

runtime/src/installer/pkg/projects/netcoreapp/src/localnetcoreapp.override.targets

Lines 47 to 49 in 4e23cd3

<CoreCLRCrossTargetFiles>

<TargetPath>runtimes/$(CoreCLRCrossTargetComponentDirName)_$(TargetArchitecture)/native</TargetPath>

</CoreCLRCrossTargetFiles>

I think they used to work by being added after GetFilesFromPackageResolve rather than before.

Good catch!

Well, I did have the benefit of seeing the build fail before I did this. 😄 When the target paths are the same between cross and target archs, the zip gets duplicate entries which fails an unzip step later.

dagood · 2019-12-10T23:45:26Z

I don't think any of the convos are blockers--I'd like to get this merged to unblock and work on them incrementally. I've filed #749 to track the issues. I'll file PRs for smaller ones really quick when I have gaps in my work on signing/publishing. Any objection (while that CoreCLR leg is finishing up)?

dagood requested review from jkoritzinsky and ViktorHofer December 9, 2019 19:52

dagood self-assigned this Dec 9, 2019

Dotnet-GitSync-Bot added the area-Infrastructure label Dec 9, 2019

This was referenced Dec 9, 2019

WIP: Add installer build/test to the runtime.yml pipeline #529

Closed

Local live-live builds #494

Merged

safern reviewed Dec 9, 2019

View reviewed changes

eng/pipelines/common/platform-matrix.yml Outdated Show resolved Hide resolved

safern reviewed Dec 9, 2019

View reviewed changes

eng/pipelines/installer/jobs/base-job.yml Show resolved Hide resolved

safern reviewed Dec 9, 2019

View reviewed changes

eng/pipelines/libraries/build-job.yml Show resolved Hide resolved

safern reviewed Dec 9, 2019

View reviewed changes

eng/pipelines/common/clone-checkout-bundle-step.yml Show resolved Hide resolved

jkoritzinsky and others added 7 commits December 9, 2019 17:49

Add support for clang 9 to corehost build.

8d642e6

Skip tests on Linux_musl_arm64

8be6ae3

runtime.yml installer build with live artifacts

625126b

Includes refactoring the runtime legs to avoid duplicating artifact transfer logic and preserve the runtime standalone build.

Fix installer overrides: accept live CI artifacts

9937344

Use clone-checkout-bundle-step.yml everywhere

1055154

Try using isOfficialBuild var from template expr

48ca424

dagood force-pushed the live-ci-installer branch from 0427f1a to 48ca424 Compare December 9, 2019 23:50

safern reviewed Dec 10, 2019

View reviewed changes

eng/pipelines/common/variables.yml Show resolved Hide resolved

trylek approved these changes Dec 10, 2019

View reviewed changes

jkoritzinsky reviewed Dec 10, 2019

View reviewed changes

safern approved these changes Dec 10, 2019

View reviewed changes

safern mentioned this pull request Dec 10, 2019

CI failing with Git checkout failed with exit code: 128 #736

Closed

dagood mentioned this pull request Dec 10, 2019

Clean up Installer build yaml after incorporation into runtime.yml #749

Open

8 tasks

dagood merged commit 6b78906 into dotnet:master Dec 11, 2019

dagood deleted the live-ci-installer branch December 11, 2019 00:35

dagood mentioned this pull request Jan 13, 2020

Use platform-matrix for Installer jobs #1623

Merged

ghost locked as resolved and limited conversation to collaborators Dec 11, 2020

	<CoreCLRCrossTargetFiles>
	<TargetPath>runtimes/$(CoreCLRCrossTargetComponentDirName)_$(TargetArchitecture)/native</TargetPath>
	</CoreCLRCrossTargetFiles>

Add installer build/test to the runtime.yml pipeline #705

Add installer build/test to the runtime.yml pipeline #705

Uh oh!

Conversation

dagood commented Dec 9, 2019

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

dagood commented Dec 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dagood commented Dec 10, 2019

Uh oh!

trylek 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!

Choose a reason for hiding this comment

Uh oh!

jkoritzinsky commented Dec 10, 2019

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dagood commented Dec 10, 2019

Uh oh!

Uh oh!

dagood commented Dec 10, 2019 •

edited

Loading