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

Regression: files missing from solution explorer with multi-targeting #2337

Closed
davkean opened this Issue Jun 15, 2018 · 27 comments

Comments

Projects
None yet
4 participants
@davkean
Member

davkean commented Jun 15, 2018

From @onovotny on June 15, 2018 0:22

This is broken with 15.7.3 and 15.8.2 p2 with .NET Core 2.1 installed. It used to work. I think that setting

<EnableDefaultCompileItems>false</EnableDefaultCompileItems> changed the behavior where files used to be visible.

The solution explorer is missing files that are specified by the include globs.

image

ProjectSystemRegression.zip

Note that the project builds correctly, but files aren't visible in VS.

A workaround requires adding
<None Include="**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> to the project file, but I don't believe that used to be necessary, thus the regression.

Here's the csproj:

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

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;Xamarin.iOS10;MonoAndroid81;uap10.0.16299</TargetFrameworks>
    
    <EnableDefaultCompileItems>false</EnableDefaultCompileItems>
  </PropertyGroup>
  
  <ItemGroup>
    <PackageReference Include="MSBuild.Sdk.Extras" Version="1.5.4" PrivateAssets="All" />
    
    <Compile Include="**\*.shared.cs" />
    <Compile Include="**\*.shared.*.cs" />
  </ItemGroup>
  <ItemGroup Condition=" $(TargetFramework.StartsWith('netstandard')) ">
    <Compile Include="**\*.netstandard.cs" />
    <Compile Include="**\*.netstandard.*.cs" />
  </ItemGroup>
  <ItemGroup Condition=" $(TargetFramework.StartsWith('uap10.0')) ">
    <PackageReference Include="Microsoft.NETCore.UniversalWindowsPlatform" Version="6.0.8" />
    <Compile Include="**\*.uwp.cs" />
    <Compile Include="**\*.uwp.*.cs" />
  </ItemGroup>
  <ItemGroup Condition=" $(TargetFramework.StartsWith('MonoAndroid')) ">
    <PackageReference Include="Xamarin.Android.Support.CustomTabs" Version="27.0.2" />
    <PackageReference Include="Xamarin.Android.Support.Core.Utils" Version="27.0.2" />
    <Compile Include="**\*.android.cs" />
    <Compile Include="**\*.android.*.cs" />
  </ItemGroup>
  <ItemGroup Condition=" $(TargetFramework.StartsWith('Xamarin.iOS')) ">
    <Compile Include="**\*.ios.cs" />
    <Compile Include="**\*.ios.*.cs" />
  </ItemGroup>
  <Import Project="$(MSBuildSDKExtrasTargets)" Condition="Exists('$(MSBuildSDKExtrasTargets)')" />

</Project>

What's interesting here is that there's some dynamic evaluation happening. It's not just *.shared.cs that's visible, but it matches on the first conditional item group as well and shows *.netstandard.cs. It does not show any of the subsequent ones though.

@jamesmontemagno

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

@davkean

This comment has been minimized.

Member

davkean commented Jun 15, 2018

Spoke to @jamesmontemagno yesterday about this, but he mentioned the issue he was running into was https://developercommunity.visualstudio.com/content/problem/268967/multi-targeting-project-with-conditionally-include.html, so clearly lead him down the wrong path.

This is #1157 & dotnet/project-system#3181 and was a deliberate change in the SDK.

@onovotny

This comment has been minimized.

Member

onovotny commented Jun 15, 2018

More weirdness -- it shows and picks up conditional that's the first tfm on the list. If I put the UWP one first, the close/reopen the solution, I see the following.

Also, if you copy/paste the file in the solution explorer, then rename it to match another item,

image

It wrongly adds a Compile item in the csproj:
image

Where that should already be covered. VS should not be touching the csproj file here.

@davkean

This comment has been minimized.

Member

davkean commented Jun 15, 2018

The tree is generated based on the configuration and has always been the case in the new project system. You can try this by placing a condition to only include a file in DEBUG and then switch to RELEASE, and we'll update the tree to remove the file. TFMs are considered a dimension in a configuration, you just can't change the "active" one.

I'm not sure I understand the erroneous entries what is the exact steps?

@onovotny

This comment has been minimized.

Member

onovotny commented Jun 15, 2018

If I add the <None Include="**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> in, and I repeat, I get other weird behavior:

I copy Class1.uwp.cs and rename it to .android.cs
image

@onovotny

This comment has been minimized.

Member

onovotny commented Jun 15, 2018

Something bad changed in the behavior though, perhaps due to the #1157, but this makes working with other tfm's other than the first, active one, impossible now.

@davkean

This comment has been minimized.

Member

davkean commented Jun 15, 2018

There has been zero changes in the way we handle this in VS, this is an SDK-only change. VS will go out of its way to avoid double-including an item, which has been the behavior since VS 2017 RTM.

File separate bugs for the VS behavior, with a clear "actual" and "expected" so I can understand your expectation.

@onovotny

This comment has been minimized.

Member

onovotny commented Jun 15, 2018

Exact repro steps from the repro zip I uploaded initially:

  1. Add <None Include="**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> to the first ItemGroup and hit save.
  2. Select Class1.netstandard.cs and Hit Ctrl+C then Ctrl+V. This adds the following to the csproj, incorrectly
    image
  3. Rename the pasted file from Class1 - Copy.netstandard.cs to Class1 - Copy.ios.cs. The csproj now shows this:
    image

Neither the Compile Include nor None Remove should be present after doing this.

@davkean

This comment has been minimized.

Member

davkean commented Jun 15, 2018

This behavior is exactly how it was designed and has been the same for ~2 years, under the following constraints:

  • New items will get a default item type; *.cs files will by default be under the <Compile /> item
  • Copying and pasting an item will copy the source's item type
  • Visual Studio only evaluates/manipulates a single configuration (and hence "single TFM") when modifying the project file
  • Visual Studio will go out of its way to prevent double-including a file under two different items (hence why it removed "Class1 - Copy.netstandard.cs" from None item, otherwise it would be included twice)

The behavior above can be described by the last two constraints. This behavior however is separate to the SDK change and hence, if you'd like to see a different behavior please file a new bug over http://github.com/dotnet/project-system.

@onovotny

This comment has been minimized.

Member

onovotny commented Jun 15, 2018

@davkean Happy to file a new issue or add to an existing one. I assume that the single TFM issue is the main issue; is there an open issue for that?

@davkean

This comment has been minimized.

Member

davkean commented Jun 15, 2018

The tree is completely based around a "configuration", we represent TFMs a dimension of a configuration; debug|anycpu|net45 and debug|anycpu|netstandard20, etc.

I think this bug you opened two years ago; dotnet/project-system#935 is the bug that best represents this the tree manipulation. The (avoid) double-including of a file behavior is slightly different issue and we should treat separately. I'm completely open to better ideas how to handle this but it is a bloody hard problem to solve and gets very complex very quickly.

@onovotny

This comment has been minimized.

Member

onovotny commented Jun 15, 2018

The double-including behavior doesn't bother me as much for now (unless it causes other issues).

Is solving dotnet/project-system#935 still in scope for 16.0? Like, can we think about how to solve it?

@jamesmontemagno

This comment has been minimized.

jamesmontemagno commented Jun 15, 2018

The original issue outlined is a change in functionality from 15.7.1 to 15.7.3. essentially the valuations for the tree stop. In 15.7.1 all files showed correct and 15.7.3 they stopped.

They other issue of it addijg Includes I think has always been there, we manually delete them, it is more annoying then anything, but different then the original issue, so we should focus on that if possible.

@davkean

This comment has been minimized.

Member

davkean commented Jun 15, 2018

Just to clear things up a little;

  • This change that caused the above issue was an intentional change to fix other issues.
  • This change occurred in the SDK, which is considered a separate installable product, I'm not sure if we upgrade SDKs in VS patch updates (@dsplaisted @livarcocc?), if we do - we should avoid it or avoid pulling in changes like this to prevent these sorts of issues.

While it "appears" that the files shown are correct, they are shown with the wrong item type. If you were to manipulate the item's properties, it would not behave as you would expect. It looks like we/you got lucky that we correctly recognize these files as "source files" and they displayed correctly in the editor. This most definitely was not a "designed thing" and kinda just fell out of our implementation. The entire "how do we display multiple TFMs source files" was being tracked by dotnet/project-system#935.

