Skip to content

Introducing PR verification x86 CI build #1309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Oct 26, 2018
Merged

Conversation

artidoro
Copy link
Contributor

@artidoro artidoro commented Oct 19, 2018

Fixes #1389.

This PR adds a PR verification build for x86. Starting an x86 build is not too difficult, however running tests requires downloading an x86 .NET Core SDK

Main contributions:

  • runs debug and release x86 build
  • depending on the argument -buildArch=x86 it will download the .NET Core SDK for x86 or for x64 to be able to run the tests

@artidoro artidoro added the Build Build related issue label Oct 19, 2018
@artidoro artidoro changed the title Introducing PR verification x86 CI build WIP: Introducing PR verification x86 CI build Oct 19, 2018
@Ivanidzo4ka
Copy link
Contributor

Ivanidzo4ka commented Oct 19, 2018

We need to put conditions on TensorFlow transform tests since it's work only with x64 bits (at least from what I see from test failure).
Here is example:

[ConditionalFact(typeof(Environment), nameof(Environment.Is64BitProcess))] // x86 fails with "An attempt was made to load a program with an incorrect format."
#Resolved

@eerhardt
Copy link
Member

Looks good so far. Just need to get the tests passing.

{
return CheckEqualityCore(dir, name, nameBase ?? name, true, digitsOfPrecision);
int precision = digitsOfPrecision ?? (Environment.Is64BitProcess ? DigitsOfPrecision_x64 : DigitsOfPrecision_x86);
Copy link
Member

@danmoseley danmoseley Oct 23, 2018

Choose a reason for hiding this comment

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

Seems that int precision = digitsOfPrecision ?? (Environment.Is64BitProcess ? DigitsOfPrecision_x64 : DigitsOfPrecision_x86); could be pulled out into a property like public int DigitsOfPrecision => Environment.Is64BitProcess ? DigitsOfPrecision_x64 : DigitsOfPrecision_x86; rather than repeated #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found that having a different precision was not helpful, so I reverted that change. Thank you for the comment though!


In reply to: 227575101 [](ancestors = 227575101)

Copy link
Member

@tannergooding tannergooding left a comment

Choose a reason for hiding this comment

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

LGTM

@artidoro artidoro changed the title WIP: Introducing PR verification x86 CI build Introducing PR verification x86 CI build Oct 24, 2018
@adamsitnik
Copy link
Member

adamsitnik commented Oct 25, 2018

@artidoro regarding the failing benchmark integration test. You can add the [ConditionalFact(typeof(Environment), nameof(Environment.Is64BitProcess))] for it as well and create a new issue to fix it and assign me. #Resolved

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

:shipit:

@artidoro artidoro merged commit 216f672 into dotnet:master Oct 26, 2018
@artidoro artidoro deleted the addx86prbuild branch January 5, 2019 00:02
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Build Build related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants