Skip to content
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

Use default imports for Microsoft.Common.props and Microsoft.CSharp.targets #628

Closed
jongalloway opened this issue Oct 19, 2016 · 19 comments
Closed

Comments

@jongalloway
Copy link

Every example csproj file includes two imports - one for Microsoft.common.props at the beginning and one for Microsoft.CSharp.targets at the end:

<Project>
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" />
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp1.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <Compile Include="**\*.cs" />
  </ItemGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.NETCore.App" Version="1.0.0" />
    <PackageReference Include="Microsoft.NET.SDK" Version="1.0.0" />
  </ItemGroup>
  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
</Project>

Couldn't these two imports be implied, with an option to override them if needed?

I understand this may have been discussed internally, but would like to have it publicly documented and tracked since it will likely be a common question.

@terrajobst
Copy link
Member

@dotnet/msbuild thoughts?

@colhountech
Copy link

+1

@vlesierse
Copy link

I really like this one. The less XML I have to write the better.

@gulbanana
Copy link

perhaps Microsoft.common.props could be considered a standard implementation-defined prelude..

@ErikSchierboom
Copy link

Would love to see these removed!

@jchannon
Copy link

jchannon commented Oct 20, 2016

Had exactly the same thoughts after reading this https://blogs.msdn.microsoft.com/dotnet/2016/10/19/net-core-tooling-in-visual-studio-15/

@filipw
Copy link

filipw commented Oct 20, 2016

How about a csproj without <PropertyGroup>, <ItemGroup>, the 2 default imports and an implicit <Compile Include="**\*.cs" />

<Project>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp1.0</TargetFramework>

    <PackageReference Include="Microsoft.NETCore.App" Version="1.0.0" />
    <PackageReference Include="Microsoft.NET.SDK" Version="1.0.0" />
</Project>

@baronfel
Copy link
Member

Do keep in mind how alternate project systems (VB/F#/etc) might want to define a common set of import targets that aren't the C# ones.

@ErikSchierboom
Copy link

@baronfel Sure, but VB and F# don't use .csproj files, so it should be possible to figure this out based on the extension of the project file.

@terrajobst
Copy link
Member

@filipw

Let's keep this issue focused on imports. Feel free to file a separate issues for PropertGroup/ItemGroup removal.

@MaximRouiller
Copy link

@terrajobst @filipw

Created the issue: #632

@terrajobst
Copy link
Member

@MaximRouiller Thanks!

@rainersigwald
Copy link
Member

I don't see a way to make this work without breaking many existing MSBuild users.

While trying to find a way to do something like this, we saw several problems:

  • Ideally MSBuild doesn't have to know anything about projects--maintaining a distinction between "language requirements" and "standard usage conventions" is a pretty common language-design technique.
  • As pointed out, there are many different sets of imports for different project types, so defaulting anything is potentially a problem.
  • Even with an extension restriction, not every .csproj file actually imports these files.
  • In some (rare but not as rare as you'd hope) cases, it's useful to define a property before the initial Microsoft.Common.props import.
  • In many cases, it's useful to override a target imported through Microsoft.CSharp.targets--which requires defining the override after the import.
  • Larger codebases (that use the above features) often import a local .props/.targets pair that applies customizations even to things that are at heart standard C# projects.

Since this would be a nice boilerplate-reduction, but in the end saves only a couple of lines per project, we opted to spend our development time elsewhere (allowing item metadata in properties saves 2 lines/NuGet reference, better supporting wildcards in the IDE saves a ton of lines, and so on).

@MaximRouiller
Copy link

@rainersigwald

This was moved to dotnet/msbuild#1236

At least, we now have a tracking of why it wouldn't work.

If anyone asks, we can say:

See Issue #???

@srivatsn
Copy link
Contributor

@MaximRouiller, I believe @rainersigwald's response is about the original issue here of not having to import the targets\props.

@MaximRouiller
Copy link

@srivatsn Ah! My misunderstanding. Time to hit the coffee shop I guess. ☕️

@clayton-taylor
Copy link

Perhaps this could be implemented in a backwards-compatible way by adding an attribute to the Project element that allows one to opt-in to the desired behavior.

@dsplaisted
Copy link
Member

Perhaps this could be implemented in a backwards-compatible way by adding an attribute to the Project element that allows one to opt-in to the desired behavior.

Another way to do this would be to only import the default props and targets files if there is no xmlns attribute on the Project element. That attribute will no longer be necessary so we could use it to light up new, but not backwards compatible, functionality.

If we do this based on the xmlns, I'd suggest allowing a UseDefaultImports="false" property on the Project element to disable the default imports in case you want to do something before the props or after the targets or you just don't want to use the default imports at all.

@rainersigwald
Copy link
Member

This is possible for new .NET Core projects with dotnet/msbuild#1403, released with the updated VS2017 RC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests