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

Use .Net Core MSBuild for OS X and Linux Builds #5711

Merged
merged 1 commit into from
Jan 28, 2016

Conversation

joperezr
Copy link
Member

Use .Net Core MSBuild for OS X and Linux Builds

@@ -49,14 +49,14 @@
<ToolsDir Condition="'$(UseToolRuntimeForToolsDir)'=='true'">$(ToolRuntimePath)</ToolsDir>
<ToolsDir Condition="'$(ToolsDir)'==''">$(ProjectDir)Tools/</ToolsDir>
<DotnetCliPath Condition="'$(DotnetCliPath)'==''">$(ToolRuntimePath)dotnetcli/bin/</DotnetCliPath>
<BuildToolsTaskDir Condition="'$(BuildToolsTargets45)' == 'true'">$(ToolsDir)net45/</BuildToolsTaskDir>
<BuildToolsTaskDir Condition="'$(BuildToolsTargets45)' == 'true'">$(ToolsDir)/net45/</BuildToolsTaskDir>

<!-- Packaging configuration -->
<PreReleaseLabel>rc3</PreReleaseLabel>
<PackageDescriptionFile>$(ProjectDir)pkg/descriptions.json</PackageDescriptionFile>
<RuntimeIdGraphDefinitionFile>$(ProjectDir)pkg/Microsoft.NETCore.Platforms/runtime.json</RuntimeIdGraphDefinitionFile>
<!-- Add a condition for this when we are able to run on .NET Core -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this comment, or do we need a condition here?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can certainly add a condition, but technically is not required. The reason is that .Net Core MSBuild can only load assemblies living in the same directory as itself, so even though this properties say that the assemblies loaded should come from different folders, it is loading the .Net Core versions that live on the same dir. I'll add the condition either way to make it more clear.

@mellinoe
Copy link
Contributor

I can test this on my machine when the packages have been uploaded to buildtools and the temporary sources are removed, since the CI won't be building any of this.

I'm happy to see this is finally going in, it should hopefully get us much closer to a unified build process for x-plat builds.

@joperezr
Copy link
Member Author

@mellinoe I just pushed a change to my buildtools PR dotnet/buildtools#396 that removes the temporary sources given that I'm done pushing the packages into the buildtools feed now. That means that you should be able to sync to my branch in buildtools and build to produce a buildtools package, and use that to build xplat using the code in this PR.

@joperezr
Copy link
Member Author

cc @weshaggard @naamunds @JohnChen0

</ItemGroup>

<PropertyGroup>
<DnxPackageDir Condition="'$(DnxPackageDir)'==''">$(PackagesDir)/$(DnxPackageName)/</DnxPackageDir>
<DnuToolPath Condition="'$(DnuToolPath)'=='' and '$(OsEnvironment)'!='Unix'">$(DnxPackageDir)\bin\dnu.cmd</DnuToolPath>
<DnuToolPath Condition="'$(DnuToolPath)'=='' and '$(OsEnvironment)'=='Unix'">$(DnxPackageDir)/bin/dnu</DnuToolPath>
<DnuToolPath Condition="'$(DnuToolPath)'=='' and ('$(OsEnvironment)'!='Unix' or '$(OsEnvironment)'!='OSX')">$(DnxPackageDir)/bin/dnu.cmd</DnuToolPath>
Copy link
Member

Choose a reason for hiding this comment

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

Is this still necessary with your change to set OsEnviroment to Unix?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not anymore. I think that the ideal solution is not to change OsEnvironment to Unix, but instead handle the cases for OS X as well, but that is not super easy given that we have so many files that special-case Unix. Given that treating OS X as Unix was the way that builds in Mac used to work, I will remove this conditions for now, and log an issue so that we can move to special casing OSX as well.

@joperezr joperezr force-pushed the RestoreMSBuild branch 2 times, most recently from 276188a to ec57c6a Compare January 27, 2016 17:32
@mellinoe
Copy link
Contributor

@joperezr I think there may be an issue with the Open signing key. So far most of the projects have built fine, but I'm getting this error from the projects that do use the new key:

CSC : error CS7027: Error signing output with public key from file '/home/eric/projects/corefx/Tools/Open.snk' -- Cannot marshal 'return value': Invalid managed [/home/eric/projects/corefx/src/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj]

Still running the build (haven't gotten to to tests or packaging yet), but everything seems to be running smoothly other than the above.

@joperezr
Copy link
Member Author

@mellinoe yeah I saw that as well, that started happening this morning when I rebased. It seems to be a compiler bug, I'll log the issue and push the workaround

@mellinoe
Copy link
Contributor

Workaround looks fine to me. Like I mentioned, we are doing the same thing with the ECMA key right now.

EDIT: Actually, maybe we should make the workaround the same as the ECMA Key one:

https://github.com/dotnet/corefx/blob/0dd1ab7223dea82f236ec81d42f4ae0227176bf4/dir.props

<!--
      Delay signing with the ECMA key currently doesn't work.
      https://github.com/dotnet/roslyn/issues/2444
    -->
    <UseECMAKey>false</UseECMAKey>

In the project files we just do:

<UseECMAKey Condition="'$(UseECMAKey)' == ''">true</UseECMAKey>

I'm fine either way, but it might be nice to have both of these properties in one place so we can remember to address them both (I'm assuming they're related)

@joperezr joperezr force-pushed the RestoreMSBuild branch 2 times, most recently from 812179d to 42ffafd Compare January 27, 2016 23:37
@weshaggard
Copy link
Member

I kind of agree with @mellinoe that we should probably just add the explicit set to false in the same place and in the individual projects only set it to true if the property is empty.

@joperezr
Copy link
Member Author

@weshaggard @mellinoe Sounds good, I'll do that.

@weshaggard
Copy link
Member

LGTM please make sure to file the various tracking bugs for the comments.

@weshaggard
Copy link
Member

@dotnet-bot test Innerloop OSX Release Build and Test

@mellinoe
Copy link
Contributor

LGTM

joperezr added a commit that referenced this pull request Jan 28, 2016
Use .Net Core MSBuild for OS X and Linux Builds
@joperezr joperezr merged commit ab6ec2f into dotnet:master Jan 28, 2016
@joperezr joperezr deleted the RestoreMSBuild branch January 28, 2016 23:51
@ghost
Copy link

ghost commented Feb 18, 2016

@ellismg, can we have a similar upgrade in CoreCLR? Currently building CoreCLR on NetBSD is requiring Mono; which unlike FreeBSD is bit more inconvenient -- although we have installed full mono packs on our systems, but this is something we would greatly appreciate and even willing to pull off ourselves (by taking inspirations from @joperezr's work in this PR), if we get a 'go ahead'. =)

@ellismg
Copy link
Contributor

ellismg commented Feb 18, 2016

@jasonwilliams200OK We are 99.99% of the way there, and dotnet/coreclr#3214 will remove the last mono dependency.

@ghost
Copy link

ghost commented Feb 18, 2016

Thanks @ellismg! Perfect timings I'd say.. 🎯

@karelz karelz modified the milestone: 1.0.0-rtm Dec 3, 2016
macrogreg pushed a commit to open-telemetry/opentelemetry-dotnet-instrumentation that referenced this pull request Sep 24, 2020
Use .Net Core MSBuild for OS X and Linux Builds

Commit migrated from dotnet/corefx@ab6ec2f
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Use .Net Core MSBuild for OS X and Linux Builds

Commit migrated from dotnet/corefx@ab6ec2f
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
7 participants