Live-live CI pipeline #294

trylek · 2019-11-26T15:58:52Z

I have rebased the change against Santiago's CoreFX YAML refactoring
which simplified the change substantially. I have received an almost green
run and my cursory validation confirmed that live artifacts are indeed being
used.

I hit trouble on Linux x64 that doesn't seem to support Python3, I'm currently
experimenting with downgrading the patch-corefx.py script to not explicitly
target Python3. As a form of instrumentation to simplify development I'm
still hijacking the PR YAML file under CoreCLR for the "live-live pipeline", that's
not to be merged in, otherwise I believe the change to be mostly ready for
review.

I have slightly unified naming of the libraries jobs with the coreclr jobs. There's
a slight wrinkle in CoreCLR supporting three configurations (debug / checked
/ release) wherease CoreFX only supports debug / release. In this change
I have fixed CoreFX YAML scripts to also pretend to support the three configs,
just internally remapping "checked" to "debug" in base-job.yml.

[After all, in the live-live mode, there's technically a difference between debug
and checked version of CoreFX w.r.t. consuming the respective version of
CoreLib. I guess this logic is up to further development to support various
cross-building scenarios, e.g. debug CoreFX builds consuming release CoreLib.
I haven't tried to address these scenarios in my change but they should be
pretty straightforward to implement on top of it.]

As I assume that most people are on the Thanksgiving prolonged weekend,
I continue developing the change as I doubt I'll get it sufficiently reviewed
before next Monday. In particular, I have started working on incorporating
CoreFX testing. If any reviewers happen to be available (if I start receiving
review feedback), I'll fix the instrumentation by moving the YAML file and
I'll finalize the change so that it can be merged in.

Thanks

Tomas

trylek · 2019-11-29T16:42:40Z

I'm also not clear on what I've broken in the "coreclr-libraries" pipeline so if anyone can help shed light on that, I would be super grateful.

trylek · 2019-11-30T17:07:39Z

Other than that, I must admit I find it almost impossible to conclusively prove that we're using the live-live artifacts everywhere with so much nuget downloading going on everywhere around; for now I tend to think this needs to be a collective effort after merging in the initial implementation of the live-live CI pipeline involving scrutiny by experts on build logic for the individual subrepos.

trylek · 2019-11-30T18:13:45Z

I'm also trying to figure out how to incorporate core_setup aka installer testing in the pipeline but I know very little about what actually happens there - I tried to run the "installer" script locally and it seems to have Crossgened the framework and then crashed with some nuget download bug - so I'm in doubt whether I should follow up on that with help from the relevant experts or just merge in a limited version of the change without the installer chores and leave those to people more familiar with this logic.

jaredpar · 2019-12-02T15:20:29Z

I hit trouble on Linux x64 that doesn't seem to support Python3,

@jkoritzinsky where do we document our software requirements around tools like Python for Linux? The Linux requirements document doesn't specify this in the same way that the Windows one does.

trylek · 2019-12-02T15:20:37Z

According to my best knowledge, this commit should end up with a green runtime-libraries and runtime-coreclr (at this moment still representing my initial implementation of the live-live pipeline). Remaining issues I'm aware of:

Where exactly to put the remapping layer between 3 CoreCLR configs (debug / checked / release) vs. 2 CoreFX configs (Debug / Release)? Today I'm doing this in several places in base-job, build-job, run-test-job and the CoreCLR build-test.sh and cmd scripts. I'm not sure if this is cleanly solvable right now but we should discuss the ideal longer-term state.
I had to open-code some of the variables in eng/pipelines/libraries/variables.yml (sourcesRoot, _buildScriptFileName) - as according to my discussion with Jared and Chris Costa from several months back there's no way to include the variable template "in the middle" of the template tree, just at the root (i.e. in "pipeline" scripts like pr.yml), there's no easy way to import it into the live-live pipeline spanning multiple subrepos.
I think I have managed to stand up the CoreFX test build and execution within the initial live-live pipeline implementation, for now I have abandoned the attempt to include the installer jobs in the pipeline as I'm simply not yet sufficiently familiar with them so that I would probably end up producing some unusable nonsense anyway.
Physically moving the live-live YAML file from its temporary location under eng/pipeline/coreclr/pr.yml (which is most convenient for developing the PR) to its final location; according to initial suggestion from Viktor is should probably become something like eng/pipelines/runtime.yml.

