Packing iOS binding projects with NativeReference fails

[mattleibow]

Steps to reproduce

I have created a repository with this issue:
https://github.com/mattleibow/XamarinNativeReferencesBug

I have opened an issue on the MSBuild.Sdk.Extras repository as they can add a workaround there as well: novotnyllc/MSBuildSdkExtras#176

Project file

<Project>
<Project Sdk="MSBuild.Sdk.Extras/2.0.31">
  <PropertyGroup>
    <TargetFrameworks>xamarinios1.0</TargetFrameworks>
    <IsBindingProject>true</IsBindingProject>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
  </PropertyGroup>
  <ItemGroup>
    <Compile Remove="ApiDefinition.cs" />
    <Compile Remove="StructsAndEnums.cs" />
    <ObjcBindingApiDefinition Include="ApiDefinition.cs" />
    <ObjcBindingCoreSource Include="StructsAndEnums.cs" />
    <NativeReference Include="..\native\Aardvark.framework" Kind="Framework" ForceLoad="true" SmarkLink="true" Frameworks="MessageUI" LinkerFlags="-ObjC" />
  </ItemGroup>
</Project>

Command line

msbuild /restore /t:pack /bl

Expected behavior

The project builds with two packages as output.

Actual behavior

The project fails with an error:

error : The file '/Users/matthew/Projects/Testing/XamarinNativeReferences/source/Square.Aardvark/bin/Debug/xamarinios1.0/Native.Square.Aardvark.manifest' to be packed was not found on disk.

Environment data

msbuild /version output:

Microsoft (R) Build Engine version 16.2.0-ci for Mono
Copyright (C) Microsoft Corporation. All rights reserved.

16.200.19.36501

OS info:
macOS Mojave 10.14.5 (18F132)

[Redth]

@chamons your team should probably be aware of / involved in this discussion as well :)

[livarcocc]

@mattleibow why do you believe this is an issue with MSBuild?

[mattleibow]

Because MSBuild (Microsoft.Common.CurrentVersion.targets) assumes that all instances of NativeReference is a sort of thing used to generate a manifest or something:

https://github.com/microsoft/msbuild/blob/v16.2.32702/src/Tasks/Microsoft.Common.CurrentVersion.targets#L5548-L5554

    <ItemGroup>
      <_BuiltProjectOutputGroupOutputIntermediate Include="$(OutDir)$(_DeploymentTargetApplicationManifestFileName)" Condition="'@(NativeReference)'!='' or '@(_IsolatedComReference)'!=''">
        <TargetPath>$(_DeploymentTargetApplicationManifestFileName)</TargetPath>
        <!-- For compatibility with 2.0 -->
        <OriginalItemSpec>$(OutDir)$(_DeploymentTargetApplicationManifestFileName)</OriginalItemSpec>
      </_BuiltProjectOutputGroupOutputIntermediate>
    </ItemGroup>

The assumption of the existence of a NativeReference instance does not mean that we need to have a file named Native.$(AssemblyName).manifest added to the output.

Maybe it should, then in that case, it appears that MSBuild does not output that file for iOS projects. If that file is created, then it causes issues with other parts of the build.

That could be a Xamarin thing - not generating the file - so we could move it to the https://github.com/xamarin/xamarin-macios repo.

[rolfbjarne]

That could be a Xamarin thing - not generating the file - so we could move it to the https://github.com/xamarin/xamarin-macios repo.

No, we (Xamarin) is not generating that file because it serves us no purpose.

The assumption of the existence of a NativeReference instance does not mean that we need to have a file named Native.$(AssemblyName).manifest added to the output.

This is the problem: the existence of a NativeReference item doesn't mean that such a manifest file will be created.

[rolfbjarne]

So we found a workaround for this:

https://github.com/xamarin/xamarin-macios/blob/df395f2cff9b121fb3b7f1dc3a37f2948dbebc04/msbuild/Xamarin.Shared/Xamarin.Shared.targets#L208-L225

unfortunately the workaround does not work when a project uses TargetFrameworks (plural) instead of TargetFramework (singular), because in that case none of our targets files are imported.

Example binlog: dotnet.binlog.zip

For comparison here's a buildlog using TargetFramework instead: dotnet-targetframework.binlog.zip

This basically means that we (Xamarin) can't work around it in this scenario, because none of our code is involved in the build (pack) process.

[rainersigwald]

unfortunately the workaround does not work when a project uses TargetFrameworks (plural) instead of TargetFramework (singular), because in that case none of our targets files are imported.

You should be able to hook into the buildCrossTargeting imports to get your code into that scenario, but #7564 looks fine to me.

msbuild/src/Tasks/Microsoft.Common.CrossTargeting.targets

Lines 20 to 23 in 12bf0a8

	<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.CrossTargeting.targets\ImportBefore\*.targets"
	Condition="'$(ImportByWildcardBeforeMicrosoftCommonCrossTargetingTargets)' == 'true' and exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.CrossTargeting.targets\ImportBefore')"/>
	<Import Project="$(CustomBeforeMicrosoftCommonCrossTargetingTargets)" Condition="'$(CustomBeforeMicrosoftCommonCrossTargetingTargets)' != '' and Exists('$(CustomBeforeMicrosoftCommonCrossTargetingTargets)')"/>

[rolfbjarne]

You should be able to hook into the buildCrossTargeting imports to get your code into that scenario

You mean we could add a targets file to the CustomBeforeMicrosoftCommonCrossTargetingTargets property? Not sure where we could do that, because nothing from our iOS workload is loaded.

[rainersigwald]

That's what I expected but I might be missing something in the workload implementation.

Do you have a timeframe for when you need the fix?

[rolfbjarne]

Do you have a timeframe for when you need the fix?

The sooner the better of course, but for now it's only been reported internally, so I'm unsure how many customers would be affected. Unless somebody else runs into it, I wouldn't backport this anywhere, and just let it flow to a stable release. I'm guessing that would be with .NET 7 in the fall?

Also we have a workaround available, so nobody should be blocked by it.

[rainersigwald]

We can easily get it into 6.0.400, but it would be very difficult to get it into 6.0.300 at this point.

[rolfbjarne]

6.0.400 should work just fine!

[mattjohnsonpint]

A workaround if you don't want to use 6.0.400 while it's in preview:

<Target Name="RemoveInvalidManifest" AfterTargets="BuiltProjectOutputGroup">
  <ItemGroup Condition="!Exists('$(OutDir)$(_DeploymentTargetApplicationManifestFileName)')">
    <BuiltProjectOutputGroupOutput Remove="$([System.IO.Path]::GetFullPath('$(OutDir)$(_DeploymentTargetApplicationManifestFileName)'))" />
  </ItemGroup>
</Target>

This applies the same effect as was done in #7564, and works for me.