Fix the Mono and OSX builds #4273

dsyme · 2018-01-26T17:26:49Z

The settings for the MonoPackaging build were removed inadvertently by @brettfo, this re-establishes them. These changes have been verified for Mono in http://github.com/fsharp/fsharp.

The build.sh for building on OSX and Linux was toast. This re-establishes that, at least for OSX. Some more work will be needed here to make this correctly build .NET Core on Linux and Mac (if that's even how we want to build things). It's really annoying to have duplication between build.cmd and build.sh but I've removed large chunks of VS-specific stuff from build.sh that we don't need.

CI is also updated to run build.sh debug and release on Linux.

KevinRansom · 2018-01-26T17:44:55Z

FSharp.sln

@@ -31,7 +31,7 @@ Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "FSharp.Tests.FSharpSuite",
 EndProject
 Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "FSharp.Compiler.UnitTests", "tests\FSharp.Compiler.UnitTests\FSharp.Compiler.UnitTests.fsproj", "{A8D9641A-9170-4CF4-8FE0-6DB8C134E1B5}"
 EndProject
-Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "FSharp.Core.UnitTests", "tests\FSharp.Core.UnitTests\FSharp.Core.UnitTests.fsproj", "{88E2D422-6852-46E3-A740-83E391DC7973}"
+Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "FSharp.Core.UnitTests", "tests\FSharp.Core.UnitTests\FSharp.Core.Unittests.fsproj", "{88E2D422-6852-46E3-A740-83E391DC7973}"


Good grief!!!! we should probably fix this casing by renaming the file on disk, rather than in the projects. No one is ever going to remember to lowercase the tests.

Yes, this one was missed when I normalized the file names.

However I've had hell sometimes with git when changing casing of filenames, so for the moment lets leave this. We can fix it later

Yes. It's hard to change casing on windows. It's nearly impossible to get it spread to all devs and not regress by someone.

KevinRansom · 2018-01-26T17:48:37Z

src/FSharpSource.Settings.targets

  </PropertyGroup>

+<!--


Why is this commented out?

Thanks, yes, checking this now

KevinRansom · 2018-01-26T17:58:55Z

src/FSharpSource.targets

-      </PropertyGroup>
-    </Otherwise>
-  </Choose>
+  <PropertyGroup>


@dsyme I'm not clear how the signing settings end up for a proto build with this new factoring, I.e ... when Configuration='proto'

It uses the default settings used for all compiler binaries - delay signed on windows, test.snk signed on Mono

The default settings for non-Mono are in FSharpSource.Settings.targets. I'm not sure why, they were there before

<PropertyGroup> <SkipSigning>false</SkipSigning> <UseOpenSourceSign>true</UseOpenSourceSign> <SignAssembly>true</SignAssembly> <AssemblyOriginatorKeyFile>$(FSharpSourcesRoot)\fsharp\msft.pubkey</AssemblyOriginatorKeyFile> <StrongNames>true</StrongNames> <DelaySign>true</DelaySign> </PropertyGroup>

dsyme · 2018-01-26T18:11:40Z

packages.config

@@ -21,6 +21,10 @@
  <package id="Microsoft.DiaSymReader" version="1.1.0" />
  <package id="System.ValueTuple" version="4.3.1" />
  <package id="Microsoft.VisualFSharp.Msbuild.15.0" version="1.0.1" />
+  <package id="Microsoft.Build" version="14.3.0" />


@KevinRansom These are needed when building the MonoPackaging: for better or worse the fsharp/fsharp build must be able to build using only packages from nuget.org.

The Microsoft.VisualFSharp.Msbuild.15.0 is a private package but I don't think we actually need rely on it even for the DevDiv build

If we could avoid using any private packages for the compiler/build tools themselves that would b much appreciated.

dsyme · 2018-01-26T18:16:55Z

@KevinRansom Still working on this, will check final details later tonight

brettfo · 2018-01-26T19:00:16Z

src/fsharp/FSharp.Compiler.Private/AssemblyInfo.fs

+open System.Runtime.InteropServices
+
+#if STRONG_NAME_FSHARP_COMPILER_WITH_TEST_KEY
+[<assembly:System.Runtime.CompilerServices.InternalsVisibleTo("fsc, PublicKey=002400000480000094000000060200000024000052534131000400000100010077d32e043d184cf8cebf177201ec6fad091581a3a639a0534f1c4ebb3ab847a6b6636990224a04cf4bd1aec51ecec44cf0c8922eb5bb2ee65ec3fb9baa87e141042c96ce414f98af33508c7e24dab5b068aa802f6693881537ee0efcb5d3f1c9aaf8215ac42e92ba9a5a02574d6890d07464cb2f338b043b1c4ffe98efe069ee")>]


These should be handled by the <InternalsVisibleTo /> elements in the project file.

@brettfo That stuff doesn't work in the Mono build. Or perhaps it's using the wrong key? I only just noticed that you hardwire the key here:

<_PublicKey>002400000480000094000000060200000024000052534131000400000100010007D1FA57C4AED9F0A32E84AA0FAEFD0DE9E8FD6AEC8F87FB03766C834C99921EB23BE79AD9D5DCC1DD9AD236132102900B723CF980957FC4E177108FC607774F29E8320E92EA05ECE4E821C0A5EFE8F1645C4C0C93C1AB99285D622CAA652C1DFAD63D745D6F2DE5F17E5EAF0FC4963D261C8A12436518206DC093344D5AD293</_PublicKey>

I'll check if making that conditional does the trick.

The key can be overridden via this like so:

<ItemGroup Condition="not mono"> <InternalsVisibleTo Include="fsc" /> </ItemGroup> <ItemGroup Condition="mono"> <InternalsVisibleTo Include="fsc" Key="1234" /> </ItemGroup>

Ah ok, will try that, thanks

brettfo · 2018-01-26T19:03:03Z

@dsyme Before you merge this I'd like to do a full internal build to make sure we don't regress any assembly signing. I'll try to keep an eye on this and as soon as it's green I'll start the signed build.

dsyme · 2018-01-26T22:07:42Z

before_install.sh

@@ -32,7 +32,7 @@ fi
 # We need to run the command twice -- on some systems (e.g. macOS) the certs are installed in the user store,
 # and on other systems (e.g., Ubuntu) they're installed to the machine store. certmgr only shows what's in
 # the selected store, which is why we need to check both.
-if [ "$(certmgr -list -c Trust | grep -c -F "X.509")" -le 1 ] && [ "$(certmgr -list -c -m Trust | grep -c -F "X.509")" -le 1 ]; then
+if [ $OS = 'Linux' ] && [ "$(certmgr -list -c Trust | grep -c -F "X.509")" -le 1 ] && [ "$(certmgr -list -c -m Trust | grep -c -F "X.509")" -le 1 ]; then


AFAIK these certificates are only needed on older versions of Mono. At least on OSX

dsyme · 2018-01-26T22:08:22Z

build/targets/GitHash.props

@@ -5,7 +5,7 @@
 Copied from RepoToolset. Might be slightly modified to adjust for the current F# build system specifics if necessary.
 -->

-<Project>
+<Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">


This was preventing the project files from compiling on Mono

dsyme · 2018-01-27T00:39:44Z

@brettfo Any reason not to use this for the global setting in FSharpSource.targets?

    <PropertyGroup Condition="'$(MonoPackaging)' != 'true'">
      <_PublicKey>002400000480000094000000060200000024000052534131000400000100010007D1FA57C4AED9F0A32E84AA0FAEFD0DE9E8FD6AEC8F87FB03766C834C99921EB23BE79AD9D5DCC1DD9AD236132102900B723CF980957FC4E177108FC607774F29E8320E92EA05ECE4E821C0A5EFE8F1645C4C0C93C1AB99285D622CAA652C1DFAD63D745D6F2DE5F17E5EAF0FC4963D261C8A12436518206DC093344D5AD293</_PublicKey>
    </PropertyGroup>
    <PropertyGroup Condition="'$(MonoPackaging)' == 'true'">
      <_PublicKey>002400000480000094000000060200000024000052534131000400000100010077d32e043d184cf8cebf177201ec6fad091581a3a639a0534f1c4ebb3ab847a6b6636990224a04cf4bd1aec51ecec44cf0c8922eb5bb2ee65ec3fb9baa87e141042c96ce414f98af33508c7e24dab5b068aa802f6693881537ee0efcb5d3f1c9aaf8215ac42e92ba9a5a02574d6890d07464cb2f338b043b1c4ffe98efe069ee</_PublicKey>
    </PropertyGroup>

brettfo · 2018-01-27T00:45:55Z

That seems good to me, and will reduce the number of <InternalsVisibleTo /> elements.

dsyme · 2018-01-27T17:41:21Z

This is ready to merge, subject to your checks @brettfo

@dsyme Before you merge this I'd like to do a full internal build to make sure we don't regress any assembly signing. I'll try to keep an eye on this and as soon as it's green I'll start the signed build.

I've done a careful check of the conditions listed. The logic is actually simpler now, especially for the Debug/Release build case for the regular build, where the default settings here https://github.com/Microsoft/visualfsharp/pull/4273/files#diff-88901e0c749aca7d233ea1ae8a4e5275R112 are used since none of the other overriding cases (Proto, MonoPackaging, StrongNames=false) apply.

brettfo · 2018-01-27T21:49:39Z

Internal build started. I'll reply back as soon as it's done.

brettfo · 2018-01-27T23:18:22Z

@dsyme Internal build is green, merge whenever you're ready.

dsyme · 2018-01-29T19:30:59Z

@brettfo Thanks, done!

fix build.sh for OSX and a little cleanup

dd0e119

KevinRansom reviewed Jan 26, 2018

View reviewed changes

dsyme force-pushed the osx-fixes branch from 379f46f to 6ad826c Compare January 26, 2018 18:07

dsyme commented Jan 26, 2018

View reviewed changes

brettfo reviewed Jan 26, 2018

View reviewed changes

dsyme commented Jan 26, 2018

View reviewed changes

fix build for mono packaging

5c8e8b7

dsyme force-pushed the osx-fixes branch from 8fed26e to 5c8e8b7 Compare January 27, 2018 00:42

dsyme added 2 commits January 27, 2018 01:32

fix build for mono packaging

f1b3f40

integrate master

74bb6cc

dsyme mentioned this pull request Jan 27, 2018

FSharp.Core.Unittests do not pass on Mono/OSX #4275

Closed

dsyme added 2 commits January 27, 2018 02:05

disable some tests by default on OSX with build.sh test

627cd5f

fix build from source IVTs

b49021d

dsyme mentioned this pull request Jan 29, 2018

"build coreclr" rebuilds everything every time #3295

Closed

Merge branch 'master' into osx-fixes

dd67cbb

dsyme merged commit bca446d into dotnet:master Jan 29, 2018

brettfo mentioned this pull request Feb 2, 2018

Merge master to dev15.7 #4302

Closed

brettfo mentioned this pull request Mar 29, 2018

[FS-1047] - match! (match-bang) #4427

Merged

		</PropertyGroup>

		<!--

Fix the Mono and OSX builds #4273

Fix the Mono and OSX builds #4273

Uh oh!

Conversation

dsyme commented Jan 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsyme commented Jan 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsyme Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettfo Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettfo commented Jan 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsyme commented Jan 27, 2018

Uh oh!

brettfo commented Jan 27, 2018

Uh oh!

dsyme commented Jan 27, 2018

Uh oh!

brettfo commented Jan 27, 2018

Uh oh!

brettfo commented Jan 27, 2018

Uh oh!

dsyme commented Jan 29, 2018

Uh oh!

Uh oh!

dsyme Jan 26, 2018 •

edited

Loading

brettfo Jan 26, 2018 •

edited

Loading