Skip to content

Conversation

@baronfel
Copy link
Owner

@baronfel baronfel commented May 17, 2019

Goals are to be able to build the whole compiler toolchain on latest stable mono with a recent msbuild, for net472 (I guess?).

We need to replicate the make && make install capabilities of the fsharp/fsharp makefile.

Vague plan of attack:

  • get current state building for net472 on stable mono + msbuild
  • enable tests to some degree
  • port over install logic
  • add this flow to the vsts ci platform

baronfel added 5 commits May 17, 2019 15:26
not currently working, though if you manually run msbuild /t:Restore followed by msbuild /t:Build /p:TargetFramework=net472 for that project, it all works great.
last step is to translate the dotnet test commands to msbuild /t:Test equivalents
@baronfel baronfel force-pushed the makefile-updates-for-mono branch from fae0f54 to d906c81 Compare May 18, 2019 15:55
@baronfel
Copy link
Owner Author

As of now we can build and test on mono 5.20 stable 🎆

<FrameworkPathOverride Condition="'$(TargetFramework)' == 'net47'">$(MonoLibFolder)/4.7-api</FrameworkPathOverride>
<FrameworkPathOverride Condition="'$(TargetFramework)' == 'net471'">$(MonoLibFolder)/4.7.1-api</FrameworkPathOverride>
<FrameworkPathOverride Condition="'$(TargetFramework)' == 'net472'">$(MonoLibFolder)/4.7.2-api</FrameworkPathOverride>
<FrameworkPathOverride Condition="'$(TargetFramework)' == 'net472'">$(MonoLibFolder)/4.7.2-api;$(MonoLibFolder)/4.7.2-api/Facades</FrameworkPathOverride>
Copy link
Owner Author

Choose a reason for hiding this comment

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

this can be reverted I believe. the stable msbuild targets for mono handle setting facades from what I can see now, and this property just ends up being reverted if you look at a detailed log.

$(DotNetExe) build src/fsharp/FSharp.Build/FSharp.Build.fsproj -f netstandard2.0 -c Proto
$(DotNetExe) build src/fsharp/fsc/fsc.fsproj -f netcoreapp2.1 -c Proto
proto:
$(RestoreCommand) $(NF472) src/buildtools/buildtools.proj
Copy link
Owner Author

Choose a reason for hiding this comment

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

virtually all changes to this file revolve around using pure msbuild equivalents to previous dotnet sdk commands.

<!-- version numbers from files -->
<RoslynVersion>$([System.IO.File]::ReadAllText('$(MSBuildThisFileDirectory)..\RoslynPackageVersion.txt').Trim())</RoslynVersion>
<!-- System.* packages -->
<SystemCollectionsVersion>4.3.0</SystemCollectionsVersion>
Copy link
Owner Author

Choose a reason for hiding this comment

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

initially I thought that adding a version here and flowing through a packagereference would be what we needed to get some projects to build on net472 for mono, but in the end plain References where what we needed.


<PropertyGroup>
<FsLexPath Condition="'$(FsLexPath)' == ''">$(ArtifactsDir)\Bootstrap\fslex.dll</FsLexPath>
<FsLexPath Condition="'$(FsLexPath)' == '' And '$(MonoPackaging)' == 'false' ">$(ArtifactsDir)\Bootstrap\netcoreapp2.1\fslex.dll</FsLexPath>
Copy link
Owner Author

Choose a reason for hiding this comment

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

changes to this file are around making fxlex/fsyacc invoke correctly on mono/unix. Directories have changed too and need to be validated (ie building by the makefile we don't get the Bootstrap subdirectory. if this is desired we should solve the artifactsdir problem then adjust these paths.

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp2.1</TargetFramework>
<TargetFrameworks>net472;netcoreapp2.1</TargetFrameworks>
Copy link
Owner Author

Choose a reason for hiding this comment

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

need to be able to invoke these tools on mono, so they build for net472 now.

<PropertyGroup>
<OutputType>Library</OutputType>
<TargetFrameworks>net472;netcoreapp2.1</TargetFrameworks>
<TargetFrameworks Condition="'$(OS)' == 'Unix'">netcoreapp2.1</TargetFrameworks>
Copy link
Owner Author

Choose a reason for hiding this comment

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

This was removed for several projects in order to enable mono builds


<ItemGroup Condition="'$(TargetFramework)' == 'net472'">
<PackageReference Include="Microsoft.VisualFSharp.Type.Providers.Redist" Version="$(MicrosoftVisualFSharpTypeProvidersRedistVersion)" />
<Reference Include="System.Collections" />
Copy link
Owner Author

Choose a reason for hiding this comment

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

Should this be strongly-versioned? (same question for the other instances)

<PackageReference Include="Microsoft.Build.Tasks.Core" Version="$(MicrosoftBuildTasksCoreVersion)" />
<PackageReference Include="Microsoft.Build.Utilities.Core" Version="$(MicrosoftBuildUtilitiesCoreVersion)" />
<PackageReference Include="Microsoft.Win32.Registry" Version="$(MicrosoftWin32RegistryVersion)" />
<PackageReference Include="System.Collections" Version="4.3.0" />
Copy link
Owner Author

Choose a reason for hiding this comment

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

Initially I wanted to use a packagereference for System.Collections like other assemblies do, but this didn't work for two reasons: I couldn't use the version from Versions.props (it never got picked up?) and the packagereference didn't delegate and add the appropriate gac reference.


<ItemGroup Condition="'$(TargetFramework)' == 'net472'">
<Reference Include="ISymWrapper, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" />
<Reference Include="System.Collections" />
Copy link
Owner Author

Choose a reason for hiding this comment

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

same story here for these Reference nodes. Should they be strongly-referenced and/or is there a package equivalent?

<AllowCrossTargeting>true</AllowCrossTargeting>
<DefineConstants>$(DefineConstants);FSHARP_CORE</DefineConstants>
<DefineConstants Condition="'$(Configuration)' == 'Proto'">BUILDING_WITH_LKG;$(DefineConstants)</DefineConstants>
<DefineConstants Condition="'$(MonoPackaging)' == 'true'">BUILD_FROM_SOURCE;$(DefineConstants)</DefineConstants>
Copy link
Owner Author

Choose a reason for hiding this comment

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

How is BUILD_FROM_SOURCE specified with the official builds? I couldn't replicate it with a command line flag and without this we get errors in prim-types.fsi where the inref/byref types are defined and used.

@baronfel
Copy link
Owner Author

@dsyme @cartermp @KevinRansom I'm starting work to make the main repo build on mono and wanted to isolate the changes as much as possible. Can you all look over the changes above (which are the ones most relevant to building and testing the repository on mono)? These should be the ones with the most overlap with the dotnet-sdk-based build chain that's already established, and the remainder of the work would be porting the install make target, which is more a question of file organization than build process IMO. I'd also be amenable to breaking any or all of this out in parts to upstream eg jsut the build-and-test portions in hopes of getting a mono-based ci build integrated before we began working on the mono-install tasks.

@baronfel
Copy link
Owner Author

I added a dockerfile and realized that you can only call the VsTest target on .net sdk 3 previews, per this statement from the vstest project readme:

The Test Platform currently ships as part Visual Studio 2017, and in the .NET Core Tools Preview 3.

So I've removed the test step from the plan for now. The remaining steps work just fine though, and can be run locally via make or in a docker mono 5.20 environment via docker build -f mono.Dockerfile .

@baronfel baronfel force-pushed the makefile-updates-for-mono branch from 04d15b2 to 6f85e74 Compare May 19, 2019 15:20
@baronfel baronfel force-pushed the makefile-updates-for-mono branch from 6f85e74 to 6f1bde0 Compare May 19, 2019 15:22
@baronfel
Copy link
Owner Author

I added a few mono patches that they'd added to address increased mono msbuild compatibility to msbuild v16, as well as allowing for a custom PREFIX in the makefile (nice for testing locally without blowing away your local install).

I also got testing working for FSharp.Core.UnitTests and FSharp.Build.UnitTests on mono by discovering and running nunit directly (though right now it's using the latest nunit.consolerunner package present and I'd like to pull that version from Versions.props or whereever is correct.

I'm still iterating on the install logic right now, trying to replicate the current directory layout structure as much as possible

@baronfel
Copy link
Owner Author

Tagging @ninjarobot on this as well for input and status updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants