Skip to content

[macos] Fix msbuild issues with shprojj/projitem library - #1936

Merged
chamons merged 3 commits into
masterfrom
xm_bojan
Mar 30, 2017
Merged

[macos] Fix msbuild issues with shprojj/projitem library#1936
chamons merged 3 commits into
masterfrom
xm_bojan

Conversation

@chamons

@chamons chamons commented Mar 29, 2017

Copy link
Copy Markdown
Contributor
  • In some build cases this chunk of code:

    <_ExplicitReference Include="$(FrameworkPathOverride)\mscorlib.dll" />

would trigger and force us to use mscorlib from system mono. That does not work well.

  • By setting FrameworkPathOverride, we can get the right mscorlib
  • However, that ItemGroup happens outside of a target, so we must move our setting to match for it to take effect

- In some build cases this chunk of code:

    <ItemGroup Condition=" '$(NoCompilerStandardLib)' == 'true' and '$(NoStdLib)' != 'true' ">
          <!-- Note that unlike VB, C# does not automatically locate System.dll as a "standard library"
               instead the reference is always passed from the project. Also, for mscorlib.dll
               we need to provide the explicit location in order to maintain the correct behaviour
     -->
        <_ExplicitReference Include="$(FrameworkPathOverride)\mscorlib.dll" />
    </ItemGroup>

would trigger and force us to use mscorlib from system mono. That does not work well.
- By setting FrameworkPathOverride, we can get the right mscorlib
- However, that ItemGroup happens outside of a target, so we must move our setting to match for it to take effect

@radical radical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The msbuild change looks fine. I didn't really look at the test.

@chamons
chamons requested review from emaf and kzu March 29, 2017 22:15
@chamons

chamons commented Mar 29, 2017

Copy link
Copy Markdown
Contributor Author

Added VS guys since it is an msbuild change.

I forgot to add you to these two msbuild changes that happened in recent past:

#1846
#1827

apologies.

@monojenkins

Copy link
Copy Markdown
Contributor

Build success

@spouliot spouliot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 for adding a test case :)


<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<!-- MSBuild specific hacks - Teach msbuild where to find our BCL and facades -->
<PropertyGroup>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should have a Condition="'$(OS)' != 'Win_NT'"

<MacBclPath Condition="'$(TargetFrameworkIdentifier)' == 'Xamarin.Mac'">$(XamarinMacFrameworkRoot)/lib/mono/Xamarin.Mac</MacBclPath>
<FrameworkPathOverride>$(MacBclPath)</FrameworkPathOverride>
</PropertyGroup>
<Target Name="FixTargetFrameworkDirectory" AfterTargets="GetReferenceAssemblyPaths">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ditto here: Condition="'$(OS)' != 'Win_NT'"

@chamons

chamons commented Mar 30, 2017

Copy link
Copy Markdown
Contributor Author

@kzu Changes made.

@monojenkins

Copy link
Copy Markdown
Contributor

Build failure

1 similar comment
@monojenkins

Copy link
Copy Markdown
Contributor

Build failure

@chamons

chamons commented Mar 30, 2017

Copy link
Copy Markdown
Contributor Author

Known random failure - https://bugzilla.xamarin.com/show_bug.cgi?id=52968

@chamons
chamons merged commit 1ec3849 into master Mar 30, 2017
@chamons
chamons deleted the xm_bojan branch March 30, 2017 20:04
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.

8 participants