Thanks

Tomas

jaredpar · 2019-12-02T15:22:24Z

eng/pipelines/libraries/build-job.yml

@@ -5,6 +5,7 @@ parameters:
  archType: ''
  framework: netcoreapp
  isOfficialBuild: false
+  useLiveCoreClr: false


Going forward when should this ever be false and what would the impact of that be?

In this change I haven't yet dropped the support for both "live-live runs" and "partial runs" picking up some artifacts from external locations. I can easily imagine this limitation may be temporary but, in its current form, dropping this condition would invalidate the "legacy" runtime-libraries pipeline. I assume it's up to further discussion in our meetings how exactly to sequence the transformation.

jashook · 2019-12-02T16:33:29Z

jashook

This is an overview feedback. This change looks like it is removing the idea of specific pipelines per area path. Either that or it is modifying coreclr/pr.yml into a root pr.yml. Either way I think we should change the direction to do the following:

Create a new root pr.yml without modifying coreclr/py.yml
- The contents of this pr.yml should be similar to what is currently in this pr. I will make suggestions on what to remove assuming the pr.yml changed in this pr will be the new pr.yml for the root pipeline
Undo the changes to coreclr/pr.yml so that we will not lose coverage. If there is work to be done that is outside of the scope of what has been done already remove/comment out the bits that do not work such that we have minimum coverage detailed Live-live CI pipeline #294 (comment)
Installer will need to have work in its CI, which I expect to be less than corefx and coreclr. This should be done in a subsequent PR.

jashook · 2019-12-02T16:50:08Z

I will continue a deeper review assuming that the coreclr/pr.yml will be essentially copied to become root/pr.yml

jashook

The rest looks good. The overview review sounds a bit daunting at first; however, the framework that you guys have built should make it I think pretty smooth to transition.

Thanks for all the great work.

eng/pipelines/coreclr/pr.yml

jashook · 2019-12-02T16:56:20Z

src/coreclr/build-test.cmd