I'm not sure what to do this with this at this point, and will need to sync up with the SDK folks to discuss. @dsplaisted Can we discuss in our Monday sync-up?

@dsplaisted

This comment has been minimized.

Member

dsplaisted commented Jun 15, 2018

@davkean What do you mean by the patch version of VS? If a version like 15.7.3 is major.minor.patch, then we wouldn't put a change like this in a patch update to VS. We would (and did) put an update like this in a minor update. For the SDK we don't really have a difference between major and minor updates, since we release both with Visual Studio and with .NET Core, which can release major versions on different schedules.

@davkean

This comment has been minimized.

Member

davkean commented Jun 15, 2018

@dsplaisted @jamesmontemagno and @onovotny mentioned that this was regressed between 15.7.1 to 15.7.3.

@dsplaisted

This comment has been minimized.

Member

dsplaisted commented Jun 15, 2018

@jamesmontemagno Is it possible that when testing on 15.7.3, that the .NET Core SDK 2.1.300 was installed, while only 2.1.200 was installed together with 15.7.1?

@onovotny

This comment has been minimized.

Member

onovotny commented Jun 15, 2018

That is possible.

@jamesmontemagno

This comment has been minimized.

jamesmontemagno commented Jun 17, 2018

I am on 15.7.3 + 2.1.300. I can try to go back down to 2.1.200

@dsplaisted

This comment has been minimized.

Member

dsplaisted commented Jun 18, 2018

@jamesmontemagno Yes, it would be helpful if you could try it on 2.1.200 just so we're sure about what causes this.

@jamesmontemagno

This comment has been minimized.

jamesmontemagno commented Jun 20, 2018

So some results here on my work machine:

  • VS 2017 - 15.7.3 with 2.1.201: Working just fine
  • VS 2018 - 15.7.3 with 2.1.300: Not working
@jamesmontemagno

This comment has been minimized.

jamesmontemagno commented Jun 21, 2018

So, now with 15.7.4 still same issue with 2.1.300 sdk, however once I delete 2.1.300 from the dotnet/sdk folder it continues to work again, so it looks like there was a change here that is making it happen.

@jamesmontemagno

This comment has been minimized.

jamesmontemagno commented Jun 21, 2018

Also for reference the reason i had 2.1.300 was for blazor demos

@dsplaisted

This comment has been minimized.

Member

dsplaisted commented Jun 21, 2018

@jamesmontemagno Thanks, this confirms that the behavior you're seeing is the result of what we thought it was: namely, the fix to #1157 and dotnet/project-system#3181.

We will discuss what we can do here to make it easier to browse conditionally included source files in a multi-targeted project. As a workaround, you can try adding something like this to your project file (after any Compile items):

<None Include="**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder);@(Compile)" />
@dsplaisted

This comment has been minimized.

Member

dsplaisted commented Jun 21, 2018

@onovotny If you add @(Compile) to the Exclude as in my previous comment, it may fix the weird behavior you saw with what was added to the project file.

@dsplaisted

This comment has been minimized.

Member

dsplaisted commented Jul 9, 2018

@onovotny @jamesmontemagno Can you try the following "workaround"?

<ItemGroup>
    <None Include="**/*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder);$(Compile)" />
</ItemGroup>

This should go after any Compile items are declared.

This should get you back to the pre-2.1.300 SDK behavior, where all the source code will show in the solution explorer, but various things (adding new items, moving, renaming) in the IDE may not work or may work strangely.

So I don't recommend adding this arbitrarily to all projects, just projects where you have TargetFramework-specific source files that you want to show up in solution explorer.

@dsplaisted

This comment has been minimized.

Member

dsplaisted commented Jul 9, 2018

I'm going to close this issue, as we believe we understand the behavior change here, and the real fix is for solution explorer to show files from all the target frameworks, which is tracked by dotnet/project-system#935.

@jamesmontemagno

This comment has been minimized.

jamesmontemagno commented Aug 16, 2018

Can confirm the workaround works.

onovotny added a commit to onovotny/MSBuildSdkExtras that referenced this issue Aug 17, 2018

onovotny added a commit to onovotny/MSBuildSdkExtras that referenced this issue Aug 17, 2018

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