paket.targets per package

[ctaggart]

When I adopted Paket, I was expecting paket.targets files per NuGet package to soon happen. It was mentioned before in #197. I'd really like to see this happen. Something like this:

<Import Project="..\..\..\packages\FSharp.Core\paket.targets" Condition="..\..\..\packages\FSharp.Core\paket.targets" />
<Import Project="..\..\..\packages\Newtonsoft.Json\paket.targets" Condition="..\..\..\packages\Newtonsoft.Json\paket.targets" />

There are way to many changes being made to the project files and this causes lots of noise in the diff and some frustration when merging. An example is this pull request I just made for FAKE: https://github.com/fsharp/FAKE/pull/602

[forki]

I know that this is a requested feature and I already started working on it. But it's not an easy one.

Regarding the FAKE changes. That's not that bad. The real issue was that I didn't make that Paket 0.17 transition before, so that you had both, the improved Paket output and your FSharp.Core changes in one Commit. But it's still good progress:

[forki]

sneak peek for v0.19:

[isaacabraham]

awesome

[ctaggart]

That is so beautiful!

You may wish to stick with lowercase paket.targets instead of Paket.targets since all the other files are lowercase. Since they are auto-generated, it doesn't matter much to me. May be that actually helps differentiate it from the other paket.targets used for msbuild targets.

[panesofglass]

Lovely! Does this mean packages need to publish paket.targets files?

[ctaggart]

No, they get generated by Paket when it unzips the nupkg.

[forki]

how about generated.paket.targets this will make it easy to find?

[forki]

There is still one issue left:

We can't create the targets files per package since we have to create different When clauses if the csproj file already contains the same framework assemblies.

So we have two options: 1) Create a targets file per package and project and put it under packages

• How can we create a good naming pattern to match projects and targets files? 2) Create a targets file per project (which contains all packages) and put it next to the project.
• If the project is call MyProject.csproj we could call these MyProject.paket.targets
• One could gitignore all *.paket.targets

What do you guys think?

/cc @agross @mexx

[TWith2Sugars]

I'd prefer option 1 with MyProject.paket.targets

[forki]

that wouldn't work since in one repository can be man MyProject.csproj files in different folders.

[agross]

If we go for option 1 we should regenerate the project structure and put the targets there.

[agross]

packages/Foo
packages/targets/src/Bar/Bar.csproj.paket.targets
src/Bar/Bar.csproj
src/Bar/paket.references

[TWith2Sugars]

how about to keep it all at the same level myproject.projectguid.paket.target?

[agross]

I'm against using GUIDs, they make it hard to reason about the relationship between ??proj and targets file.

[TWith2Sugars]

Not a new guid, the one that has been assigned to the *proj file.

[agross]

I think my argument still holds true ;-)

IMHO, GUIDs are for machines, not for human consumption.

[TWith2Sugars]

It's already in there and referenced by the .sln. This is just to make sure the target file is unique, if we keep the myproject on the front it should be easy to see which target file matches to the _proj. I don't think I've seen that many projects with the same name._proj.

[TWith2Sugars]

Fair enough :)

[TWith2Sugars]

Ah but aren't these target files mainly used for the machines ;)

[forki]

I vote for 2) ;-)

[TWith2Sugars]

@forki I'd be happy with either way, overall having .targets in regardless of where is still a huge benefit.

[forki]

I mean this still looks like good progress:

[TWith2Sugars]

nice

currently sitting here waiting for this to be merged to try it.

[forki]

you might want to test latest 0.19-alpha ;-)

already dogfooding: https://github.com/fsprojects/Paket/commit/99c038156c3df94022d1268f2b05a3a076923657

[TWith2Sugars]

@forki boom, works like a dream.

If everything is inside .paket.target then will you still need the true?

[forki]

If everything is inside .paket.target then will you still need the true?

the what?

[TWith2Sugars]

@forki updated previous comment

[forki]

ah very good point

[forki]

the question is: do we want to support the old mode where we don't create targets files. We could remove A LOT of code if not.

[agross]

I don't see a point in supporting two models. As long as we allow the user to edit targets files and include them in sourc control I think we're golden.

[forki]

"to edit targets files" - mmm. That would be defeating the whole point.

[TWith2Sugars]

I'd vote to support this mode (which people wanted for a while anyway) only.

[agross]

If only to temporarily fix a problem with some reference that's still covered by paket's reference logic. You know what I mean, tomorrow MS invents a new convention for the lib folder and we play whack-a-mole again.

[agross]