@@ -577,8 +581,10 @@ REM ============================================================================

 if NOT "%__LocalCoreFXPath%"=="" (
    echo Patch CoreFX from %__LocalCoreFXPath%
+    set __CoreFXBuildType=Debug


This should be the opposite, assume it is release. If the build type is debug then use a debug corefx. That way we can correctly use a release corefx build with a checked runtime.

Fixed, thanks for pointing that out. Ultimately we may want to introduce options for [semi-]arbitrarily combining configuration modes for the individual components (CoreCLR, CoreFX, tests in either subrepos - not sure what degrees of freedom we have) but I think it will be easier to address in a follow-up change.

jashook · 2019-12-02T16:56:33Z

src/coreclr/build-test.sh

@@ -136,7 +136,12 @@ patch_corefx_libraries()
 {
    echo "${__MsgPrefix}Patching CORE_ROOT: '${CORE_ROOT}' with CoreFX libaries from enlistment '${__LocalCoreFXPath}"

-    patchCoreFXArguments=("-clr_core_root" "${CORE_ROOT}" "-fx_root" "${__LocalCoreFXPath}" "-arch" "${__BuildArch}" "-build_type" "${__BuildType}")
+    __CoreFXBuildType=Debug
+    if [ "$__BuildType" == "release" ]; then


Same comment as the windows file.

jashook · 2019-12-02T17:27:24Z

CI succeeded excellent work!

safern · 2019-12-02T17:36:06Z

eng/pipelines/libraries/base-job.yml

+        - buildScriptFileName: libraries
+        - _BuildConfig: Debug
+        - ${{ if eq(parameters.buildConfig, 'release') }}:
+          - _BuildConfig: Release


Should this be part of xplat-setup.yml?

Btw is comparison here case-sensitive? because in libraries/.azure-ci.yml we pass it down as upper-case.

safern · 2019-12-02T17:37:10Z

eng/pipelines/libraries/base-job.yml

        # Windows variables
        - ${{ if eq(parameters.osGroup, 'Windows_NT') }}:
          - _msbuildCommand: powershell -ExecutionPolicy ByPass -NoProfile eng\common\msbuild.ps1 -warnaserror:0 -ci
          - _runtimeOSArg: /p:RuntimeOS=win10
+          - _archiveExtension: '.zip'


Why do we need this? This should already be defined in xplat-setup.yml

Ahh, you're right, this should be no longer necessary with all your refactorings, will remove, thank you!

safern · 2019-12-02T17:40:26Z

eng/pipelines/libraries/build-job.yml

@@ -37,11 +44,13 @@ jobs:
          - EMSDK_PATH: $(Build.BinariesDirectory)/emsdk
        - ${{ if eq(parameters.runTests, true) }}:
          - _buildAction: -build -buildtests /p:ArchiveTests=true
-          - ${{ if ne(parameters.framework, 'allConfigurations') }}:
+          - ${{ if and(ne(parameters.framework, 'allConfigurations'), ne(parameters.useLiveCoreClr, true)) }}:


Why do we want to skip restoring the tests when useLiveCoreClr is not being used?

I'll double-check this, I may have confused this with the other property to not restore the CoreCLR framework (as we're using the live one).

Yeah. That property is to avoid restoring libraries test-projects. AllConfigurations is a special framework which doesn't build libraries test projects at all (it builds custom package tests) so we don't need to restore test projects as part of its restore step.

These complementary properties should let us drop all automatic transformations of CoreCLR vs. library build modes and thus substantially simplify the live-live pipeline and make it more general at the same time. I have reverted the changes to the build-test scripts that were taking care of this logic on the local build side as well as the change to eng/pipelines/libraries/base-job and bits of the other YAML files on the CI side. Thanks Tomas

safern · 2019-12-03T19:21:05Z

eng/pipelines/coreclr/templates/build-test-job.yml

+          cleanUnpackFolder: false
+          artifactFileName: '$(librariesArtifactName)$(archiveExtension)'
+          artifactName: '$(librariesArtifactName)'
+          displayName: 'live-built CoreFX'


CoreFX -> Libraries.

safern · 2019-12-03T19:22:07Z

eng/pipelines/coreclr/templates/build-test-job.yml

+      dependsOn:
+      - ${{ format('coreclr_product_build_{0}{1}_{2}_{3}', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
+      - ${{ if ne(parameters.liveLibrariesBuildConfig, '') }}:
+        - ${{ format('libraries_build_{0}_{1}{2}_{3}_{4}', 'netcoreapp', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.liveLibrariesBuildConfig) }}


It might make sense to remove netcoreapp from the libraries build names when target framework is netcoreapp as libraries is the only subset that builds for other frameworks. That way we don't need to hard code it here. Something to consider on a follow up PR.

safern · 2019-12-03T19:49:00Z

eng/pipelines/runtime.yml

+    - Windows_NT_arm64
+    jobParameters:
+      isOfficialBuild: false
+      testScope: empty


No need to pass this down here. The default is already '' . This is just used when running the tests as part of the build-job.

safern · 2019-12-03T20:45:16Z

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

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


It might make sense to move these artifactName variables to xplat-setup template as it is used to upload and download and we already have all the required parameters at that point. Anyway, that can be done in a follow up PR, just calling it out so that we don't forget.

safern

LGTM from the yml changes. I don't have much context on the coreclr test scripts, but looks good to my small knowledge of it.

Feedback addressed.

trylek added the area-Infrastructure label Nov 26, 2019

trylek force-pushed the CombinedBuild2 branch 4 times, most recently from 8e240f9 to 56e396e Compare November 29, 2019 16:18

trylek requested review from safern, jashook and ViktorHofer November 29, 2019 16:37

trylek changed the title ~~WIP: live-live CI (2nd attempt)~~ Live-live CI pipeline Nov 29, 2019

trylek requested a review from jkoritzinsky November 29, 2019 16:39

trylek force-pushed the CombinedBuild2 branch from 7425f14 to b43c421 Compare November 29, 2019 17:23

trylek requested review from jaredpar, stephentoub and dagood November 30, 2019 17:12

trylek force-pushed the CombinedBuild2 branch 2 times, most recently from e5e78e6 to 9701d36 Compare December 1, 2019 22:20

jaredpar reviewed Dec 2, 2019

View reviewed changes

jashook previously requested changes Dec 2, 2019

View reviewed changes

jashook reviewed Dec 2, 2019

View reviewed changes

safern reviewed Dec 2, 2019

View reviewed changes

trylek added 10 commits December 3, 2019 19:26

Bump up msbuild diagnostics for the sendtohelix script

7284d22

Fix configuration ID in calls to helix.yml

daa935d

Remove instrumentation; hopefully fix the coreclr-libraries pipeline

052313e

Addressing most of PR feedback

3b23c9a

Fix unification of archiveExtension

58f8c52

Santiago's feedback - rename coreFx to libraries in the new code

b8d1264

Rename root.yml to runtime.yml per PR feedback

91ce5a4

Add build-test argument to specify CoreFX configuration

b1bf69b

Remove msbuild verbosity instrumentation; fix YAML typo

5ec9607

trylek force-pushed the CombinedBuild2 branch 5 times, most recently from 993d487 to 12f30fa Compare December 3, 2019 18:42

Address PR feedback

b668b57

trylek force-pushed the CombinedBuild2 branch from 12f30fa to b668b57 Compare December 3, 2019 18:44

safern reviewed Dec 3, 2019

View reviewed changes

Latest bits of PR feedback and disabling test runs for now

7674b40

safern reviewed Dec 3, 2019

View reviewed changes

Stop passing testScope: empty to libraries build

7402ce7

safern reviewed Dec 3, 2019

View reviewed changes

safern approved these changes Dec 3, 2019

View reviewed changes

jashook approved these changes Dec 3, 2019

View reviewed changes

Sigh - one last bit, propagation of condition wasn't working properly

28a52c4

trylek merged commit 8bdcba1 into dotnet:master Dec 3, 2019

trylek deleted the CombinedBuild2 branch December 3, 2019 22:40

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Live-live CI pipeline #294

Live-live CI pipeline #294

trylek commented Nov 26, 2019 •

edited

Loading

trylek commented Nov 29, 2019

trylek commented Nov 30, 2019

trylek commented Nov 30, 2019

jaredpar commented Dec 2, 2019

trylek commented Dec 2, 2019

jaredpar Dec 2, 2019

trylek Dec 2, 2019

jashook commented Dec 2, 2019 •

edited

Loading

jashook left a comment •

edited

Loading

jashook commented Dec 2, 2019

jashook left a comment

jashook Dec 2, 2019

trylek Dec 2, 2019

jashook Dec 2, 2019

trylek Dec 2, 2019

jashook commented Dec 2, 2019

safern Dec 2, 2019

safern Dec 2, 2019

safern Dec 2, 2019

trylek Dec 2, 2019

safern Dec 2, 2019

trylek Dec 2, 2019

safern Dec 2, 2019

safern Dec 3, 2019

safern Dec 3, 2019

safern Dec 3, 2019

safern Dec 3, 2019

safern left a comment

Live-live CI pipeline #294

Live-live CI pipeline #294

Conversation

trylek commented Nov 26, 2019 • edited Loading

trylek commented Nov 29, 2019

trylek commented Nov 30, 2019

trylek commented Nov 30, 2019

jaredpar commented Dec 2, 2019

trylek commented Dec 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jashook commented Dec 2, 2019 • edited Loading

jashook left a comment • edited Loading

Choose a reason for hiding this comment

jashook commented Dec 2, 2019

jashook left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jashook commented Dec 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

safern left a comment

Choose a reason for hiding this comment

trylek commented Nov 26, 2019 •

edited

Loading

jashook commented Dec 2, 2019 •

edited

Loading

jashook left a comment •

edited

Loading