-
Notifications
You must be signed in to change notification settings - Fork 554
[msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes #4235 and #4237. #4512
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
Conversation
…ixes dotnet#4235 and dotnet#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 dotnet#4235. Fixes dotnet#4237.
|
Is there anybody else knowledgable about nugets who can verify that this fix makes sense? |
|
@VincentDondain please have a look since you did the original fix. |
|
Build failure Test results1 tests failed, 0 tests skipped, 81 tests passed.Failed tests
|
…ckPathsOverride 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 dotnet#4111 is merged.
|
Build failure Test results1 tests failed, 0 tests skipped, 81 tests passed.Failed tests
|
|
The new test fails because it requires MSBuild, and MSBuild picks up some assemblies from the system XI, not the locally built one. IOW the new tests in this PR requires PR #4111, so let's wait a bit to see if we can get that completed. |
|
@radical might be a good set of eyes here. Do we need this hack for Xamarin.Mac? |
Yeah, probably, I'll test it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me! Thanks for the test, that's what I wanted to write in my initial PR but I too got blocked by #4111.
|
I've added a XM test showing that somehow (?!?) we don't need a fix for macOS. This is more than a bit surprising, and I pinged @radical for help understanding for what's going on. I've pushed my test to the PR, please review and see what i'm missing. |
|
Build failure Test results3 tests failed, 0 tests skipped, 215 tests passed.Failed tests
|
|
I've hopefully un-failed at reading comprehension and added a test that actually uses XM extensions, which still doesn't need a fix (?) |
| { | ||
| string testPath = Path.Combine (TI.FindSourceDirectory (), @"Today/TodayExtensionTest.csproj"); | ||
| TI.BuildProject (testPath, isUnified: true); | ||
| TI.CopyDirectory (Path.Combine (TI.FindSourceDirectory (), @"Today"), tmpDir); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extension tests were building in place, which was lazy and wrong of me. Let's copy them to temp.
We also need this since we added a variable to substitute in today.
|
It looks like we're failing on nuget restore, (due to not having a way to redirect msbuild?). In any case, I'll disable the tests for now :( |
|
Build failure Test results# Test run in progress: Built: 23, Succeeded: 193, Failed: 2, Ignored: 1131Failed tests
|
|
build |
|
Build failure Test results1 tests failed, 0 tests skipped, 217 tests passed.Failed tests
|
|
Would the new-ish Microsoft.Build.Locator package help with the MSBuild redirection? /cc @rainersigwald |
|
@chamons can you disable the MSBuild/iOS test so we can merge the PR ? |
|
We already have #4110 open which I refer'ed to in each comment out. When we fix that, we'll have to search for that (along with the original bugzilla number) and comment in a bunch of tests. I also merged master to fix the merge conflict. |
|
Build failure Test resultsTest run in progress: InProgress, Waiting: 1, Built: 145, Running: 1, RunQueued: 49, Succeeded: 22, Ignored: 1131 |
|
failure due to disk space |
|
build |
|
Build failure Test results# Test run in progress: Built: 113, Succeeded: 101, Failed: 1, Ignored: 1131, Crashed: 3Failed tests
|
|
build |
|
Build failure Test results1 tests failed, 0 tests skipped, 217 tests passed.Failed tests
|
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
|
build |
|
Build failure Test results1 tests failed, 0 tests skipped, 217 tests passed.Failed tests
|
|
Test failure is unrelated (https://github.com/xamarin/maccore/issues/827, test actually succeeded, the reporting of it failed). |
…ixes dotnet#4235 and dotnet#4237. (dotnet#4512) * [msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes dotnet#4235 and dotnet#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 dotnet#4235. Fixes dotnet#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 dotnet#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 dotnet#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 dotnet#4235 and dotnet#4237. (dotnet#4512) * [msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes dotnet#4235 and dotnet#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 dotnet#4235. Fixes dotnet#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 dotnet#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 dotnet#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 #4235 and #4237. (#4512) (#4648) * [msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes #4235 and #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 #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
…ixes dotnet#4235 and dotnet#4237. (dotnet#4512) (dotnet#4648) * [msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes dotnet#4235 and dotnet#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 dotnet#4235. Fixes dotnet#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 dotnet#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 dotnet#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
|
@monojenkins backport to xcode10 |
…ixes dotnet#4235 and dotnet#4237. (dotnet#4512) (dotnet#4648) * [msbuild] Set 'CopyNuGetImplementations' to true for app extensions. Fixes dotnet#4235 and dotnet#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 dotnet#4235. Fixes dotnet#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 dotnet#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 dotnet#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 #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
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.