Upcoming feature in NuGet 2.3: Auto import .targets and .props file from package

Topics: General
Developer
Feb 18, 2013 at 5:48 PM
Edited Feb 18, 2013 at 6:00 PM
Hi all,

I'm excited to introduce a new feature in NuGet 2.3 that I'm sure many of you have been requesting for a while: the ability for NuGet to import MSBuild-based .targets and .props files into the project.

For this feature, we introduce a new top-level folder, build, in addition to content, lib and tools. Under this folder, you can place two files with fixed names, <package id>.targets or <package id>.props. These two files can be either directly under \build or under framework-specific folders just like the other folders. The rule for picking the best-matched framework folder is exactly the same as in those.

When NuGet installs a package with \build content, it will add an <Import> element in the project file pointing to the .targets and .props files. The .props file is added at the top, whereas the .targets file is added to the bottom.
Feb 18, 2013 at 6:35 PM
@dotnetjunky this is a great feature. As discussed on Twitter, this will be fully compatible with package restore which is a huge win. Package restore compatibility is the main thing eluding myself and other authors of current packages which import targets using Install.ps1.

I'm keen to give this a try - has it already been merged into master?
Feb 18, 2013 at 6:39 PM
+1 for what @adamralph said :-)
Give a shout when we can download the pre-release bits, eager to give it a try as well!
Feb 18, 2013 at 6:56 PM
Great feature.
Just wondering what happens if I:
  1. Setup my solution to enable nuget package restore
  2. Install a package with \build content
  3. Remove my packages directory
  4. (Re-)build my solution
Will I get msbuild errors about missing .targets/.props files that msbuild tries to import, or are the project files smart enough to ignore importing these files while restoring nuget packages?
Developer
Feb 18, 2013 at 6:57 PM
It's not in master yet. I'll reply to this thread when it's ready. Should be this week.

Regarding package restore compatibility, we expect it to work, but admittedly, we haven't tested thoroughly. When you play with the bits, do let us know any issues that you discover.
Feb 18, 2013 at 7:59 PM
I just tried the latest version in the kinhln-buildfiles branch and it seems to work well! The properties and targets are imported as specified and it also works with package restore.

I just noticed one problem, here's what I did:
  1. I packaged a version of StyleCop.MSBuild using the \build folder.
  2. I installed it into a project, enabled package restore, closed the project, wiped out the packages folder and then re-opened the project.
  3. I put a deliberate StyleCop violation into the code.
  4. The first time I build, I can see that the packages get restored but I get no StyleCop violation.
  5. The second time I build, I do get a StyleCop violation.
What this leads me to believe is that, during the first build, the packages are restored but the targets are not imported for that build. After the first build has finished, the targets do seem to get imported somehow and are present for the second build.

I think this needs to be fixed since the first build needs to be fully indicative of the state of the code. E.g. I might do a git clean and then pull down someone's pull request branch and build. I don't want to have to build a second time to find out they've made a StyleCop violation before merging.
Feb 18, 2013 at 8:04 PM
Interesting... Package restore is a pre-build step, so I'd assume the imports to happen before the actual compilation starts. Is StyleCop a pre-build or post-build step?
Feb 18, 2013 at 8:21 PM
Edited Feb 18, 2013 at 8:43 PM
StyleCop is post build. The problem is that package restore happens too late. i.e. Imports are done, then packages are restored. See my comment at https://bitbucket.org/adamralph/stylecop-msbuild/issue/6/stylecopmsbuild-fails-with-nuget-package#comment-2939412
Feb 18, 2013 at 8:34 PM
I've confirmed that the above problem is also present for properties. They are not imported in the first build that does the package restoration. Only for subsequent builds are the properties actually imported. I've also confirmed that the behaviour is exactly the same outside VS, i.e. invoking msbuild from the command line.

This unfortunately leaves us in no better shape than we are in currently with regard to package restore. If the change that was intended to make this work with package restore was simply the addition of the Condition to the Import element then that is not enough. E.g.
<Import Project="..\packages\StyleCop.MSBuild.4.7.44.2\build\net35\StyleCop.MSBuild.Targets" Condition="Exists('..\packages\StyleCop.MSBuild.4.7.44.2\build\net35\StyleCop.MSBuild.Targets')" />
I added a similar conditional to StyleCop.MSBuild almost a year ago. It allows the project to load initially, but it doesn't ensure that the targets are imported for that build. For this reason I put a whole bunch of warnings/errors into the StyleCop.MSBuild installation so that the user is aware of the situation and how to rectify it. If I were to switch StyleCop.MSBuild to the new \build folder with this issue present then I would gain nothing in terms of package restore compatibility but I would lose the warnings/errors (unless I left my Install.ps1 present just do that but that would defeat the purpose of moving to \build).

For reference, when StyleCop.MSBuild is installed into a project, the following is added to the csproj file:
  <PropertyGroup>
    <StyleCopMSBuildTargetsFile>..\..\packages\StyleCop.MSBuild.4.7.44.1\tools\StyleCop.targets</StyleCopMSBuildTargetsFile>
  </PropertyGroup>
  <Import Condition="Exists('$(StyleCopMSBuildTargetsFile)')" Project="$(StyleCopMSBuildTargetsFile)" />
  <PropertyGroup>
    <StyleCopMSBuildMessageMissing>Failed to import StyleCop.MSBuild targets from '$(StyleCopMSBuildTargetsFile)'. The StyleCop.MSBuild package was either missing or incomplete when the project was loaded. Ensure that the package is present and then restart the build. If you are using an IDE (e.g. Visual Studio), reload the project before restarting the build.</StyleCopMSBuildMessageMissing>
    <StyleCopMSBuildMessagePresent>Failed to import StyleCop.MSBuild targets from '$(StyleCopMSBuildTargetsFile)'. The StyleCop.MSBuild package was either missing or incomplete when the project was loaded (but is now present). To fix this, restart the build. If you are using an IDE (e.g. Visual Studio), reload the project before restarting the build.</StyleCopMSBuildMessagePresent>
    <StyleCopMSBuildMessageRestore>Failed to import StyleCop.MSBuild targets from '$(StyleCopMSBuildTargetsFile)'. The StyleCop.MSBuild package was either missing or incomplete when the project was loaded. To fix this, restore the package and then restart the build. If you are using an IDE (e.g. Visual Studio), you may need to reload the project before restarting the build. Note that regular NuGet package restore (during build) does not work with this package because the package needs to be present before the project is loaded. If this is an automated build (e.g. CI server), you may want to ensure that the build process restores the StyleCop.MSBuild package before the project is built.</StyleCopMSBuildMessageRestore>
    <StyleCopMSBuildMessageRestored>Failed to import StyleCop.MSBuild targets from '$(StyleCopMSBuildTargetsFile)'. The StyleCop.MSBuild package was either missing or incomplete when the project was loaded (but is now present). To fix this, restart the build. If you are using an IDE (e.g. Visual Studio), reload the project before restarting the build. Note that when using regular NuGet package restore (during build) the package will not be available for the initial build because the package needs to be present before the project is loaded. If package restore executes successfully in the intitial build then the package will be available for subsequent builds. If this is an automated build (e.g. CI server), you may want to ensure that the build process restores the StyleCop.MSBuild package before the initial build.</StyleCopMSBuildMessageRestored>
  </PropertyGroup>
  <Target Name="StyleCopMSBuildTargetsNotFound">
    <Warning Condition="!Exists('$(StyleCopMSBuildTargetsFile)') And $(RestorePackages)!=true And $(StyleCopTreatErrorsAsWarnings)!=false" Text="$(StyleCopMSBuildMessageMissing)" />
    <Warning Condition="Exists('$(StyleCopMSBuildTargetsFile)')  And $(RestorePackages)!=true And $(StyleCopTreatErrorsAsWarnings)!=false" Text="$(StyleCopMSBuildMessagePresent)" />
    <Warning Condition="!Exists('$(StyleCopMSBuildTargetsFile)') And $(RestorePackages)==true And $(StyleCopTreatErrorsAsWarnings)!=false" Text="$(StyleCopMSBuildMessageRestore)" />
    <Warning Condition="Exists('$(StyleCopMSBuildTargetsFile)')  And $(RestorePackages)==true And $(StyleCopTreatErrorsAsWarnings)!=false" Text="$(StyleCopMSBuildMessageRestored)" />
    <Error Condition="!Exists('$(StyleCopMSBuildTargetsFile)') And $(RestorePackages)!=true And $(StyleCopTreatErrorsAsWarnings)==false" Text="$(StyleCopMSBuildMessageMissing)" />
    <Error Condition="Exists('$(StyleCopMSBuildTargetsFile)')  And $(RestorePackages)!=true And $(StyleCopTreatErrorsAsWarnings)==false" Text="$(StyleCopMSBuildMessagePresent)" />
    <Error Condition="!Exists('$(StyleCopMSBuildTargetsFile)') And $(RestorePackages)==true And $(StyleCopTreatErrorsAsWarnings)==false" Text="$(StyleCopMSBuildMessageRestore)" />
    <Error Condition="Exists('$(StyleCopMSBuildTargetsFile)')  And $(RestorePackages)==true And $(StyleCopTreatErrorsAsWarnings)==false" Text="$(StyleCopMSBuildMessageRestored)" />
  </Target>
  <PropertyGroup>
    <PrepareForBuildDependsOn Condition="!Exists('$(StyleCopMSBuildTargetsFile)')">StyleCopMSBuildTargetsNotFound;$(PrepareForBuildDependsOn)</PrepareForBuildDependsOn>
  </PropertyGroup>
Developer
Feb 18, 2013 at 8:43 PM
Does this work with task assemblies that the target imports?
Feb 18, 2013 at 8:45 PM
I don't think it matters what exactly is being imported. Properties demonstrate that. I believe the issue is that msbuild loads the project, performs the imports, and only then does package restore occur. Too late for the imports to happen after package restore has occurred. I've no idea how to get round this. I spent ages trying...
Coordinator
Feb 18, 2013 at 8:52 PM
I don't want to hijack this thread, so I started a new one, but here's a question... Why is it important that package restore can happen via msbuild? Why not just require nuget.exe install before you can build the project?

http://nuget.codeplex.com/discussions/433581
Feb 18, 2013 at 9:06 PM
Edited Feb 18, 2013 at 9:07 PM
@jeffhandley sure, you can do that. If you look at the warnings/errors that StyleCop.MSBuild shows to the user in these cases, it suggests this course of action:
If this is an automated build (e.g. CI server), you may want to ensure that the build process restores the StyleCop.MSBuild package before the initial build.
The problem is, it requires the build script writer to take this extra step and that is easily missed, specially if the package gives no warning/error as StyleCop.MSBuild does.

In my opinion this feature is a no go unless this issue is fixed. As is, it doesn't provide any significant benefit over what we have now. Anyone building a targets package would still have to provide exactly the same warnings/errors as StyleCop.MSBuild is doing currently. I know that many of such packages don't do this but they should, otherwise any build from a fresh checkout is inconclusive, whether it's a user executing manually or, more importantly, a CI build which is configured to use a fresh checkout on each build.
Coordinator
Feb 18, 2013 at 9:13 PM
I would argue that in the worst case scenario, where Package Restore doesn't fully work, this feature is still useful. Especially if NuGet also imported MSBuild step to indicate that the targets file wasn't imported and a second build is needed (causing the build to fail very early).

Then, instead of this being PowerShell install/uninstall scripts, it's still a built-in feature.

Of course, it would be 10,000 times better if package restore worked!
Feb 18, 2013 at 9:19 PM
Edited Feb 18, 2013 at 9:20 PM
That is true, if NuGet effectively replicated what StyleCop.MSBuild does with the warnings/errors then that would leave us in the same situation functionally, but much simpler in implementation, since we can rely on NuGet to do everything and discard the ps1 scripts.

I guess the error message can be much simpler, StyleCop.MSBuild's is rather verbose, and probably just a single consolidated message, regardless of whether package restore is on or not.

Also, StyleCop.MSBuild switches between warning and error depending on whether StyleCopTreatErrorsAsWarnings=true/false but that's a special case for the StyleCop targets. I'd be happy with it just being an error every time.
Feb 18, 2013 at 9:26 PM
I find this proposal a very valuable idea, but I have a feeling that consumers who use package restore and consume a package that imports MSBuild targets causing trouble will either:
  • commit that package to source control
  • or commit all packages to source control and disable package restore at all due to many headaches/annoyances
I think @jeffhandley is asking a fair question: can't pkg restore be made a pre-compilation step without relying on MSBuild?
Most build systems simply run nuget.exe before starting the compilation phase. I don't think there's any hard requirement for it to be MSBuild based? If NuGet ships with every Visual Studio SKU, it could as well have an option for package restore built-in (in similarity to the "run tests after build" setting).
Feb 19, 2013 at 3:19 AM
I love this idea; OctoPack for example provides a custom .targets file that hooks into the project build. Having NuGet automatically import it would be great. Some OctoPack users complain that the .targets file and a custom MSBuild task DLL needs to be checked into source control even when Package Restore is enabled, due to the problem that adamralph points out. Jeff's suggestion of performing package restore prior to invoking MSBuild seems like a good option.
Feb 19, 2013 at 5:48 AM
Edited Feb 19, 2013 at 5:54 AM
I agree that the invocation of package restore prior to MSBuild is the solution but I strongly believe it should be the responsibility of NuGet to raise an appropriate error when this has not been done. Otherwise the burden is shifted to either package producers to provide these errors (as currently implemented in StyleCop.MSBuild) or to package consumers (VS user, build script writer, build system provider) to remember to either restore packages before the first build or run a second build (messy).

Whilst the VS/build system cases can probably be taken care of by the tooling as suggested above, I should also be able to git clone and run build.bat/rake/etc. and rely on a complete build taking place, and if not, I should be informed (and then presumably I'd question the build script author as to why they didn't add package restore as a first step!).

I propose that in addition to the Import elements for .props and .targets, NuGet injects a further target into PrepareForBuildDependsOn (see StyleCop.MSBuild example above) which raises an error if a packaged .targets or .props has not been imported. If the core team don't have the bandwidth to add this, I'd be happy to send a PR once the branch has made it onto master.

I had plenty of enquiries about StyleCop.MSBuild and package restore before I added the error messages, after which the enquiries largely disappeared. I guess that some people were also running automated builds on fresh checkouts without even realising that their StyleCop targets were not being imported and invoked. I guess they'd find out eventually, but it just puts speed bumps in the dev cycle.

Ultimately, the absolute fix-it-for-all-no-further-questions solution to all this may well be to introduce NuGet package restore as an MSBuild (and XBuild) feature in it's own right. I.e. NuGet package restore would not be 'just another target' but it's something that is outside that process, which takes place before the import of targets, etc. but I guess this is probably not feasible to get done or perhaps may not be desirable from an MSBuild architecture POV.
Feb 19, 2013 at 6:18 AM
adamralph wrote:
Whilst the VS/build system cases can probably be taken care of by the tooling as suggested above, I should also be able to git clone and run build.bat/rake/etc. and rely on a complete build taking place, and if not, I should be informed (and then presumably I'd question the build script author as to why they didn't add package restore as a first step!).
+1, regardless of whatever happens to pkg restore, consumers should be informed about any issues that can be detected by the NuGet CLI.
Feb 22, 2013 at 9:42 PM
What are the thoughts from the core team about raising build errors when build packages are missing and not imported?

The support for build packages is a great feature and I already have some ideas for some new packages which can take advantage of it.

I'd also love to discard Install.ps1 from StyleCop.MSBuild and switch it to a build package but I can't unless the build errors are added. As I mentioned above, I was forced into adding the build errors to StyleCop.MSBuild because there was so much confusion among consumers as to what was going on with package restore. If I were to switch to using the build packages feature in it's current form, I'd effectively be regressing StyleCop.MSBuild back to that previous state.

For this reason, I don't believe this feature is ready to ship without the build errors. However, I'm really keen to see it go out in 2.3 as planned. Would the core team be willing to take a pull request for the build errors?
Developer
Feb 23, 2013 at 7:24 AM
When I worked with feathercowboy on this feature to support native library project, he tried with package restore when his package was missing. It worked for him because his imported .targets file only contained added properties. It didn't contain any <Target> elements. Perhaps MSBuild is happy to evaluate properties on demand after the package restore phase. I'll ask him to reply on this thread to confirm.

Hence, we need to think more about always raising build errors when build packages are missing. I don't want to raise build errors at least with the native library packages. Give me some time to talk to feathercowboy. I'll get back to this thread on Monday when I find out.
Feb 25, 2013 at 3:56 PM
I've been doing a lot of plumbing-style work with MSBUILD recently; I now think there is probably an easy enough way to do this generically inside the nuget.targets file; I too would like this feature, but I wasn't willing to block 2.3 for it.

I'm just getting back after handling a family emergency, let me knock this around for a day or so, and get back to you by wednesday (Ping this thread if I don't ...)

G
Feb 25, 2013 at 4:59 PM
Edited Feb 25, 2013 at 5:00 PM
+1 fearthecowboy I was thinking along similar lines. E.g. if the csproj defined the targets to import in a Property/ItemGroup then NuGet.targets can do all the work and no need to write anything further into the csproj. It would be of huge value if this made into 2.3. Thanks for looking into it!
Mar 27, 2013 at 9:32 PM
@fearthecowboy did you manage to get anywhere with this?
Mar 27, 2013 at 9:36 PM
adamralph wrote:
@fearthecowboy did you manage to get anywhere with this?
Yes. Yes I did.

First, I'll point you to a blog post I did this morning on the subject.

I've created a tool that will do all of the heavy lifting where it comes to generating packages that need .props and .targets files (so, native packages)

I haven't yet documented it, but I'm hoping to get somewhere with that in the next couple of days.
Apr 2, 2013 at 11:58 AM
@fearthecowboy thanks.

@dotnetjunky has any more thought been given to adding error raising into NuGet?