Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes #4235 and #4237. (#4512) #4648

Merged
merged 1 commit into from
Aug 23, 2018

Conversation

rolfbjarne
Copy link
Member

This is a backport of #4512.

In Xamarin.iOS.Common.targets, just before the _CompileToNative target, we
modify the mtouch references to ensure that we get the lib assemblies for
nugets, and not the ref references:

<ItemGroup>
<!-- Skip the reference assemblies. There is currently no scenario where it is valid to pass them to 'mtouch'. They are impossible to AOT. Fixes: https://github.com/xamarin/xamarin-macios/issues/3199 -->
<!-- Remove exact references, such as if a package had a framework reference to 'System' that we already have -->
<!-- This is exactly what "Microsoft.NuGet.Build.Tasks"'s 'ResolveNuGetPackageAssets' target is doing -->
<!-- Effectively 'ResolveNuGetPackageAssets' computes the NuGet packages using their json asset file, adds the full assemblies to 'ReferenceCopyLocalPaths' and outputs the resolved references via '_ReferencesFromNuGetPackages' -->
<ReferencePath Remove="@(_ReferencesFromNuGetPackages)" />
<MTouchReferencePath Include="@(ReferenceCopyLocalPaths)" Condition="'%(Extension)' == '.dll'" />
</ItemGroup>

This logic removes nuget references, and then re-adds any copy-local dll
references.

This works fine in executable projects, but not in library projects (aka
extensions), because nugets aren't copied for library projects:

https://github.com/NuGet/NuGet.BuildTasks/blob/cf4b0a12cf1f75e0654f28c2a9020251c41d126a/src/Microsoft.NuGet.Build.Tasks/Microsoft.NuGet.targets#L86

So we need to set the CopyNuGetImplementations variable to 'true' for our
library projects.

Fixes #4235.
Fixes #4237.

  • [tests] Redirect MSBuildExtensionsPath to MSBuildExtensionsPathFallbackPathsOverride when running msbuild for package reference tests.

This fixes a problem where nuget restore would fail for projects with
PackageReferences, because a variable would be empty and msbould would try to
write to /:

nuget restore ../MyAppWithPackageReference/MyAppWithPackageReference.csproj
MSBuild auto-detection: using msbuild version '15.0' from '/Library/Frameworks/Mono.framework/Versions/Current/lib/mono/msbuild/15.0/bin/'.
Restoring packages for /Users/builder/jenkins/workspace/xamarin-macios-pr-builder/msbuild/tests/MyAppWithPackageReference/MyAppWithPackageReference.csproj...
Committing restore...
Generating MSBuild file /MyAppWithPackageReference.csproj.nuget.g.props.
Path / is a directory

This will become unnecessary when PR #4111 is merged.

  • Add Xamarin.Mac test showing that fix is not needed (?!?)

  • Add AppExtension test with packagereference

  • Make extension actually have json code generated

  • Fix ProjectTypeGuids of checked in extension projects, as they were not openable in VSfM

  • XM extension test now correctly fails

  • Now that we have a failing test, fix XM same as rest of platforms

  • Disable XM tests due to msbuild redirect sadness

  • Disable iOS tests as well due to msbuild-mac tests should be running with msbuild in addition to xbuild #4110

  • Disable iOS tests by using the Ignore attribute.

Disable tests by using the Ignore attribute, because just commenting out the
TestCase attributes makes the test fail:

1) NotRunnable : Xamarin.iOS.Tasks.ProjectReferenceTests.BasicTest
   No suitable constructor was found

…ixes xamarin#4235 and xamarin#4237. (xamarin#4512)

* [msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes xamarin#4235 and xamarin#4237.

In Xamarin.iOS.Common.targets, just before the _CompileToNative target, we
modify the mtouch references to ensure that we get the lib assemblies for
nugets, and not the ref references:

https://github.com/xamarin/xamarin-macios/blob/9e31d07ecc08a64372dd562e843c3d8950d24985/msbuild/Xamarin.iOS.Tasks.Core/Xamarin.iOS.Common.targets#L784-L791

This logic removes nuget references, and then re-adds any copy-local dll
references.

This works fine in executable projects, but not in library projects (aka
extensions), because nugets aren't copied for library projects:

https://github.com/NuGet/NuGet.BuildTasks/blob/cf4b0a12cf1f75e0654f28c2a9020251c41d126a/src/Microsoft.NuGet.Build.Tasks/Microsoft.NuGet.targets#L86

So we need to set the CopyNuGetImplementations variable to 'true' for our
library projects.

Fixes xamarin#4235.
Fixes xamarin#4237.

* [tests] Redirect MSBuildExtensionsPath to MSBuildExtensionsPathFallbackPathsOverride when running msbuild for package reference tests.

This fixes a problem where nuget restore would fail for projects with
PackageReferences, because a variable would be empty and msbould would try to
write to /:

    nuget restore ../MyAppWithPackageReference/MyAppWithPackageReference.csproj
    MSBuild auto-detection: using msbuild version '15.0' from '/Library/Frameworks/Mono.framework/Versions/Current/lib/mono/msbuild/15.0/bin/'.
    Restoring packages for /Users/builder/jenkins/workspace/xamarin-macios-pr-builder/msbuild/tests/MyAppWithPackageReference/MyAppWithPackageReference.csproj...
    Committing restore...
    Generating MSBuild file /MyAppWithPackageReference.csproj.nuget.g.props.
    Path / is a directory

This will become unnecessary when PR xamarin#4111 is merged.

* Add Xamarin.Mac test showing that fix is not needed (?!?)

* Add AppExtension test with packagereference

* Make extension actually have json code generated

* Fix ProjectTypeGuids of checked in extension projects, as they were not openable in VSfM

* XM extension test now correctly fails

* Now that we have a failing test, fix XM same as rest of platforms

* Disable XM tests due to msbuild redirect sadness

* Disable iOS tests as well due to xamarin#4110

* Disable iOS tests by using the Ignore attribute.

Disable tests by using the Ignore attribute, because just commenting out the
TestCase attributes makes the test fail:

    1) NotRunnable : Xamarin.iOS.Tasks.ProjectReferenceTests.BasicTest
       No suitable constructor was found
@rolfbjarne rolfbjarne added this to the d15-9 milestone Aug 20, 2018
@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
Generator Diff (only version changes)
Test run succeeded

@spouliot spouliot merged commit 5a2a10d into xamarin:d15-9 Aug 23, 2018
spouliot pushed a commit to spouliot/xamarin-macios that referenced this pull request Aug 24, 2018
…ixes xamarin#4235 and xamarin#4237. (xamarin#4512) (xamarin#4648)

* [msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes xamarin#4235 and xamarin#4237.

In Xamarin.iOS.Common.targets, just before the _CompileToNative target, we
modify the mtouch references to ensure that we get the lib assemblies for
nugets, and not the ref references:

https://github.com/xamarin/xamarin-macios/blob/9e31d07ecc08a64372dd562e843c3d8950d24985/msbuild/Xamarin.iOS.Tasks.Core/Xamarin.iOS.Common.targets#L784-L791

This logic removes nuget references, and then re-adds any copy-local dll
references.

This works fine in executable projects, but not in library projects (aka
extensions), because nugets aren't copied for library projects:

https://github.com/NuGet/NuGet.BuildTasks/blob/cf4b0a12cf1f75e0654f28c2a9020251c41d126a/src/Microsoft.NuGet.Build.Tasks/Microsoft.NuGet.targets#L86

So we need to set the CopyNuGetImplementations variable to 'true' for our
library projects.

Fixes xamarin#4235.
Fixes xamarin#4237.

* [tests] Redirect MSBuildExtensionsPath to MSBuildExtensionsPathFallbackPathsOverride when running msbuild for package reference tests.

This fixes a problem where nuget restore would fail for projects with
PackageReferences, because a variable would be empty and msbould would try to
write to /:

    nuget restore ../MyAppWithPackageReference/MyAppWithPackageReference.csproj
    MSBuild auto-detection: using msbuild version '15.0' from '/Library/Frameworks/Mono.framework/Versions/Current/lib/mono/msbuild/15.0/bin/'.
    Restoring packages for /Users/builder/jenkins/workspace/xamarin-macios-pr-builder/msbuild/tests/MyAppWithPackageReference/MyAppWithPackageReference.csproj...
    Committing restore...
    Generating MSBuild file /MyAppWithPackageReference.csproj.nuget.g.props.
    Path / is a directory

This will become unnecessary when PR xamarin#4111 is merged.

* Add Xamarin.Mac test showing that fix is not needed (?!?)

* Add AppExtension test with packagereference

* Make extension actually have json code generated

* Fix ProjectTypeGuids of checked in extension projects, as they were not openable in VSfM

* XM extension test now correctly fails

* Now that we have a failing test, fix XM same as rest of platforms

* Disable XM tests due to msbuild redirect sadness

* Disable iOS tests as well due to xamarin#4110

* Disable iOS tests by using the Ignore attribute.

Disable tests by using the Ignore attribute, because just commenting out the
TestCase attributes makes the test fail:

    1) NotRunnable : Xamarin.iOS.Tasks.ProjectReferenceTests.BasicTest
       No suitable constructor was found
spouliot pushed a commit to spouliot/xamarin-macios that referenced this pull request Sep 7, 2018
…ixes xamarin#4235 and xamarin#4237. (xamarin#4512) (xamarin#4648)

