Always build FSharp.Core.UnitTests against the FSharp.Core in the repository #6360
Conversation
|
@Frassle , I will take a look at this, I don't think this is really the right fix. Because after all BUILD_IN_FSHARP_REPOSITORY should be true, in which case, it wouldn't use the nuget package. However, something is causing it to be false. Probably this is due to one of the directory.targets or directory.props, or I suppose both not looking up a directory. The scenario, where this is false is interesting, is for coreclr testing. Sometimes they will want to rebuild the tests, without access our full repo so they just suck an fsharp.core from nuget. I hope that makes sense. Kevin |
|
I mentioned this problem here too #6325 (comment) What is |
|
We build a nuget package with unit tests for the coreclr. They need to be able to build the tests, this is so they can build our unit tests from vs without having our full repo. |
Ah I see, ok. This is activating in our CI. We need a |
|
@don Syme<mailto:[email protected]> it is a bug, the true is set in the props file. However, something is stopping the build from picking it up, I think that is Directory.props file in the test directory that shouldn’t be there in our build, but needs to be there in the coreclr build. It’s just msbuild complicated nonsense. I will look into it tonight.
From: Don Syme <[email protected]>
Sent: Thursday, March 21, 2019 3:55 PM
To: Microsoft/visualfsharp <[email protected]>
Cc: Kevin Ransom <[email protected]>; Comment <[email protected]>
Subject: Re: [Microsoft/visualfsharp] Always build FSharp.Core.UnitTests against the FSharp.Core in the repository (#6360)
We build a nuget package with unit tests for the coreclr. They need to be able to build the tests, this is so they can build our unit tests from vs without having our full repo.
Ah I see, ok. This is activating in our CI. We need a #define for this to test features and fixes not yet available in the nuget package, and CI should test with that define both on and off.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.amrom.workers.dev%2FMicrosoft%2Fvisualfsharp%2Fpull%2F6360%23issuecomment-475433279&data=02%7C01%7CKevin.Ransom%40microsoft.com%7C73821871ba1b49e1b2b308d6ae503789%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636888056899085609&sdata=brsMI0rlDb7slOXCluHHccCPNlqCNUa5pAXQqQsnTu8%3D&reserved=0>, or mute the thread<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.amrom.workers.dev%2Fnotifications%2Funsubscribe-auth%2FAE76FgqjSvbsMBmqhOvROetpwTo0hHKVks5vZA22gaJpZM4cCYXU&data=02%7C01%7CKevin.Ransom%40microsoft.com%7C73821871ba1b49e1b2b308d6ae503789%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636888056899095617&sdata=QLceTj%2FWDYTICv2%2BSdl2%2BYsvg1Vhf1%2FXsrn8zd3Anh8%3D&reserved=0>.
|
|
OK taking all the above into account that this is needed in some cases I traced through to try and understand how it might suppose to work and why it's broken. FSharp.Core.UnitTests uses a project reference for FSharp.Core if BUILD_IN_FSHARP_REPOSITORY is set to true. That variable is set to true inside FSharpTests.Directory.Build.props. That props file is included by the tests\EndToEndBuildTests\Directory.Build.props and conditionally by FSharp.Directory.Build.props if FSharpTestCompilerVersion is not blank. FSharpTestCompilerVersion only seems to be set by files within the EndToEndBuildTests. So because the unit tests don't explicitly include FSharpTests.Directory.Build.props instead just including each Directory.Build.props file in the folder chain, and because nothing in the unit test build sets FSharpTestCompilerVersion then BUILD_IN_FSHARP_REPOSITORY is never set. All the obvious fixes would be to somehow include FSharpTests.Directory.Build.props but I'm not sure it's right that the unit tests should start including that instead of FSharpBuild.Directory.Build.props (what they include now). It all seems horrifically complicated, and that's before even considering how coreclr are using this repo. |
|
@Frassle you are not wrong about the horrifically complicated. |
3 participants
This changes FSharp.Core.UnitTests to always build against the FSharp.Core library in the repository. This ensures that FSharp.Core changes are correctly tested by CI, and it's easier to build and test changes to the library via make/build.cmd.