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

MSBuild should have a directory import in Microsoft.Common.Targets #222

Closed
Serivy opened this issue Sep 21, 2015 · 28 comments

Comments

@Serivy
Copy link

commented Sep 21, 2015

My request is to be able to put enterprise level logic in a msbuild target at a solution level rather than modifying every single .*proj file. This way I can modify one file in the solution directory and all projects which implement Microsoft.Common.Targets can set things like $(OutDir), Reference Paths, Analyzers, Nuget restores/updates or even Build server logic (Strongname signing, digital signuates, assembly versions). It would be priceless to enforce logic on a solution without relying on developers to install the custom target file into the right directory. Currently I do this by modifying all the projects to import a solution wide target file.

There are more details on the uservoice page including other suggestions to change but it would essentially be adding <Import Project="$(SolutionDir)Custom.$(MSBuildThisFile)" Condition="$(SolutionDir)Custom.$(MSBuildThisFile)')"/> or some variation of this to Microsoft.Common.CurrentVersion.targets just under $(MSBuildProjectFullPath).user.

I created a user voice post previously but when I saw the news about MSBuild being open source I decided to raise it directly with you here. https://visualstudio.uservoice.com/forums/121579-visual-studio/suggestions/9674760-add-a-custom-solution-import-in-microsoft-common-t
I apologise for my persistence with this request however it would make maintaining an enterprise build system much easier.

I am happy to answer any queries you may have, especially if it strengthens the possibility of this becoming a reality! Thanks

@rainersigwald

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2015

This is an interesting idea, and there is definitely room for improvement in large-scale configurability. However, I have a number of concerns about this approach.

The main issue is that, since a solution isn't a core MSBuild concept, using this extensibility point would be confusing for anyone who lives in a world where they don't only build through the solution.

That would be a problem for many CI build servers that are configured to build through a traversal project rather than a solution (which is quite common).

It would also be confusing in the fairly common case of having multiple "subsolutions" for different teams or roles within a codebase. One would have to have the canonical configuration, but how would changes be coordinated between solutions?

And tying a project to a solution would also dramatically change single-project builds from the command line, since MSBuild would have no way to know what solution to find configuration in.

In general, I favor the explicit approach of adding an import to all projects, as you currently do.

Thoughts?

@Serivy

This comment has been minimized.

Copy link
Author

commented Oct 10, 2015

Thanks for taking the time to write a reply. Sorry if this is a long winded reply but you asked for my thoughts. :)

I can see your concern with solutions being out of scope of MSBuild. Perhaps I jumped the gun in suggesting a solution rather than giving my user story of the pain in managing our enterprise solution. Explicitly adding imports to every project does work because we currently do it, however we have been caught out time after time when one of the developers decide to add a new project and are unaware of the need to import a custom target, or import it in the wrong place. In our organisation we have been split between developers using a solution and build servers using .proj files, we would really just love to target the solution in both worlds. I would love the day when solutions are restructured to natively be msbuild projects but I can't see that happening in the near future. At the end of the day I am struggling to have all my developers and build servers use the same build tools and processes in their own environments.

I do not feel adding an import to the solution directory would be any more confusing than some of the existing 23 imports in Microsoft.Common.CurrentVersion.targets such as the visual studio specific <Import Project="$(MSBuildProjectFullPath).user" Condition="Exists('$(MSBuildProjectFullPath).user')"/> line, or the presence of the SolutionDir property in the file already. I can agree that just because those exist doesn't mean more hacks should be added :) The truth is the extensibility point that answers all my needs already exists in the file: the property $(CustomBeforeMicrosoftCommonTargets) but setting it from msbuild targeting a solution needs hacks and from visual studio is impossible (To my knowledge).

There currently is an extensibility point in solutions using after.MySolution.sln.targets (Setting Set MSBuildEmitSolution=1, running msbuild.exe /v:n MySolution.sln and then looking at the MySolution.sln.metaproj that is generated shows the import) but this calls MSBuild on each project without letting me set the properties, and visual studio completely ignores the import (Which is understandable). Through using some creative hacks (See post) I am able to inject that property in using msbuild.exe but I still have no way of tricking visual studio.

To be honest, I had to look up traversal projects and while I understand the concept I have never had to use them in my career. Perhaps I am lucky enough to be working on small enough solutions or they are broken up in a way that allows me to build 30 projects at a time and chain the next set as dependencies from a build server. I personally don't think it would create much confusion using traversal project or single projects, I would just suggest they do not use the file/import. :)