* [msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes xamarin#4235 and xamarin#4237.

In Xamarin.iOS.Common.targets, just before the _CompileToNative target, we
modify the mtouch references to ensure that we get the lib assemblies for
nugets, and not the ref references:

https://github.com/xamarin/xamarin-macios/blob/9e31d07ecc08a64372dd562e843c3d8950d24985/msbuild/Xamarin.iOS.Tasks.Core/Xamarin.iOS.Common.targets#L784-L791

This logic removes nuget references, and then re-adds any copy-local dll
references.

This works fine in executable projects, but not in library projects (aka
extensions), because nugets aren't copied for library projects:

https://github.com/NuGet/NuGet.BuildTasks/blob/cf4b0a12cf1f75e0654f28c2a9020251c41d126a/src/Microsoft.NuGet.Build.Tasks/Microsoft.NuGet.targets#L86

So we need to set the CopyNuGetImplementations variable to 'true' for our
library projects.

Fixes xamarin#4235.
Fixes xamarin#4237.

* [tests] Redirect MSBuildExtensionsPath to MSBuildExtensionsPathFallbackPathsOverride when running msbuild for package reference tests.

This fixes a problem where nuget restore would fail for projects with
PackageReferences, because a variable would be empty and msbould would try to
write to /:

    nuget restore ../MyAppWithPackageReference/MyAppWithPackageReference.csproj
    MSBuild auto-detection: using msbuild version '15.0' from '/Library/Frameworks/Mono.framework/Versions/Current/lib/mono/msbuild/15.0/bin/'.
    Restoring packages for /Users/builder/jenkins/workspace/xamarin-macios-pr-builder/msbuild/tests/MyAppWithPackageReference/MyAppWithPackageReference.csproj...
    Committing restore...
    Generating MSBuild file /MyAppWithPackageReference.csproj.nuget.g.props.
    Path / is a directory

This will become unnecessary when PR xamarin#4111 is merged.

* Add Xamarin.Mac test showing that fix is not needed (?!?)

* Add AppExtension test with packagereference

* Make extension actually have json code generated

* Fix ProjectTypeGuids of checked in extension projects, as they were not openable in VSfM

* XM extension test now correctly fails

* Now that we have a failing test, fix XM same as rest of platforms

* Disable XM tests due to msbuild redirect sadness

* Disable iOS tests as well due to xamarin#4110

* Disable iOS tests by using the Ignore attribute.

Disable tests by using the Ignore attribute, because just commenting out the
TestCase attributes makes the test fail:

    1) NotRunnable : Xamarin.iOS.Tasks.ProjectReferenceTests.BasicTest
       No suitable constructor was found
spouliot added a commit that referenced this pull request Sep 10, 2018
…ixes #4235 and #4237. (#4512) (#4648) (#4773)

In Xamarin.iOS.Common.targets, just before the _CompileToNative target, we
modify the mtouch references to ensure that we get the lib assemblies for
nugets, and not the ref references:

https://github.com/xamarin/xamarin-macios/blob/9e31d07ecc08a64372dd562e843c3d8950d24985/msbuild/Xamarin.iOS.Tasks.Core/Xamarin.iOS.Common.targets#L784-L791

This logic removes nuget references, and then re-adds any copy-local dll
references.

This works fine in executable projects, but not in library projects (aka
extensions), because nugets aren't copied for library projects:

https://github.com/NuGet/NuGet.BuildTasks/blob/cf4b0a12cf1f75e0654f28c2a9020251c41d126a/src/Microsoft.NuGet.Build.Tasks/Microsoft.NuGet.targets#L86

So we need to set the CopyNuGetImplementations variable to 'true' for our
library projects.

Fixes #4235.
Fixes #4237.

* [tests] Redirect MSBuildExtensionsPath to MSBuildExtensionsPathFallbackPathsOverride when running msbuild for package reference tests.

This fixes a problem where nuget restore would fail for projects with
PackageReferences, because a variable would be empty and msbould would try to
write to /:

    nuget restore ../MyAppWithPackageReference/MyAppWithPackageReference.csproj
    MSBuild auto-detection: using msbuild version '15.0' from '/Library/Frameworks/Mono.framework/Versions/Current/lib/mono/msbuild/15.0/bin/'.
    Restoring packages for /Users/builder/jenkins/workspace/xamarin-macios-pr-builder/msbuild/tests/MyAppWithPackageReference/MyAppWithPackageReference.csproj...
    Committing restore...
    Generating MSBuild file /MyAppWithPackageReference.csproj.nuget.g.props.
    Path / is a directory

This will become unnecessary when PR #4111 is merged.

* Add Xamarin.Mac test showing that fix is not needed (?!?)

* Add AppExtension test with packagereference

* Make extension actually have json code generated

* Fix ProjectTypeGuids of checked in extension projects, as they were not openable in VSfM

* XM extension test now correctly fails

* Now that we have a failing test, fix XM same as rest of platforms

* Disable XM tests due to msbuild redirect sadness

* Disable iOS tests as well due to #4110

* Disable iOS tests by using the Ignore attribute.

Disable tests by using the Ignore attribute, because just commenting out the
TestCase attributes makes the test fail:

    1) NotRunnable : Xamarin.iOS.Tasks.ProjectReferenceTests.BasicTest
       No suitable constructor was found
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.

5 participants