Fix XM Full netstandard to copy in correct assemblies by chamons · Pull Request #2685 · dotnet/macios

chamons · 2017-09-13T19:57:27Z

Applications will build but crash on launch otherwise

- Applications will build but crash on launch otherwise

chamons · 2017-09-13T19:58:20Z

I caught this while writing an auto test.

radical · 2017-09-13T19:59:59Z

 		Inputs="$(TargetDir)$(TargetFileName)"
 		Outputs="$(_AppBundlePath)Contents\MacOS\$(_AppBundleName)">
+		<ItemGroup>
+			<ReferencesOnly Include="@(ReferenceCopyLocalPaths)" Condition="'%(ReferenceCopyLocalPaths.Extension)' == '.dll'" />


Maybe name like ReferenceCopyLocalAssemblyPaths?

Wouldn't that just append to the existing item group of that name?

oh, woops, the existing name doesn't have the word Assembly in it. n/m :)

yeah, I just wanted to make it easy to see where that extracted item was sourced from!

radical · 2017-09-13T20:06:14Z

and a commit message like:

[msbuild] Fix XM Full netstandard to copy in correct assemblies

nuget packages can have `ref` (reference assemblies) and `lib` (implementation assemblies),
with the former meant for use with the compiler and the latter required for execution.

The `ResolveAssemblyReference` task populates `@(ReferencePath)` with the assemblies meant
for use with the compiler, so these get the `ref` assemblies. And it populates `@(ReferenceCopyLocalPaths)`
with any assemblies or files related (.pdb/.mdb/.xml etc) to the references, and this one gets the `lib`
assemblies.

XM needs to bundle the assemblies required for execution, but it uses `@(ReferencePath)` and thus
ends up with the reference assembly, which would fail to load at runtime! Instead, we extract the `.dll`
files from `@(ReferenceCopyLocalPaths)` and use that.

chamons · 2017-09-13T20:13:07Z

I'll steal that when i squash merge.

chamons · 2017-09-13T20:13:23Z

build

monojenkins · 2017-09-13T22:03:32Z

Build failure

monojenkins · 2017-09-13T22:17:26Z

Build failure

chamons · 2017-09-14T14:19:35Z

Looks like the breakages are non-trivial, will have to dig into them.

chamons · 2017-09-14T19:41:23Z

Ok, that fixes all tests locally and them when I hack it to run msbuild (https://gist.github.com/chamons/8fb988deb4daca052663efa116cea084).

I think the change is safe to stew in master, but i'm questioning 15.4...

monojenkins · 2017-09-14T21:30:27Z

Build failure

chamons · 2017-09-15T15:21:11Z

This is strange, I can build things just fine locally, even with no XM installed. Let's try one more time to verify before I start digging up the world...

chamons · 2017-09-15T15:21:15Z

build

monojenkins · 2017-09-15T18:26:00Z

Build failure

chamons · 2017-09-18T17:21:42Z

Ok, somehow this broke build machine builds but not my local builds.

Digging...

chamons · 2017-09-18T18:08:16Z

I can reproduce if I git clean external/guiunit

monojenkins · 2017-09-19T18:59:54Z

Build failure

chamons · 2017-09-19T21:33:58Z

Test failure is known - https://bugzilla.xamarin.com/show_bug.cgi?id=59277

chamons · 2017-09-19T21:38:36Z

I feel more confident landing this, as it is nearly identical to what XI does.

…2731) - https://bugzilla.xamarin.com/show_bug.cgi?id=59474 - The idea is to force Full and Modern to expand facades the same way. That way, we get the same, working behavior. - f79f2e4 was not sufficient, even though it matched XI, because of the difference between XI (and Modern) and what Full was doing. - Some context: PR #2685 And that was problematic because it was expanding the netstandard facades from `Microsoft.NET.Build.Extensions` in the `ImplicitlyExpandNETStandardFacades` target. But we want to build against XM's bundled facades *only*. So we disable the ns facades completely by setting `$(ImplicitlyExpandNETStandardFacades) = false`. But now we are in the situation where a XM/Full project referencing a ns project might fail to build because of a missing `netstandard.dll` reference! And this same case was fixed for XM/Modern projects in #2643 . So, we enable the use of that for XM/Full projects too through `Xamarin.Mac.msbuild.targets`.

