search

dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.68k stars 1.06k forks source link

Cannot publish core console app + core app library tests #1834

Open orloffm opened 6 years ago

orloffm commented 6 years ago

The solution is here: https://github.com/orloffm/failingpublish

The first project is a netcoreapp2.0 exe, the second is a test netcoreapp2.0 project for it. The solution dotnet builds, but when I do

dotnet publish -c Release --self-contained -r win-x64

I get the following:

Microsoft (R) Build Engine version 15.5.179.9764 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 43.37 ms for C:\dev\failing_build\a\a.csproj.
  Restore completed in 75.49 ms for C:\dev\failing_build\a.tests\a.tests.csproj.
C:\Program Files\dotnet\sdk\2.1.3\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.RuntimeIdentifierInference.targets(116,5): error : It is not supported to build or publish a self-contained application without specifying a RuntimeIdentifier.  Please either specify a RuntimeIdentifier or set SelfContained to false. [C:\dev\failing_build\a\a.csproj]
  a -> C:\dev\failing_build\a\bin\Release\netcoreapp2.0\win-x64\a.dll
  a -> C:\dev\failing_build\a\bin\Release\netcoreapp2.0\win-x64\publish\

If I remove the test project from the solution, it works fine:

Microsoft (R) Build Engine version 15.5.179.9764 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 43.62 ms for C:\dev\failing_build\a\a.csproj.
  a -> C:\dev\failing_build\a\bin\Release\netcoreapp2.0\win-x64\a.dll
  a -> C:\dev\failing_build\a\bin\Release\netcoreapp2.0\win-x64\publish\

I expect it to work, as this is almost a copy of a https://github.com/dotnet/docs/tree/master/samples/core/getting-started/unit-testing-using-dotnet-test, but with the main project switched to netcoreapp2.0.

dasMulli commented 6 years ago

Are you using dotnet publish on the solution or a single project? I'm not sure if it is supposed to work on a solution..

As a workaround, you could try to omit the --self-contained parameter, since it is on by default. However, I believe the underlying issue would also affect any self-contained app that references another application since the build of a project reference will remove a global RuntimeIdentifier but not a SelfConained property..

orloffm commented 6 years ago

I just do it in the solution folder. Yes, I understand I can specify the project, but the message is at least misleading if it depends on the presence of test projects.

dasMulli commented 6 years ago

As suspected, the issue also occurs when an app references another app. selfcontainedrepro.zip also fails in the same way when the testmainapp is published with dotnet publish -r osx-x64 --self-contained.

This likely needs a change to MSBuild similar to https://github.com/Microsoft/msbuild/pull/1674 to remove the SelfContained global property for RID-agnostic P2Ps. (Or maybe for even for RID-specific ones?). cc @nguerrera @livarcocc

alexzaytsev-newsroomly commented 6 years ago

This seems to still affect the 2.1 SDK

dasMulli commented 6 years ago

@peterhuene @dsplaisted could you make a change similar to https://github.com/Microsoft/msbuild/pull/1674 to remove SelfContained as well as the new app host property as global properties for building P2P references? If no one is actively working on it (@peterhuene is assigned) I can give it a go myself..

dasMulli commented 6 years ago

@alexzaytsev-newsroomly @orloffm sry forgot to post the workaround half a year ago:

The workaround is to change the project reference to exclude the property:

<ItemGroup>
  <ProjectReference Include="..\other\app.csproj"
                    GlobalPropertiesToRemove="SelfContained" />
</ItemGroup>

At the moment, you may also get an "was restored using 2.1.1 but … 2.1.0 … used insead" error. To work around this one, add this to the referenced(!) project(s):

<PropertyGroup>
  <TargetLatestRuntimePatch>true</TargetLatestRuntimePatch>
</PropertyGroup>
peterhuene commented 6 years ago

Thanks @dasMulli. I'm going to try to get this resolved for 2.1.4xx this week.

dasMulli commented 6 years ago

I'm wondering if there can be a way to do these kind of things without hardcoding these properties in msbuild.. like a target that modifies the GlobalPropertiesToRemove before msbuild's PrepareProjectReferences or a variable in msbuild that the sdk could set. or an additional ItemDefinitionGroup etc.

peterhuene commented 6 years ago

That's definitely something that I can bring up with the MSBuild team. I'd certainly prefer not to have to modify their targets for SDK properties.

nguerrera commented 6 years ago

@peterhuene @dasMulli, +@rainersigwald

We discussed this back in https://github.com/dotnet/sdk/pull/993/files/6eaa5f97e01d3ac55a1a15961c20f61d90b035bd#diff-e1389449b5e96849141625fcaa2713d6 and I forget why it was abandoned now. :(

Things changed quite a bit since, mostly for the double evaluation fix. Unfortunately, this stuff gets quite complicated.

peterhuene commented 6 years ago

Unfortunately, this is not so simple a fix as to not propagate a global SelfContained property to referenced projects. Even if we fix it such that the property isn't propagated, a larger issue is exposed: what we restore for referenced netcoreapp* targeted Exe projects differs from how they build or publish due to the new "target latest runtime patch level" feature.

The TargetLatestRuntimePatch property is keyed off of SelfContained, which itself is keyed off of RuntimeIdentifier. RuntimeIdentifier does not propagate to the referenced projects by (current) design. As a result, if SelfContained is propagated, we get the expected error originally mentioned in this issue.

However, if SelfContained is not propagated as a global property, then TargetLatestRuntimePatch defaults to false (because RuntimeIdentifier did not propagate, and hence SelfContained defaults to false) and the build fails because the restore was against the latest runtime patch level, but the build operation was not (the NuGet error mentioned above by @dasMulli).

I don't know if we want to fix SelfContained propagation without having a fix for restoring / building for the appropriate patch level since the two are very much related. There are currently several issues tracking problems related to the patch level feature, as well.

For now, there are workarounds such as what's listed here and in https://github.com/dotnet/sdk/issues/2364. I'm going to move this to 2.2.1xx to see if we an make an appropriate, all-encompassing fix.

dasMulli commented 6 years ago

Making NuGet and MSBuild use the same properties for P2Ps would be great, I remember seeing this in https://github.com/dotnet/cli/issues/9544#issuecomment-399674198

NuGet does <GetRestoreProjectReferencesTask …ProjectReferences="@(ProjectReference)"> while MSBuild has logic to carefully unset some global properties (PrepareProjectReferences> @(AnnotatedProjects)), including RuntimeIdentifier. I believe that NuGet and MSBuild evaluation will then see a different list of global properties. Is that correct?

For -r RID this always worked since NuGet always creates a non-RID dependency graph in addition to the RID-specific one. But evaluation-wise, this will now become interesting with the plural RuntimeIdentifiers property which will need additional properties being set to restore only once for multiple RIDs and then publish for them. Maybe this will need some doc changes - https://docs.microsoft.com/en-us/dotnet/core/deploying/deploy-with-cli#simpleSelf

tkstanczak commented 4 years ago

we are experiencing the same issue when trying to publish a project that is used to execute tests: image