So it seems this doesn't work with Paket's own restore via paket.targets. Starting from a clean checkout (no paket.exe, no <project>.paket.targets) the process is as follows:

• Run MSBuild
• MSBuild loads ??proj
• While the project is loaded the <project>.paket.targets file is not yet created, so it is ignored
• paket.targets injects itself to the build pipeline
• paket.targets downloads paket.exe
• paket.targets runs paket.exe restore which now generates <project>.paket.targets
• Build goes on, but the now-available <project>.paket.targets won't be reloaded thanks to MSBuild's declarative nature
• Build fails because lots of referenced assemblies weren't referenced

So it seems we either support auto-restore via paket.targets or we support <project>.paket.targets. You cannot have them both because this creates an ordering requirement that is, from my point of view, not satisfiable using MSBuild.

[forki]

there seems to be no way to get VS restore working - and even the nuget guys don't do that anymore.

[forki]

Seems we have to discontinue automatic VS restore if we want target files. NuGet team came to the same conclusion: http://docs.nuget.org/docs/reference/package-restore

Instead we (as a comunity) could work on @hmemcpy's https://github.com/hmemcpy/Paket.VisualStudio and add restore to it.

[TWith2Sugars]

I personally never used pakets VS auto restore so it's something I wouldn't even notice if it went.

[forki]

I know that at least @ovatsus and @vasily-kirichenko think this is essential.

[TWith2Sugars]

Could this help? http://stackoverflow.com/questions/4919204/how-to-turn-off-caching-of-build-definitions-in-visual-studio

[vasily-kirichenko]

Yes, auto restore is essential. I cannot tell people that they must run something called Paket before they are able to build their solution.

[forki]

@twith2sugars how would that help?

It's really really sad. Instead of supporting PreSolutionBuild event in VS (which is already available in proper MSBuild) they built a nuget addin and shipped it in the VS box. This sucks big time.

Question is if a paket VS addin would be an acceptable solution.

[ctaggart]

To support the people who want to clone the project and build it from Visual Studio without touching the command line, a Paket Visual Studio extension would be an acceptable solution. This is similar to having the NUnit or xUnit extension in order to run unit tests from Visual Studio.

[maartenba]

@forki Nothing of interest in http://www.nuget.org/packages/NuGetEnablePackageRestore/ ? This used to be NuGet package restore long, long ago and worked from within VS.

[ovatsus]

Forcing to install a Paket extension in VS in order for your project to build is even worse than running the paket command line. Any open source .Net project should be able to just build by opening the sln. It's a real barrier to new users if they always have to go look at the readme to check what's the requirement for every specific project

[agross]

@Uli-Armbruster also wouldn't be pleased to learn that automatic restore in VS was discontinued.

[forki]

Ok I think there is no good solution to this. I propose the following:

• we add the targets feature
• people who want to use VS restore have to commit the targets file for now
• people who trust their build.cmd can gitignore the file

This is far from perfect, but even for the autorestore people this might be a small benefit since they know they can nuke the targets file if a merge error occurs.

We will look for better solutions. Maybe we can talk to @JeffHandley and propose to open up the nuget addin.

What do you think?

[TWith2Sugars]

Sounds like a reasonable workaround

[jeffhandley]

As was pointed out, these problems are precisely why we ditched msbuild-integrated restore. Within VS, our extension hooks into the SolutionBuild event and executes restore BEFORE VS launches msbuild. Outside VS, it requires that build scripts run nuget.exe restore. That achieved the results that we wanted: no-touch restore in VS, but very scriptable outside VS. It's worked out great.

[forki]

@JeffHandley we know that. The only issue for paket is: we are not in the Visual Studio box and I don't see a good chance to get in. On Dec 10, 2014 9:25 PM, "Jeff Handley" notifications@github.com wrote:

As was pointed out, these problems are precisely why we ditched msbuild-integrated restore. Within VS, our extension hooks into the SolutionBuild event and executes restore BEFORE VS launches msbuild. Outside VS, it requires that build scripts run nuget.exe restore. That achieved the results that we wanted: no-touch restore in VS, but very scriptable outside VS. It's worked out great.

— Reply to this email directly or view it on GitHub https://github.com/fsprojects/Paket/issues/417#issuecomment-66517859.

[isaacabraham]

@ovatsus why is an extension such a bad thing? there's a plug-in for nuget, there could (should?) be one for paket as well. In addition to doing the Nuget-esque pre-build steps, it could do other things as well such as integrated ability to modify paket via a GUI integrated into VS rather than having to fall back to the command line (or fsx).