Switch over CoreCLR pr.yml to use live-built libraries #520

trylek · 2019-12-04T20:03:52Z

No description provided.

safern · 2019-12-04T22:20:43Z

FYI: @BruceForstall this will remove the CoreFX stress legs running against a checked CoreCLR and we will need to bring those back later once all pipeline changes are settled.

safern · 2019-12-04T22:21:57Z

eng/pipelines/coreclr/pr.yml

+- template: /eng/pipelines/common/platform-matrix.yml
+  parameters:
+    jobTemplate: /eng/pipelines/libraries/build-job.yml
+    buildConfig: Debug


Does it really matter having Debug vs Release builds of the libraries product? The API surface area doesn't change at all.

I don't fully know what it is used on CoreCLR for, but it might not be needed to have Debug builds as well.

I think we should push to always use a release build of libraries in coreclr, even for a debug build of coreclr. If plumbing this through is more work at the moment then we can work on this after the change goes in.

I don't think it shouldn't be much work. It is just a matter of including this platforms on the Release template expansion and passing down the correct parameter for liveLibrariesBuildConfig set to Release.

Actually there's already a Release builds section for libraries which is building these two platforms, so we should just update all coreclr build that depend on libraries to use Release bits.

Thanks for pointing that out, fixed.

BruceForstall · 2019-12-04T23:08:26Z

FYI: @BruceForstall this will remove the CoreFX stress legs running against a checked CoreCLR and we will need to bring those back later once all pipeline changes are settled.

Thanks for the heads-up. I knew this was coming.

jashook · 2019-12-05T00:01:10Z

ci.yml will need the same changes correct?

jashook · 2019-12-05T00:02:01Z

eng/pipelines/coreclr/pr.yml

+- template: /eng/pipelines/common/platform-matrix.yml
+  parameters:
+    jobTemplate: /eng/pipelines/libraries/build-job.yml
+    buildConfig: Debug


I think we should push to always use a release build of libraries in coreclr, even for a debug build of coreclr. If plumbing this through is more work at the moment then we can work on this after the change goes in.

jashook · 2019-12-05T00:03:45Z

I also assume all the other stress pipelines will need to be modified to be live live as well.

jashook · 2019-12-05T00:04:07Z

If that is the case ci.yml is the highest priority, the others are not blocking.

safern

One comment, otherwise, LGTM.

Also, yes we should update ci.yml as part of this change or a follow-up change.

safern · 2019-12-05T17:43:11Z

eng/pipelines/coreclr/templates/xplat-pipeline-job.yml


    - ${{ if ne(parameters.liveLibrariesBuildConfig, '') }}:
-      - librariesArtifactName: ${{ format('libraries_bin_{0}_{1}{2}_{3}_{4}', 'netcoreapp', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}
+      - librariesArtifactName: ${{ format('libraries_bin_netcoreapp_{0}_{1}_{2}', parameters.osGroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}


Why are we removing the osSubgroup? wouldn't this break the linux_musl libraries builds?

Oh sorry. I see this is just in coreclr's consumption, but this might break if you guys start depending on an artifact that contains ossubgroup like musl.

Hmm, I was under the impression you yourself told me the other day that netcoreapp artifacts don't include OS subgroup information.

safern · 2019-12-05T18:43:17Z

eng/pipelines/common/xplat-setup.yml

    osSubgroup: ${{ parameters.osSubgroup }}
    archType:  ${{ parameters.archType }}

+    librariesBuildArtifactName: ${{ format('libraries_bin_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}


Oopps. This is not a variable, this is defined as a parameter. It seems like you want it under the variables section.

Thanks for doing the clean-up btw.

Right you are, thanks for spotting, hopefully fixed now.

Hmm, it looks like you actually sent me down a pretty nasty rathole. I'm right now trying to figure out how to consolidate the movement of buildConfig to xplat-setup with the liveLibrariesBuildConfig parameter but the hope to get this merged in today just keeps fading away.

Long story short, having an independent configuration pair for CoreCLR and CoreFX is not an ideal start for a common xplat-setup script used by both.

For now I'm inclined to roll back all the "library artifact cleanup changes" as I see no other way how to fix the pr.yml script.

sounds good to me and sorry about that. We can do that clean-up later.

Sorry for sending you that path, didn't think it was going to be that hard.

No worries, it also came as a surprise to me. But yes, I think it's better to postpone the cleanup after things have settled down, we're still pretty much shooting at a moving target right now.

This is needed so that the library name artifacts are properly synthesized. This extra delta is somewhat noisy but hopefully trivial. Thanks Tomas

Closes dotnet#520

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

trylek force-pushed the LiveLiveCoreCLR branch from d254921 to 90dc300 Compare December 4, 2019 20:07

trylek changed the title ~~WIP: Switch over CoreCLR pr.yml to use live-built libraries~~ Switch over CoreCLR pr.yml to use live-built libraries Dec 4, 2019

trylek requested review from jashook and safern December 4, 2019 22:19

safern reviewed Dec 4, 2019

View reviewed changes

jashook approved these changes Dec 5, 2019

View reviewed changes

safern approved these changes Dec 5, 2019

View reviewed changes

safern reviewed Dec 5, 2019

View reviewed changes

safern approved these changes Dec 5, 2019

View reviewed changes

trylek added 6 commits December 5, 2019 22:22

Switch over CoreCLR pr.yml to use live-built libraries

9acdbc7

Delete CoreFX runs against CoreCLR

62c1ab0

PR feedback - only use release CoreFX builds for CoreCLR testing

b4e49a6

Remove OSSubgroup from CoreFX build artifact name

3c028d7

Clean up propagation of library artifacts per Santi's feedback

7c934fd

Fix typo in placement of the artifact names (thanks Santi for spotting!)

f84dccd

trylek force-pushed the LiveLiveCoreCLR branch 2 times, most recently from 7ef06b7 to 9cf0389 Compare December 5, 2019 21:27

Make xplat-setup aware of buildConfig

a6e6b28

This is needed so that the library name artifacts are properly synthesized. This extra delta is somewhat noisy but hopefully trivial. Thanks Tomas

trylek force-pushed the LiveLiveCoreCLR branch from 9cf0389 to a6e6b28 Compare December 5, 2019 21:37

Rollback cleanup of library artifact naming as it doesn't work

9a5f05a

trylek force-pushed the LiveLiveCoreCLR branch from 58bcfaa to 9a5f05a Compare December 5, 2019 23:35

trylek merged commit f621f44 into dotnet:master Dec 6, 2019

trylek deleted the LiveLiveCoreCLR branch December 6, 2019 15:56

trylek mentioned this pull request Jan 9, 2020

Relax some asserts in SpanHelpers.T.cs #1472

Closed

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

MichalStrehovsky pushed a commit to MichalStrehovsky/runtime that referenced this pull request Mar 25, 2021

Delete CommonRuntimeTypes (dotnet#616)

99201cb

Closes dotnet#520

Switch over CoreCLR pr.yml to use live-built libraries #520

Switch over CoreCLR pr.yml to use live-built libraries #520

Uh oh!

Conversation

trylek commented Dec 4, 2019

Uh oh!

safern commented Dec 4, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

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!

BruceForstall commented Dec 4, 2019

Uh oh!

jashook commented Dec 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jashook commented Dec 5, 2019

Uh oh!

jashook commented Dec 5, 2019

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

safern Dec 5, 2019 •

edited

Loading