Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Move ARM64 Windows boxen to be Helix-provisioned #20204

Merged
merged 1 commit into from
Oct 9, 2018
Merged

Conversation

MattGal
Copy link
Member

@MattGal MattGal commented Oct 1, 2018

@MattGal MattGal requested a review from jashook October 1, 2018 21:25
@jashook jashook requested a review from BruceForstall October 1, 2018 21:26
@@ -762,7 +762,7 @@ def static setMachineAffinity(def job, def os, def architecture, def options = n
label('Windows.10.Amd64.ClientRS4.DevEx.Open')
}
} else {
Utilities.setMachineAffinity(job, os, 'arm64-windows_nt')
Utilities.setMachineAffinity(job, 'windows.10.arm64.open')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will work. I think you need:

job.with {
    label('Windows.10.Arm64.Open')
}

Is there a "correct" casing (upper/lower)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The queue API is case insensitive, but I think we're trending towards all lower case. Use whatever you like here, case insensitivity (since Service Bus Messaging queues are) is here to stay until we change our queuing technology.
  • I believe the job.with format though is assuming that the build will occur on the ARM machine, not just a test run as this was being used before. I think the setMachineAffinity is still the right way to go, but I defer to @mmitche .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casing shouldn't matter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The job.with format is identical to the setMachineAffinity format underneath.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I see. there's something a little too tricky here, because with the "os" argument, it looks up in the agent table, but without it, it uses it directly.

@MattGal
Copy link
Member Author

MattGal commented Oct 1, 2018

@BruceForstall nothing remaining involves Windows ARM, should I merge?

@jashook
Copy link

jashook commented Oct 1, 2018

Quick question before this goes in, do these machines have git and java installed?

@MattGal
Copy link
Member Author

MattGal commented Oct 2, 2018

@jashook yes, looks like there's 4 machines. Checking them out, I see:

  • JRE Version 1.8.0_181_b13 (x86 of course)
  • OS version: RTM version of 10.0.17133.1, activated

@jashook
Copy link

jashook commented Oct 2, 2018

Upon further thought I realized that these machine will also need to have native smarty installed before this change will work.

Copy link

@jashook jashook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Machines will need further setup marking as questing changes for now.

@MattGal
Copy link
Member Author

MattGal commented Oct 2, 2018

Smarty is a very CoreCLR-repository-specific consideration, and to the best of my understanding is not in a traditional sense installed on the machine (i.e. via MSI/MSU/exe installer) and could instead just be XCopy-style deployed, for instance brought down as part of the build process.

Since the queue here is not CoreCLR specific, it makes good sense to not fill the machine with CoreCLR-specific stuff.

@jashook
Copy link

jashook commented Oct 2, 2018

Either way this change is blocked on either one way or the other.

@jashook
Copy link

jashook commented Oct 8, 2018

#20227 is merged which brings us closer to getting of smarty. Opened #20301 to officially remove the smarty dependency in the CI. Once that is merged this can be safely merged as well.

Copy link

@jashook jashook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smarty dependency is now gone.

@jashook jashook merged commit 8502d4d into master Oct 9, 2018
@jashook jashook deleted the arm64-windows-ci branch October 9, 2018 21:18
@jashook
Copy link

jashook commented Oct 9, 2018

I am seeing (pending—There are no nodes with the label ‘windows.10.arm64.open’) /cc @MattGal @mmitche

@MattGal
Copy link
Member Author

MattGal commented Oct 10, 2018

apologies for the delay, looking now.

@MattGal
Copy link
Member Author

MattGal commented Oct 10, 2018

Should be fixed now. What happened was thatt https://helix.dot.net/api/2018-03-14/info/queues/Windows.10.Arm64.Open was returning OS group "unknown", and Jenkins was sending unix-style environment variables to the machines. Give it 10-20 minutes for all the machines to get in use.

@MattGal
Copy link
Member Author

MattGal commented Oct 10, 2018

Looks like the work is getting picked up but that the machines expected to have git on the path. Filed https://github.com/dotnet/core-eng/issues/4365 to track this, @ilyas1974 will make sure this gets done.

@sandreenko
Copy link

It looks like this change has broken arm32/64 windows jobs:
they fail with C:\j\workspace\arm_cross_che---c874ca48\Tools\dotnetcli\sdk\2.1.301\Microsoft.Common.CurrentVersion.targets(1179,5): error MSB3644: The reference assemblies for framework ".NETFramework,Version=v4.6" were not found. To resolve this, install the SDK or Targeting Pack for this framework version or retarget your application to a version of the framework for which you have the SDK or Targeting Pack installed. Note that assemblies will be resolved from the Global Assembly Cache (GAC) and will be used in place of reference assemblies. Therefore your assembly may not be correctly targeted for the framework you intend. [C:\j\workspace\arm_cross_che---c874ca48\packages\microsoft.dotnet.buildtools\3.0.0-preview1-03309-01\lib\tool-runtime\project.csproj]

An example https://ci.dot.net/job/dotnet_coreclr/job/master/job/arm_cross_checked_windows_nt_innerloop_tst_prtest/1771/consoleFull#-195827467479494335-f7bd-47d0-8771-8661e00c2db2

@MattGal
Copy link
Member Author

MattGal commented Oct 11, 2018

@sandreenko to give some context, these are newly set up machines because the previous ones used were using a "time-bombed" version of Windows, which had expired and was causing them to not work.

As such, we may need to modify the configuration of these machines further to support the scenario. Right now, they basically just have Java 8, git (x86 version) and Python. There isn't, as far as I know, support for installing either .NET 4.6 or the targeting packs for it on an ARM64 Windows machine, so I'm curious how this worked before.

I understand the urgent need for this work from discussions with @RussKeldorph. If either of you can provide any guidance about things that should be generally available on the machines please let me know (feel free to just directly open issues in dotnet/core-eng and tag myself in them, I'll handle the rest)

@jashook
Copy link

jashook commented Oct 11, 2018

The machine setup seems correct. Thanks @MattGal and @ilyas1974. I believe the issue is in init-tools.cmd. We should not be using the desktop build tools on these machines.

@MattGal
Copy link
Member Author

MattGal commented Oct 11, 2018

whew, that's what I was hoping for.

@jashook
Copy link

jashook commented Oct 11, 2018

#20370 opened to address the issue.

echesakov added a commit that referenced this pull request Oct 24, 2018
…lix queue

Port (partially) the following changes:

*  Build xunit wrappers the same way on windows and unix (#18695)
*  Update vc-runtime package to support ARM and ARM64 with current builds (#19265)
*  build-test - fix TestWrapper CS warnings (#19180)
*  Clean up build.cmd/build-test.cmd/runtest.cmd (#19291)
*  Use runtest.py to run tests for all platforms (#19213)
*  Updating runtest.py so that it works with Python 3 (#19768)
*  Fix build-test.sh wrapper build (#19779)
*  Move ARM64 Windows boxen to be Helix-provisioned (#20204)
*  Runtest.py on Windows Arm(64) (#20227)
*  Use runtest.cmd for arm(64) windows testing (#20301)
*  Do not restore or initialize buildtools for x86/arm64 (#20370)
*  Add hack for arm64/x86 to skip build tools restore (#20390)

Update Xunit to 2.4.1 prerelease version

Use the latest version (dbf0bf1) of tests/runtest.py
echesakov added a commit that referenced this pull request Oct 24, 2018
…lix queue

Port (partially) the following changes:

*  Build xunit wrappers the same way on windows and unix (#18695)
*  Work around cmd command length limit in xunit Exec task (#19095)
*  Update vc-runtime package to support ARM and ARM64 with current builds (#19265)
*  build-test - fix TestWrapper CS warnings (#19180)
*  Clean up build.cmd/build-test.cmd/runtest.cmd (#19291)
*  Use runtest.py to run tests for all platforms (#19213)
*  Updating runtest.py so that it works with Python 3 (#19768)
*  Fix build-test.sh wrapper build (#19779)
*  Move ARM64 Windows boxen to be Helix-provisioned (#20204)
*  Runtest.py on Windows Arm(64) (#20227)
*  Use runtest.cmd for arm(64) windows testing (#20301)
*  Do not restore or initialize buildtools for x86/arm64 (#20370)
*  Add hack for arm64/x86 to skip build tools restore (#20390)

Update Xunit to 2.4.1 prerelease version

Use the latest version (dbf0bf1) of tests/runtest.py
echesakov added a commit that referenced this pull request Oct 25, 2018
…lix queue

Port (partially) the following changes:

*  Build xunit wrappers the same way on windows and unix (#18695)
*  Work around cmd command length limit in xunit Exec task (#19095)
*  Update vc-runtime package to support ARM and ARM64 with current builds (#19265)
*  build-test - fix TestWrapper CS warnings (#19180)
*  Clean up build.cmd/build-test.cmd/runtest.cmd (#19291)
*  Use runtest.py to run tests for all platforms (#19213)
*  Updating runtest.py so that it works with Python 3 (#19768)
*  Fix build-test.sh wrapper build (#19779)
*  Move ARM64 Windows boxen to be Helix-provisioned (#20204)
*  Runtest.py on Windows Arm(64) (#20227)
*  Use runtest.cmd for arm(64) windows testing (#20301)
*  Do not restore or initialize buildtools for x86/arm64 (#20370)
*  Add hack for arm64/x86 to skip build tools restore (#20390)

Update Xunit to 2.4.1 prerelease version

Use the latest version (dbf0bf1) of tests/runtest.py
echesakov added a commit that referenced this pull request Oct 26, 2018
…lix queue

Port (partially) the following changes:

*  Build xunit wrappers the same way on windows and unix (#18695)
*  Work around cmd command length limit in xunit Exec task (#19095)
*  Update vc-runtime package to support ARM and ARM64 with current builds (#19265)
*  build-test - fix TestWrapper CS warnings (#19180)
*  Clean up build.cmd/build-test.cmd/runtest.cmd (#19291)
*  Use runtest.py to run tests for all platforms (#19213)
*  Updating runtest.py so that it works with Python 3 (#19768)
*  Fix build-test.sh wrapper build (#19779)
*  Move ARM64 Windows boxen to be Helix-provisioned (#20204)
*  Runtest.py on Windows Arm(64) (#20227)
*  Use runtest.cmd for arm(64) windows testing (#20301)
*  Do not restore or initialize buildtools for x86/arm64 (#20370)
*  Add hack for arm64/x86 to skip build tools restore (#20390)

Update Xunit to 2.4.1 prerelease version

Use the latest version (dbf0bf1) of tests/runtest.py
A-And pushed a commit to A-And/coreclr that referenced this pull request Nov 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants