dotnet / project-system

The .NET Project System for Visual Studio
MIT License
969 stars 387 forks source link

All-configs property settings should be unconditional #1388

Open rainersigwald opened 7 years ago

rainersigwald commented 7 years ago

I created a new project multitargeted to net46;netstandard1.3. I went to project properties -> Build, selected "All Configurations" and "All Platforms", and checked the "Allow unsafe code" box. Instead of a simple unconditional addition of the property

<AllowUnsafeBlocks>True</AllowUnsafeBlocks>

in the existing unconditional PropertyGroup, the project file change is

+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net46|AnyCPU'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net46|AnyCPU'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|netstandard1.3|AnyCPU'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|netstandard1.3|AnyCPU'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net46|x86'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net46|x64'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net46|x86'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net46|x64'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|netstandard1.3|x86'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|netstandard1.3|x64'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|netstandard1.3|x86'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|netstandard1.3|x64'">
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>

The former is both more succinct and easier to later extend to new configurations.

srivatsn commented 7 years ago

Ya we should fix this. This is the behavior that the property pages code has had forever and changing it is risky since it affects all projects so not for RTM but we should fix it soon after.

RaulPerez1 commented 7 years ago

See the comment I made here #1300 which also falls into this bucket

davkean commented 7 years ago

@RaulPerez1 With the new config changes, what's the behavior?

RaulPerez1 commented 7 years ago

Behavior will be the same, this is independent of how you handle configurations. This is controlled via the rules for the property pages with the HasConfigurationCondition value. Anything that has it set to true will end up writing conditioned values and it looks from the example above that it's giving you the full matrix of properties for that one if it's defaulted (which is what I would expect).

davkean commented 7 years ago

Okay, I agree it would be nice to do better here.