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.72k stars 1.07k forks source link

Default platform target in Properties window is incorrect #1560

Open davkean opened 7 years ago

davkean commented 7 years ago

From @davidmatson on August 28, 2017 19:40

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net47</TargetFramework>
  </PropertyGroup>
</Project>
using System;
using System.Reflection;

class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine(Environment.Is64BitProcess);
        ImageFileMachine b;
        typeof(Program).Assembly.ManifestModule.GetPEKind(out PortableExecutableKinds a, out b);
        Console.WriteLine($"{a}: {b}");
    }
}

Actual output:

True
ILOnly: I386

But, properties window shows: image

Copied from original issue: dotnet/project-system#2744

davkean commented 7 years ago

From @davidmatson on August 28, 2017 19:42

Just to clarify, the output is fine an expected for AnyCPU without Prefer 32-bit, which apparently is actually the default for this kind of project. The only problem is what the properties window shows - if the default is actually AnyCPU without Prefer 32-bit, the UI should say so.

davkean commented 7 years ago

From @tannergooding on August 28, 2017 19:43

Could you please indicate which version of Visual Studio you are using and which versions of the SDK you have installed (both 64-bit and 32-bit versions).

davkean commented 7 years ago

From @davidmatson on August 28, 2017 19:44

Visual Studio Enterprise 2017 Version 15.3.2.

I didn't do anything special to install an SDK as far as I know - what SDK version would you like and were should I find that value?

davkean commented 7 years ago

From @tannergooding on August 28, 2017 20:5

If you didn't do anything specific to install the SDK it is probably 1.1.2.

You can see which versions you have installed (generally) by looking here: C:\Program Files\dotnet\sdk (for x64) and here: C:\Program Files (x86)\dotnet\sdk (for x86)

You should be able to look at your bin\*.deps.json file to see what version you are using (should have a line similar to "name": ".NETCoreApp,Version=v2.0".

Is this a project you can share the detailed or diagnostic verbosity 'build log' for (that will include all of the referenced paths which should explicitly indicate the full path of the SDK you are using)?

davkean commented 7 years ago

From @tannergooding on August 28, 2017 20:8

Actually, I seem to be able to repro this on the 2.0 SDK. It seems to occur whenever you single-target net* (net45, net46, net47, etc).

Going to keep looking

davkean commented 7 years ago

From @davidmatson on August 28, 2017 20:18

I have the following SDK folders:

I didn't find anything except the exe and pdb in my bin folder, but it sounds like you probably have what you need. Let me know if I can provide any more data though. Thanks!

davkean commented 7 years ago

From @tannergooding on August 28, 2017 20:46

It comes down to this line: https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.RuntimeIdentifierInference.targets#L65

Going to see if I can find the relevant people to tag here.

davkean commented 7 years ago

From @tannergooding on August 28, 2017 21:0

Ok, talked to some of the members on @dotnet/dotnet-cli and they indicated that this is 'by design'.

From what I understood, when targeting AnyCPU for .NETFramework, they have to pick a default RuntimeIdentifier for NuGet and use that. They choose x86 if not specific since that is the most common target for .NET Framework. If they find any native dependencies they do a fixup of the properties later.

There is one bug here, on the project system, that we should detect this and display the correct value in the properties window.

There is potentially a separate bug on the SDK that they should just be using win7 if the Platform is AnyCPU and should just fail (either at restore, build time, or runtime) if the user is targeting AnyCPU and is not referencing native dependencies in a portable manner (such as dynamically choosing to load the 64-bit library on x64 and the x86 library on x86) as they are inherently broken otherwise (they target AnyCPU and are referencing a 32-bit specific DLL, which can potentially cause breaks at runtime if they happen to run as 64-bit).

davkean commented 7 years ago

From @tannergooding on August 28, 2017 21:1

You can workaround the issue for the time being by explicitly specifying <PlatformTarget>AnyCPU</PlatformTarget> in your project file.

davkean commented 7 years ago

From @tannergooding on August 28, 2017 21:1

FYI. @Pilchie

davkean commented 7 years ago

From @tannergooding on August 28, 2017 21:5

FYI. @natidea as well

davkean commented 7 years ago

From @davidmatson on August 28, 2017 21:10

Thanks - yeah, I figured some combination of properties would cover a workaround.

The project system bug is that it should display Any CPU as Platform target in the project properties window in this case, right?

And the potential SDK bug is to use win7 as the default RuntimeIdentifier (i.e. PlatformTarget is implicitly AnyCPU)? That sounds right to me - it's quite odd to have Any CPU be treated as x86. Do we need to file another issue on a SDK repo to track that part?

Thanks!

davkean commented 7 years ago

From @tannergooding on August 28, 2017 21:14

Do we need to file another issue on a SDK repo to track that part?

It couldn't hurt to log one. Worst case is they close it as 'by design' and say we can't change it due to back-compat now.

davkean commented 7 years ago

We're just showing what the build tells us is the PlatformTarget, we shouldn't do anything special here - the SDK should be giving us the correct value for it.

davkean commented 7 years ago

If you look at build, you can clearly see that PlatformTarget is being set to x86:

1>Platform                       = AnyCPU
1>PlatformName                   = AnyCPU
1>Platforms                      = AnyCPU
1>PlatformTarget                 = x86
1>PlatformTargetAsMSBuildArchitecture = x86

This is an SDK bug.

am11 commented 7 years ago

@davkean, based on last comment, I was able to make the platform target set to x64 by explicitly setting all platform-like properties for net462 xunit project:

  <PropertyGroup>
    <TargetFramework>net462</TargetFramework>

    <!-- ideally we should only specify PlatformTarget like old csproj -->
    <!-- upstream issue: https://github.com/dotnet/sdk/issues/1560 -->
    <Platform>x64</Platform>
    <Platforms>$(Platform)</Platforms>
    <PlatformName>$(Platform)</PlatformName>
    <PlatformTarget>$(Platform)</PlatformTarget>
  </PropertyGroup>

then in SLN, added x64 configurations like this:

Global
  GlobalSection(ProjectConfigurationPlatforms) = postSolution
    {GUID}.Debug|x64.ActiveCfg = Debug|x64
    {GUID}.Debug|x64.Build.0 = Debug|x64
    {GUID}.Release|x64.ActiveCfg = Release|x64
    {GUID}.Release|x64.Build.0 = Release|x64
  EndGlobalSection
EndGlobal

After that, property page shows only one option:

Platform: Active (x64)

(which was the desired behavior)

nguerrera commented 6 years ago

We might need to change how the project system queries the PlatformTarget in order to show the correct value. The way it is selected automatically depends on package asset resolution targets, not pure evaluation. We could switch it to appear as "Any CPU" statically and "x86" dynamically (we do the reverse now), but that would also be incorrect if there are native nuget dependencies. In hindsight, I wish that it were an error to pull in native nuget dependencies without specifying a RID or platform target, but people were against that in v1.

davidmatson commented 6 years ago

I think the RID thing is a bit of a problem, even if it was handled otherwise in v1 - per #1545, it breaks what "AnyCPU" has always meant in .NET, right?

nguerrera commented 6 years ago

If you explicitly say AnyCPU, you get AnyCPU. It is only if you have native dependencies and no explicit AnyCPU that you get x86. However, it renders in the project system as x86 in this case due to implementation details.

am11 commented 6 years ago

Can the default (fallback) platform be x64 on x64 system and x86 on x86 system?

davidmatson commented 6 years ago

@nguerrera

If you explicitly say AnyCPU, you get AnyCPU. It is only if you have native dependencies and no explicit AnyCPU that you get x86. ...

In the second case, you don't actually get an x86 binary - according to Module.GetPEKind, it's still an AnyCPU assembly. For package restore purposes, it's treated as if it were going to be x86, but that's wrong - it's AnyCPU, and different machines will run it differently (it'll run as x86 on an x86 machine and as x64 on an x64 machine).

I think #1545 states this more serious aspect much more clearly - can we re-open that one? I'm much less concerned about what the property window shows than I am the fact that the binary is compiled one way but package restored another way.

davidmatson commented 6 years ago

Regarding treating these issues separately: I think different resolutions may make sense here for these two bugs.

For example, for resolving #1560, we might decide that for the properties window, the solution is just to display the value "AnyCPU" rather than "x86" but change nothing else - that solves the problem of the Default platform target displayed being incorrect but doesn't address the other bug.

For #1545, we might decide one of two things: a) if a binary is compiled as AnyCPU, we package restore that way regardless of whether it got that setting implicitly or explicitly; or b) We change the default PlatformTarget to be AnyCPU only if there's no native dependency; if there is a native dependency, we fully have the default PlatformTarget be x86 and compile the assembly that way (rather than as AnyCPU).

(So I agree the problems are related, but I think they are distinct problems with possibly very different resolutions.)

nguerrera commented 6 years ago

I've reopened the other bug. As I said there, whoever works on either one should consider the other.

ahdung commented 3 years ago

any news? VS2019 still here

marcpopMSFT commented 3 years ago

@sfoslund @melytc can ya'll chat to see how we might fix this? Daniel believes that we're setting the platform and then setting it back to AnyCPU after which may be confusing the property page.

sfoslund commented 3 years ago

@melytc correct me if I'm wrong but it looks like this isn't an issue with the new property pages anymore since the platform target does not display a default when there is no value in the project file. Is this intended and if so is there any action needed here?

ps-weber commented 2 years ago

I know this is just a UI bug, but I wasted some time trying to figure out what is going on. It would be great if this could be fixed.

ddelkhosh commented 2 years ago

I know this is just a UI bug, but I wasted some time trying to figure out what is going on. It would be great if this could be fixed.

This solution works for me: PlatformTarget: AnyCPU

The problem is that in Visual Studio AnyCPU is displayed, but in the .csproj file there is no information for "Platform Target".

Varorbc commented 1 year ago

any update?

TomasMalecek commented 2 months ago

When we migrated our .NET Framework 4.7.2 Windows Service project from legacy to SDK-style projects, we inadvertently removed the <PlatformTarget>x64</PlatformTarget> and the process started running as 32b. We needed to add the explicit PlatformTarget setting back to fix that. Explicit AnyCPU also works. Is that a consequence of this .NET SDK issue?

The project looked mostly like this (plus some fluff about App.config and transformation using TransformXml task from Microsoft.Web.Publishing.Tasks.dll, ProjectReferences, and other irrelevant stuff):

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net472</TargetFramework>
    <OutputType>Exe</OutputType>
  </PropertyGroup>

  <ItemGroup>
    <Reference Include="System.ServiceProcess" />
  </ItemGroup>

</Project>