[msbuild] Fix how UnpackLibraryResources handles mscorlib.dll (and potentially other framework assemblies) by spouliot · Pull Request #1011 · dotnet/macios

spouliot · 2016-10-20T20:04:37Z

Target _UnpackLibraryResources:
    Task "UnpackLibraryResources"
        Using task UnpackLibraryResources from Xamarin.MacDev.Tasks.UnpackLibraryResources, Xamarin.MacDev.Tasks, Version=1.0.6128.15885, Culture=neutral, PublicKeyToken=null
        UnpackLibraryResources Task
          Prefix: monotouch
          IntermediateOutputPath: obj/iPhone/Debug/
          NoOverwrite:
            obj/iPhone/Debug/ibtool-link/LaunchScreen.storyboardc/01J-lp-oVM-view-Ze5-6b-2t3.nib
            obj/iPhone/Debug/ibtool-link/LaunchScreen.storyboardc/Info.plist
            obj/iPhone/Debug/ibtool-link/LaunchScreen.storyboardc/UIViewController-01J-lp-oVM.nib
            obj/iPhone/Debug/ibtool-link/Main.storyboardc/BYZ-38-t0r-view-8bC-Xf-vdC.nib
            obj/iPhone/Debug/ibtool-link/Main.storyboardc/Info.plist
            obj/iPhone/Debug/ibtool-link/Main.storyboardc/UIViewController-BYZ-38-t0r.nib
          ReferencedLibraries:
            /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.dll
            /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.Xml.dll
            /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.Core.dll
            /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/Xamarin.iOS.dll
            /Users/poupou/Downloads/LinkingTest-2/RMSDKWrapper/bin/Debug//RMSDKWrapper.dll
            /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS//mscorlib.dll
            /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS//mscorlib.dll
          Skipping framework assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.dll
          Skipping framework assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.Xml.dll
          Skipping framework assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.Core.dll
          Skipping framework assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/Xamarin.iOS.dll
          Inspecting assembly: /Users/poupou/Downloads/LinkingTest-2/RMSDKWrapper/bin/Debug//RMSDKWrapper.dll
          Inspecting assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS//mscorlib.dll
          Inspecting assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS//mscorlib.dll
    Done executing task "UnpackLibraryResources"
    Done building target "_UnpackLibraryResources" in project "/Users/poupou/Downloads/LinkingTest-2/LinkingTest/LinkingTest.csproj".

The above log excerpt shows two issues:

mscorlib.dll is needlessly inspected as it's not considered a
"framework" assembly.

The current check was checking how it was resolved and not where
it was resolved to. The later is the most important as it's possible
for other assemblies to have direct paths references and we do not
want to process them.

This is fixed by comparing each assembly path with the (now) provided
TargetFrameworkDirectory
mscorlib.dll is inspected twice

That's because it's present two times in the task's input. That issue
is upstream (not sure why) of the current task but it makes make install does not work unless make all is already complete #1 twice
as costly. The fix for make install does not work unless make all is already complete #1 indirectly fix that too.

Future

It's worth investigating to move that logic into mtouch. The later must
already load all assemblies and is in charge of removing other embedded
data (e.g. native code from bindings) from the assemblies (so they are not
shipped both inside and outside the .dll in the final .app). This makes
this task seems extraneous work.

Considering that my current test case, RMSDKWrapper.dll, is 1.3GB in
size it's easy to see that the extra load (which has nothing to be
extracted wrt resources*) is quite visible in build time.

3268.201 ms UnpackLibraryResources 1 calls

it has for bindings but that's already handled by mtouch

…tentially other framework assemblies) Target _UnpackLibraryResources: Task "UnpackLibraryResources" Using task UnpackLibraryResources from Xamarin.MacDev.Tasks.UnpackLibraryResources, Xamarin.MacDev.Tasks, Version=1.0.6128.15885, Culture=neutral, PublicKeyToken=null UnpackLibraryResources Task Prefix: monotouch IntermediateOutputPath: obj/iPhone/Debug/ NoOverwrite: obj/iPhone/Debug/ibtool-link/LaunchScreen.storyboardc/01J-lp-oVM-view-Ze5-6b-2t3.nib obj/iPhone/Debug/ibtool-link/LaunchScreen.storyboardc/Info.plist obj/iPhone/Debug/ibtool-link/LaunchScreen.storyboardc/UIViewController-01J-lp-oVM.nib obj/iPhone/Debug/ibtool-link/Main.storyboardc/BYZ-38-t0r-view-8bC-Xf-vdC.nib obj/iPhone/Debug/ibtool-link/Main.storyboardc/Info.plist obj/iPhone/Debug/ibtool-link/Main.storyboardc/UIViewController-BYZ-38-t0r.nib ReferencedLibraries: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.dll /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.Xml.dll /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.Core.dll /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/Xamarin.iOS.dll /Users/poupou/Downloads/LinkingTest-2/RMSDKWrapper/bin/Debug//RMSDKWrapper.dll /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS//mscorlib.dll /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS//mscorlib.dll Skipping framework assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.dll Skipping framework assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.Xml.dll Skipping framework assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/System.Core.dll Skipping framework assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS/Xamarin.iOS.dll Inspecting assembly: /Users/poupou/Downloads/LinkingTest-2/RMSDKWrapper/bin/Debug//RMSDKWrapper.dll Inspecting assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS//mscorlib.dll Inspecting assembly: /Users/poupou/git/xamarin/xamarin-macios/_ios-build/Library/Frameworks/Xamarin.iOS.framework/Versions/Current/lib/mono/Xamarin.iOS//mscorlib.dll Done executing task "UnpackLibraryResources" Done building target "_UnpackLibraryResources" in project "/Users/poupou/Downloads/LinkingTest-2/LinkingTest/LinkingTest.csproj". The above log excerpt shows two issues: 1. mscorlib.dll is needlessly inspected as it's **not** considered a "framework" assembly. The current check was checking *how* it was resolved and not *where* it was resolved to. The later is the most important as it's possible for other assemblies to have direct paths references and we do not want to process them. This is fixed by comparing each assembly path with the (now) provided `TargetFrameworkDirectory` 2. mscorlib.dll is inspected twice That's because it's present two times in the task's input. That issue is upstream (not sure why) of the current task but it makes #1 twice as costly. The fix for #1 indirectly fix that too. Future ------ It's worth investigating to move that logic into `mtouch`. The later must already load all assemblies and is in charge of removing other embedded data (e.g. native code from bindings) from the assemblies (so they are not shipped both inside and outside the .dll in the final .app). This makes this task seems extraneous work. Considering that my current test case, `RMSDKWrapper.dll`, is 1.3GB in size it's easy to see that the extra load (which has nothing to be extracted wrt resources*) is quite visible in build time. > 3268.201 ms UnpackLibraryResources 1 calls * it has for bindings but that's already handled by mtouch

spouliot · 2016-10-20T20:05:21Z

@radical, @jstedfast feedback welcome :)

jstedfast · 2016-10-20T20:16:41Z

+		{
+			foreach (var dir in TargetFrameworkDirectory) {
+				if (asm.ItemSpec.StartsWith (dir.ItemSpec, StringComparison.Ordinal))
+					return true;


Depending on how the TaskItem is created, the ItemSpec may be a relative path so it may be better to use the full path of each item using this approach:

var path = item.GetMetadata ("FullPath");

Obviously we can do this outside the loop for asm as an optimization.

My other suggestion is that we may want to use an OrdinalIgnoreCase compare since the Mac file system is case insensitive by default.

rolfbjarne · 2016-10-28T05:11:14Z

build

monojenkins · 2016-10-28T08:22:03Z

Build failure

rolfbjarne · 2016-10-28T08:23:24Z

Build failure:

/Users/builder/jenkins/workspace/xamarin-macios-pr-builder/_mac-build/Library/Frameworks/Mono.framework/External/xbuild/Xamarin/Mac/Xamarin.Mac.Common.targets: error : Error executing task UnpackLibraryResources: Required property 'TargetFrameworkDirectory' not set.

jstedfast · 2016-10-28T12:15:14Z

Looks like TargetFrameworkDirectory does not get defined. Is this something that was supposed to be defined by xbuild?

spouliot · 2017-01-04T16:16:37Z

The build failure happened on XM - so it seems TargetFrameworkDirectory is defined only on iOS (and not macOS) and the task is shared between them.

…n bots)

monojenkins · 2017-01-10T03:30:37Z

Build failure

…ctories

spouliot · 2017-01-10T13:41:54Z

@jstedfast please double check that I understood your comment correctly, thanks!

monojenkins · 2017-01-10T14:21:34Z

Build failure

spouliot · 2017-01-10T14:53:42Z

random failure due to https://bugzilla.xamarin.com/show_bug.cgi?id=50634

spouliot · 2017-01-10T14:53:51Z

build

monojenkins · 2017-01-10T17:06:07Z

Build success

jstedfast · 2017-01-10T17:25:42Z

@spouliot yep, that's what I meant.

I approve this PR.

dnfclas added the cla-already-signed label Oct 20, 2016

jstedfast reviewed Oct 20, 2016

View reviewed changes

rolfbjarne approved these changes Oct 21, 2016

View reviewed changes

spouliot added the do-not-merge Do not merge this pull request label Nov 1, 2016

spouliot self-assigned this Jan 9, 2017

Add TargetFrameworkDirectory for XM too (should fix the build issue o…

9dcebe3

…n bots)

[msbuild] Use FullPath to detect assemblies inside the framework dire…

68c2df1

…ctories

spouliot removed the do-not-merge Do not merge this pull request label Jan 10, 2017

spouliot merged commit 08c54f7 into dotnet:master Jan 10, 2017

spouliot deleted the msbuild-unpack-avoid-mscorlib branch January 10, 2017 20:03

chamons mentioned this pull request Oct 1, 2018

Disable known failing device tests using xharness #4884

Merged

Uh oh!

Conversation

spouliot commented Oct 20, 2016 • edited by rolfbjarne Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Future

Uh oh!

spouliot commented Oct 20, 2016

Uh oh!

jstedfast Oct 20, 2016

Choose a reason for hiding this comment

Uh oh!

rolfbjarne commented Oct 28, 2016

Uh oh!

monojenkins commented Oct 28, 2016

Uh oh!

rolfbjarne commented Oct 28, 2016

Uh oh!

jstedfast commented Oct 28, 2016

Uh oh!

spouliot commented Jan 4, 2017

Uh oh!

monojenkins commented Jan 10, 2017

Uh oh!

spouliot commented Jan 10, 2017

Uh oh!

monojenkins commented Jan 10, 2017

Uh oh!

spouliot commented Jan 10, 2017

Uh oh!

spouliot commented Jan 10, 2017

Uh oh!

monojenkins commented Jan 10, 2017

Uh oh!

jstedfast commented Jan 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

spouliot commented Oct 20, 2016 •

edited by rolfbjarne

Loading