- Applications will build but crash on launch otherwise

…otnet#2731) - https://bugzilla.xamarin.com/show_bug.cgi?id=59474 - The idea is to force Full and Modern to expand facades the same way. That way, we get the same, working behavior. - f79f2e4 was not sufficient, even though it matched XI, because of the difference between XI (and Modern) and what Full was doing. - Some context: PR dotnet#2685 And that was problematic because it was expanding the netstandard facades from `Microsoft.NET.Build.Extensions` in the `ImplicitlyExpandNETStandardFacades` target. But we want to build against XM's bundled facades *only*. So we disable the ns facades completely by setting `$(ImplicitlyExpandNETStandardFacades) = false`. But now we are in the situation where a XM/Full project referencing a ns project might fail to build because of a missing `netstandard.dll` reference! And this same case was fixed for XM/Modern projects in dotnet#2643 . So, we enable the use of that for XM/Full projects too through `Xamarin.Mac.msbuild.targets`.

Fix XM Full netstandard to copy in correct assemblies

d03edc3

- Applications will build but crash on launch otherwise

dnfclas added the cla-already-signed label Sep 13, 2017

chamons requested review from kzu and radical September 13, 2017 19:58

chamons mentioned this pull request Sep 13, 2017

Cherry pick netstandard fixes to 15.4 #2680

Merged

radical reviewed Sep 13, 2017

View reviewed changes

Fix variable

8d5d587

chamons added the run-mmp-tests Run the mmp tests label Sep 13, 2017

jstedfast approved these changes Sep 13, 2017

View reviewed changes

radical approved these changes Sep 13, 2017

View reviewed changes

timrisi approved these changes Sep 13, 2017

View reviewed changes

chamons added the do-not-merge Do not merge this pull request label Sep 14, 2017

Fix ReferenceCopyLocalAssemblyPaths to actually contain assemblies

804f581

chamons removed the do-not-merge Do not merge this pull request label Sep 14, 2017

rolfbjarne approved these changes Sep 15, 2017

View reviewed changes

chamons mentioned this pull request Sep 15, 2017

[iosurface] IOSurface is not usable before iOS/tvOS 11. Fixes #59201 #2706

Merged

chamons added the do-not-merge Do not merge this pull request label Sep 18, 2017

VincentDondain approved these changes Sep 18, 2017

View reviewed changes

Fix Guiunit compilation failure by also including ReferencePath like XI

d9dff78

chamons requested a review from emaf as a code owner September 19, 2017 17:20

emaf approved these changes Sep 19, 2017

View reviewed changes

chamons removed the do-not-merge Do not merge this pull request label Sep 19, 2017

chamons merged commit f79f2e4 into dotnet:master Sep 19, 2017

chamons deleted the netstandard_xmfull_fix branch September 19, 2017 21:38

radical mentioned this pull request Sep 20, 2017

Fix netstd2 issue on XM full by expanding facades the same as Modern #2731

Merged

chamons added a commit to chamons/xamarin-macios that referenced this pull request Sep 27, 2017

Fix XM Full netstandard to copy in correct assemblies (dotnet#2685)

b3aca49

- Applications will build but crash on launch otherwise

Uh oh!

Conversation

chamons commented Sep 13, 2017

Uh oh!

chamons commented Sep 13, 2017

Uh oh!

radical Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

jstedfast Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

jstedfast Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

radical Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

radical commented Sep 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chamons commented Sep 13, 2017

Uh oh!

chamons commented Sep 13, 2017

Uh oh!

monojenkins commented Sep 13, 2017

Uh oh!

monojenkins commented Sep 13, 2017

Uh oh!

chamons commented Sep 14, 2017

Uh oh!

chamons commented Sep 14, 2017

Uh oh!

monojenkins commented Sep 14, 2017

Uh oh!

chamons commented Sep 15, 2017

Uh oh!

chamons commented Sep 15, 2017

Uh oh!

monojenkins commented Sep 15, 2017

Uh oh!

chamons commented Sep 18, 2017

Uh oh!

chamons commented Sep 18, 2017

Uh oh!

monojenkins commented Sep 19, 2017

Uh oh!

chamons commented Sep 19, 2017

Uh oh!

chamons commented Sep 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

radical commented Sep 13, 2017 •

edited

Loading