I am fine if it is not in msbuild's best interest to cater for solutions any more than it currently does, but while visual studio needs developers to have them I will try make our build servers use those files.

Thanks again

@dsplaisted

This comment has been minimized.

Copy link
Member

commented Oct 11, 2015

Can you put targets files in the MSBuildExtensionsPath or MSBuildUserExtensionsPath folder? Microsoft.Common.CurrentVersion.targets will import any targets files in the following folders:

  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.targets\ImportAfter\*" Condition="'$(ImportByWildcardAfterMicrosoftCommonTargets)' == 'true' and exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.targets\ImportAfter')"/>
  <Import Project="$(MSBuildUserExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.targets\ImportAfter\*" Condition="'$(ImportUserLocationsByWildcardAfterMicrosoftCommonTargets)' == 'true' and exists('$(MSBuildUserExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.targets\ImportAfter')"/>

Now you probably don't want to put your enterprise level logic there, but you can put a targets file that will import the right targets file from a directory above the project directory. Something like this:

<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), enterpriselogic.targets))\enterpriselogic.targets" 
  Condition="Exists('$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), enterpriselogic.targets))')"/>

Does this help? It's not explicitly scoped to the solution, but rather to whatever level in the directory structure you put an enterpriselogic.targets file in.

@Serivy

This comment has been minimized.

Copy link
Author

commented Oct 11, 2015

I am aware of the Import directories, in my use case i have done something very similar except i use SolutionDir and it works perfectly. The problem is ensuring everyone who builds the application also has it installed. My ideal situation is for any developer to load up a solution with visual studio installed and they will get the same experience as anyone else. Nuget has provided a huge leap in this area with including msbuild targets but is lacking at the solution level.

I could add an conditioned to the absence of that file to tell them to ensure they put the file there and I can think of many hacks such as having the first project add the file into the directory. It may seem to be sensible to say 'everyone who builds this must first run this msi' but you'd be surprised how often you'd find someone missed a step in the setup process.

If you think this problem is currently an edge case and the existing methods are good enough I am fine with it. I was just using this as an opportunity to get feedback to see if I am the odd one out. :)

@dsplaisted

This comment has been minimized.

Copy link
Member

commented Oct 11, 2015

What if you didn't need to put anything in the Import directories, you just had to put a targets file with the right name in one of the ancestor directories of your project? This sounds similar to what you're asking for, except instead of using $(SolutionDir), which isn't always defined, it would walk up the directory tree looking for the file to import.

@Serivy

This comment has been minimized.

Copy link
Author

commented Oct 11, 2015

So basically like the import you said earlier but baked into common? That would meet all my needs. :)

I'm no expert on GetDirectoryNameOfFileAbove but I can't imagine it would be cheap to add to every single project? I would hate to have everyone else suffer a performance hit for my needs.

@dsplaisted

This comment has been minimized.

Copy link
Member

commented Oct 12, 2015

@Serivy Yes, I think we'd need to investigate the performance impact of adding an import with GetDirectoryNameOfFileAbove in it. Personally I feel like this would be broadly useful (ie you're not the only one with these type of needs).

@thargy

This comment has been minimized.

Copy link

commented Oct 12, 2015

+1

@thargy

This comment has been minimized.

Copy link

commented Oct 12, 2015

My only objection to the proposal is discoverability (not like MSBuild is super well documented as it is) and the law of unintended consequences:
e.g. "Why the hell does it build when it's on my D drive but not on my C drive?"

@andrassy

This comment has been minimized.

Copy link

commented Jan 16, 2016

+1 to the "walk up the tree" approach. Would you walk all the way up to the root gathering all the common targets or stop at the first one found?

@Serivy

This comment has been minimized.

Copy link
Author

commented Jan 16, 2016

I guess just the first one is probably safest. If you want it to continue walking you could include the same import code in that one :) Although i'm not fussed either way!

I must say I am quite excited about this change should it happen!

@aheusser

This comment has been minimized.

Copy link

commented Feb 23, 2016

I also agree with the basic need expressed by the OP. Thanks to @Serivy for taking the time to coherently capture the need. Rest assured: from my perspective, at least, you are not the "odd one out".

Having a capability along these lines would be valuable (i.e., an ability to apply "enterprise build logic" across all projects in a solution without requiring something specific be authored into each individual .csproj and without requiring something special be installed on a build machine).

Another way of stating the desire might be: the "enterprise build logic" should be something that can be reasonably and naturally stored and retrieved from the same repository structure in which you retrieve the actual solution source itself. Once retrieved, one would ideally not have to engage in "special steps" to get the associated enterprise logic incorporated into your machine's msbuild system.

@Serivy

This comment has been minimized.

Copy link
Author

commented Feb 24, 2016

Thanks @aheusser
Modifying all the proj files is something i've been doing for years and I thought there had to be better way, especially with features slipping in such as before.SOLUTIONNAME.sln.targets which helped it but didn't solve. That is a good way to look at it, I want to get to the point where you can git clone a solution, nuget restore and off we go on anyone dev/build machine.

The landscape will be changing so much in the near future with project.json files, Msbuild going cross platform, and just how node/npm has changed builds with solution level before/after targets. I would be interested if some day MSBuild adopts a similar 'gulp watch' sort of system for keeping an eye on file system changes.

Exciting times.

@jeffkl

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

The team has discussed this and we want to get some feedback on our design.

We don’t believe a solution-based approach is a good idea for MSBuild. MSBuild itself doesn’t directly support solutions since they’re really just a Visual Studio concept. MSBuild can build solutions by transforming it to a project and then performing a build. Also, when building a single project via MSBuild, there is also no mapping from a project to a solution so there would be no way for MSBuild to know which solution-based project to import. There is also a case where you could have multiple solutions in your tree and would need to replicate your build logic for each one.

We think a more generic approach would be to have MSBuild traverse up the directory tree for a project to import similar to how it will do a wildcard import of projects in a specific location. Our design would be to use the GetDirectoryNameOfFileAbove() property function to look for a Directory.Build.props to import. This Directory.Build.props could then give users all of the flexibility they need to import a project based on the solution or have a Directory.Build.props at any level of the tree. There could be a slight added overhead because whenever you build, it would be walking up the directory structure looking for a file but we believe since it’s only checking for file existence it will add very little overhead.

Another thing we’re considering is when to stop looking in parent directories. When it is determined that the search is looking at the root of a Git repository for instance, it should probably stop traversing up. For example, there could be unintended results if you place a Directory.Build.props in the root of your drive because projects could import it. If we have logic to stop traversing up the tree, we could avoid this situation. The real issue is the list of known stopping points could get hard to manage. We think adding a check for a .git folder would probably suffice.

So to summarize, we’re thinking of adding the following logic to Microsoft.Common.props and something similar to Microsoft.Common.targets:

<PropertyGroup>
    <ImportGlobalBuildProps Condition="'$(ImportGlobalBuildProps)' == ''">true</ImportGlobalBuildProps>
    <GlobalBuildProps Condition="'$(ImportGlobalBuildProps)' == 'true' and '$(GlobalBuildProps)' == ''">$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), Directory.Build.props))\Directory.Build.props</GlobalBuildProps>
</PropertyGroup>

<Import Project="$(GlobalBuildProps)" Condition="'$(ImportGlobalBuildProps)' == 'true' and exists('$(GlobalBuildProps)')"/>

Please let us know what you think.

@Serivy

This comment has been minimized.

Copy link
Author

commented Jun 30, 2016

The design sounds like it will meet all my needs.

While I was pretty keen on a solution based approach at first I agree now it isn't right. I really do wish .sln files were to become MSBuild files, rather than requiring MSBuild to generate a metaproj file from its data. Any plans on this, especially considering .Net Cores full adoption of MSBuild projects? :) Traversing up the directory tree sounds like a perfect solution to my needs. Feel free to rename this issue to something more suited. Even if I wanted to (which I don't) I could even do solution property checks in there. I am very happy to hear of the low overhead of it on the build.

Knowing when to stop sounds like a difficult problem. My commercial projects don't use git so wouldn't really impact me but I am not too worried about rogue target files in the root of my drive. I have to check MSBuild logs daily so its not too hard to see which file the target is running from. Realistically my project folders are getting filled up with a lot of these kind of configuration files, some traverse all the way to the root until they get a match (nuget.config, tslint.js, Settings.StyleCop) and some dont (.npmrc).

I will copy the proposed code to my copy of common.props just to see how it goes, if I find anything I think is important ill reply to this issue.

I know I may be sticking my nose in where its not wanted but if i could have a few moments of your time to consider something and tell me if I am barking up the wrong tree? I don't speak for the Nuget team but I follow it closely and I cant help but notice the technique we're looking at here could aid in one of their goals (http://blog.nuget.org/20141010/nuget-is-broken.html) to "Leave Project Files Alone". When you install a nuget package with a target file it will modify the project and inject an import for each nuget package along with an error task if its not found. Something like you're doing here could have a profound effect if they can make nuget just dump the .target files into a known directory structure. There may be too many issues like requiring the latest MSBuild, missing error messages if the nuget package wasn't restored, installing packages against projects which dont import common.targets or something beyond my comprehension. Either way I thought i'd just bring it to your attention in case it could be useful.

Thanks :)

@rainersigwald

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

@Serivy Regarding the NuGet stuff: we're working with the NuGet team and the .NET CLI folks on evolution of how packages get referenced. I don't think there's really anything to show yet, though.

But there is a form of NuGet that is a little cleaner today, which is how UWP apps use project.json files to refer to packages. That process doesn't require changing the project file--they have .targets files that include things from the referenced packages that get automagically included.

We expect that the future direction of NuGet would be similar to that (but again, it's in flux).

@jeffkl

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

My initial commit is out for a pull request.

Yes, as @rainersigwald said, the project.json currently addresses this by doing the following:

  1. They install a file under %ProgramFiles(x86)%\MSBuild\14.0\Microsoft.Common.Targets\ImportAfter so they can inject targets.
  2. nuget restore generates an MSBuild project for importing any .props or .targets files in the NuGet package in the folder where the project being built exists.
  3. %ProgramFiles(x86)%\MSBuild\Microsoft\NuGet\Microsoft.NuGet.props and %ProgramFiles(x86)%\MSBuild\Microsoft\NuGet\Microsoft.NuGet.targets then import the dynamically generated projects.

This means that the original CSPROJ is not modified and the imports happen magically. This is true for the assembly references as well. This only works for MSBuild 14.0 and above because of where the files are installed to.

That said, the future of project.json is up in the air and we're working closely with the NuGet team.

@Serivy

This comment has been minimized.

Copy link
Author

commented Jul 1, 2016

Thanks! Looking forward to it.

@jeffkl

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2016

This functionality has been merged.

@jeffkl jeffkl closed this Jul 7, 2016

@wangzq

This comment has been minimized.

Copy link

commented Aug 29, 2016

Sorry for the spam, when will this feature be released? Thanks

@rainersigwald

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2016

@wangzq This will be in the next full release of MSBuild, which should come with Visual Studio "15". Thanks for asking, because in the course of double-checking my "it's in Preview 4" answer, I discovered that it's actually not, because of #979! In Preview 4, you can use the .targets form, but .props is broken. We'll get it fixed before the next preview release.

@Sarabeth-Jaffe-Microsoft Sarabeth-Jaffe-Microsoft changed the title MSBuild should have a solution import in Microsoft.Common.Targets MSBuild should have a directory import in Microsoft.Common.Targets Nov 2, 2016

@JessePelton

This comment has been minimized.

Copy link

commented Apr 17, 2017

For others who find this issue but can't figure out why the global.build.props and global.build.targets files discussed above don't get loaded by MSBuild 15+: the file names were changed to Directory.Build.props and Directory.Build.targets (as hinted at by the second Feb 13 reference title).

@Serivy

This comment has been minimized.

Copy link
Author

commented Apr 18, 2017

I have been using them in production and loving them. Thanks for the change!

@rainersigwald

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2017

Directory.Build.props is now documented at https://docs.microsoft.com/en-us/visualstudio/msbuild/customize-your-build. Feedback is welcome!

@Serivy

This comment has been minimized.

Copy link
Author

commented Jun 27, 2017

I have had a read through and it is good. I have already forwarded it on to a few colleagues. Well done and thanks!

@TFTomSun

This comment has been minimized.

Copy link

commented May 23, 2019

@rainersigwald That seems not to work for old MSBuild project files (not sdk style). Should it be supported for them, too?

@Serivy

This comment has been minimized.

Copy link
Author

commented May 23, 2019

Provided it imports Common at the right tools level it will read in the Directory.Build.targets file.

If it does not import common or the tool versions are too old, such as WIX projects then i add this to the .wixproj project.

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
    <_ImportDirTarget>$([MSBuild]::GetDirectoryNameOfFileAbove('$(MSBuildThisFileDirectory)', 'Directory.Build.targets'))\Directory.Build.targets</_ImportDirTarget>
  </PropertyGroup>
  <Import Condition="'$(MSBuildToolsVersion)' &lt; '15' and Exists('$(_ImportDirTarget)')" Project="$(_ImportDirTarget)"/>
  ...
 </Project>
@TFTomSun

This comment has been minimized.

Copy link

commented May 23, 2019

@Serivy thanks for clarification. I was not aware of that the common targets need to be imported for that feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.