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

Fix the package restore for arm64 and arm32 #12532

Merged
merged 1 commit into from Jun 29, 2017

Conversation

jashook
Copy link

@jashook jashook commented Jun 28, 2017

/cc @sdmaclea I am expecting this to fix the Arm64 Windows jobs.

@jashook
Copy link
Author

jashook commented Jun 28, 2017

@dotnet-bot test Windows_NT Arm64

@jashook
Copy link
Author

jashook commented Jun 28, 2017

/cc @BruceForstall

@@ -31,7 +31,19 @@
<PropertyGroup>
</PropertyGroup>
</When>
<When Condition="'$(OSGroup)'=='Windows_NT'">
<When Condition="'$(OSGroup)'=='Windows_NT' AND '$(__BuildArch)'=='arm64'">
Copy link
Member

Choose a reason for hiding this comment

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

Can you not use an OR here (arm64 OR arm)?

@wtgodbe
Copy link
Member

wtgodbe commented Jun 28, 2017

LGTM, other than the 1 comment. Guessing this got package restore working locally?

@jashook
Copy link
Author

jashook commented Jun 28, 2017

Correct managed to get the restore working locally. I will work on the OR change.

@sdmaclea
Copy link

@jashook Thanks

<TestNugetRuntimeId>win-$(__BuildArch)</TestNugetRuntimeId>
</PropertyGroup>
</When>
<When Condition="'$(OSGroup)'=='Windows_NT' AND '$(__BuildArch)'!='$(arm64)' AND '$(__BuildArch)'!='$(arm)'">
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be:

<When Condition="'$(OSGroup)'=='Windows_NT' AND '$(__BuildArch)'!='arm64' AND '$(__BuildArch)'!='arm'">

?

Copy link
Author

Choose a reason for hiding this comment

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

Yes that is a typo thank you for catching that

@BruceForstall
Copy link
Member

@wtgodbe , others: is there a reason we can't just name the arm/arm64 packages (or whatever this is naming) using the same naming scheme as x86/x64?

@jashook jashook force-pushed the fix_arm64_and_arm32_package_restore branch from 966cd34 to 987c2fc Compare June 28, 2017 21:31
@jashook
Copy link
Author

jashook commented Jun 28, 2017

@dotnet-bot test Windows_NT Arm64

@wtgodbe
Copy link
Member

wtgodbe commented Jun 28, 2017

@BruceForstall we only publish portable packages now, which have the prefix win-{arch}. So actually, @jashook, as part of this change it might be good to just change all testNugetRuntimeID's to win-{arch} for Windows, as today in CI we're probably downloading old win7-{arch} packages for x64 & x86.

@sdmaclea
Copy link

@BruceForstall I think part of the problem is the absence of arm64 linux nuget packages. #12372 And dotnet/corefx#21236 should fix that.

@jashook
Copy link
Author

jashook commented Jun 28, 2017

@dotnet-bot test Windows_NT Arm64 Checked

1 similar comment
@jashook
Copy link
Author

jashook commented Jun 29, 2017

@dotnet-bot test Windows_NT Arm64 Checked

@jashook jashook force-pushed the fix_arm64_and_arm32_package_restore branch from 987c2fc to 0489dac Compare June 29, 2017 16:12
@jashook
Copy link
Author

jashook commented Jun 29, 2017

@dotnet-bot test Windows_NT Arm64 Checked

@jashook
Copy link
Author

jashook commented Jun 29, 2017

Updated ptal @wtgodbe

@wtgodbe
Copy link
Member

wtgodbe commented Jun 29, 2017

LGTM pending CI

@jashook jashook merged commit 07258b7 into dotnet:master Jun 29, 2017
@jashook jashook deleted the fix_arm64_and_arm32_package_restore branch June 29, 2017 19:25
@karelz karelz modified the milestone: 2.1.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants