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

Default platform target in Properties window is incorrect #1560

Open
davkean opened this Issue Sep 5, 2017 · 23 comments

Comments

Projects
None yet
5 participants
@davkean
Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

From @davidmatson on August 28, 2017 20:18

I have the following SDK folders:

  • 1.0.0
  • 1.0.2
  • 1.0.3
  • 1.0.4
  • 1.1.0
  • 2.0.0

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

From @tannergooding on August 28, 2017 21:1

FYI. @Pilchie

@davkean

This comment has been minimized.

Member

davkean commented Sep 5, 2017

From @tannergooding on August 28, 2017 21:5

FYI. @natidea as well

@davkean

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

Member

davkean commented Sep 5, 2017

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

This comment has been minimized.

am11 commented Sep 14, 2017

@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

This comment has been minimized.

Member

nguerrera commented Jan 25, 2018

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

This comment has been minimized.

davidmatson commented Jan 25, 2018

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

This comment has been minimized.

Member

nguerrera commented Jan 25, 2018

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

This comment has been minimized.

am11 commented Jan 26, 2018

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

@davidmatson

This comment has been minimized.

davidmatson commented Jan 26, 2018

@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

This comment has been minimized.

davidmatson commented Jan 26, 2018

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

This comment has been minimized.

Member

nguerrera commented Jan 26, 2018

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

@livarcocc livarcocc added this to the Unknown milestone Mar 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment