Use a build properties file instead of custom build actions to copy files

[sharwell]

This change is based on the following two observations:

1. When content items use the CopyToOutputDirectory property instead of build actions to copy contents, the copied files are automatically cleaned up by the build system during clean/rebuild commands. This is especially useful when the names of the copied files change (e.g. when the NuGet package is updated), because old versions of the library are automatically removed the next time a project is built.
2. When using the *.props feature of NuGet, there is no need to update the custom build action (if any) which the user has specified, reducing the likelihood that the user introduces some sort of error when committing the updates to source control.

I tested the functionality of the *.props mechanism locally by manually updating the contents of the .nupkg file and installing it into projects locally. This means the following very important items have not been tested yet.

1. The UpdateLibgit2ToSha.ps1 script should update the properties file, but I have not tested it.
2. The LibGit2Sharp.nuspec file should be updated to properly incorporate the new feature (and remove the now unnecessary installation scripts), but I have not tested that it produces the correct NuGet package during the standard build process.

[nulltoken]

@sharwell I like this! This looks very nice. One question though, do we actually need the file nuget.package/build/LibGit2Sharp.props as part of the commit?

@ethomson as you've played a bit with NuGet, how do you feel about this?

[sharwell]

It would be possible to generate the file during the build, but the file is small and you already make commits to update the native binaries in source control so it wouldn't save much. Including the file makes it easier for a user browsing this repository to understand how the integration is implemented.

[nulltoken]

@sharwell Oh, I get it now. The file is supposed to be part of the Update libgit2 binaries to .... kind of commits.

Out of curiosity, during your tests, have you had to opportunity to test the Visual Studio Publish mechanism? I'm wondering if this would also help solving #597.

[sharwell]

In the original commit, the Publish command would produce two copies of the native binaries in the published website. I believe this is similar to the issues described in #597, #700, and #705 (and maybe others). By changing the item type of the MSBuild items from Content to None, only one copy appears in the published output, and it's located under the bin folder.

[nulltoken]

two copies of the native binaries ... only one copy appears in the published output, and it's located under the bin folder.

Sorry. I'm far from being a MSBuild pipeline expert, so you may have lost me here a bit 😉

With this latest change, the whole NativeBinaries get transparently copied as part of the publishing mechanism?

[sharwell]

Yes, the NativeBinaries folder that appears as bin\Debug\NativeBinaries... for a normal C# project build appears in bin\NativeBinaries... when publishing an MVC project. Unlike commit 2e9bee6, the output only appears under **bin**.

[nulltoken]

@sharwell Thanks for the explanation. Looks very cool! I really like this. Looks like a simple way to solve the publishing issue.

@sitereactor How do you feel about this?

[nulltoken]

@sharwell I've tested it.... and I like this very much! ✨

Could you please squash those two commits together and rebase this PR?

[sharwell]

Done 👍

[Therzok]

One nitpick remaining on my side. Can you make it do this only for Windows?

[sharwell]

@Therzok I'm sure that is possible, but I would lean towards making that a separate request to address. My understanding is this change (as currently implemented) improves the reliability of the build process, package upgrade process, and addresses clear problems with the deployment configuration. The inclusion of Windows binaries in deployment to other systems is sub-optimal, but not problematic because the files would simply go unused.

[Therzok]

I agree with that, I meant that we should log an issue about this.

[nulltoken]

@sharwell Very very nice PR! Thanks a lot for this. 💖

I agree with that, I meant that we should log an issue about this.

Done with #835

[Allov]

Anyone having success publishing with TFS Build Server? I can get this to work with the publish from Visual Studio, the Build server can't do it for some reason ...

[nulltoken]

the Build server can't do it for some reason

Could you please elaborate regarding this. How is it configured, what error do you encounter, traces...?

/cc @ethomson

[Allov]

I'm not an expert with TFS Build, so I'll try to explain to the best of my knowledge.

The way it is configured right now, I think the problem has to do with the way MSDeploy handle the publish. So, the build is done on a machine (A) and once the build is done (which contains the NativeBinaries folder in the bin folder), it starts to deploy using a web service on another machine (B).

What the web service does is that it updates files individually, according to the publish information, but I may not be correct on this. However, what it does right now is it deletes the NativeBinaries files and start updating other file. Since the files aren't in the publish info, the NativeBinaries folder never gets to the other machine (B).

MSDeployPublish:
  Start Web Deploy Publish the Application/package to http://**/MSDEPLOYAGENTSERVICE ...
  Starting Web deployment task from source: package(D:\**\Project.zip) to Destination: auto().
  Deleting filePath (**/api\bin\NativeBinaries\amd64\git2-3f8d005.dll).
  Deleting filePath (**/api\bin\NativeBinaries\amd64\git2-3f8d005.pdb).
  Deleting filePath (**/api\bin\NativeBinaries\amd64\git2-69db893.dll).
  Deleting filePath (**/api\bin\NativeBinaries\amd64\git2-69db893.pdb).
  Deleting filePath (**/api\bin\NativeBinaries\amd64\git2-90befde.dll).
  Deleting filePath (**/api\bin\NativeBinaries\amd64\git2-90befde.pdb).
  Deleting dirPath (**/api\bin\NativeBinaries\amd64).
  Deleting filePath (**/api\bin\NativeBinaries\x86\git2-3f8d005.dll).
  Deleting filePath (**/api\bin\NativeBinaries\x86\git2-3f8d005.pdb).
  Deleting filePath (**/api\bin\NativeBinaries\x86\git2-69db893.dll).
  Deleting filePath (**/api\bin\NativeBinaries\x86\git2-69db893.pdb).
  Deleting filePath (**/api\bin\NativeBinaries\x86\git2-90befde.dll).
  Deleting filePath (**/api\bin\NativeBinaries\x86\git2-90befde.pdb).
  Deleting dirPath (**/api\bin\NativeBinaries\x86).
  Deleting dirPath (**/api\bin\NativeBinaries).
  Deleting filePath (**/api\NativeBinaries\amd64\git2-3f8d005.dll).
  Deleting filePath (**/api\NativeBinaries\amd64\git2-3f8d005.pdb).
  Deleting filePath (**/api\NativeBinaries\amd64\git2-69db893.dll).
  Deleting filePath (**/api\NativeBinaries\amd64\git2-69db893.pdb).
  Deleting filePath (**/api\NativeBinaries\amd64\git2-90befde.dll).
  Deleting filePath (**/api\NativeBinaries\amd64\git2-90befde.pdb).
  Deleting dirPath (**/api\NativeBinaries\amd64).
  Deleting filePath (**/api\NativeBinaries\x86\git2-3f8d005.dll).
  Deleting filePath (**/api\NativeBinaries\x86\git2-3f8d005.pdb).
  Deleting filePath (**/api\NativeBinaries\x86\git2-69db893.dll).
  Deleting filePath (**/api\NativeBinaries\x86\git2-69db893.pdb).
  Deleting filePath (**/api\NativeBinaries\x86\git2-90befde.dll).
  Deleting filePath (**/api\NativeBinaries\x86\git2-90befde.pdb).
  Deleting dirPath (**/api\NativeBinaries\x86).
  Deleting dirPath (**/api\NativeBinaries).
  Updating setAcl (**/api).
  Updating setAcl (**/api).
  ...
  [snip]   
  ...
  Successfully executed Web deployment task.
  Publish Succeeded.    

[Allov]

Also, another way I've been able to make it work (but I'd rather not use this as it's not easy to update), is to include the NativeBinaries folder into the project and create a BeforePublish target in the csproj file:

<Target Name="BeforePublish">
    <Copy SourceFiles="$(SolutionDir)\NativeBinaries" DestinationFolder="$(OutputDir)\bin\NativeBinaries" ContinueOnError="true" />
</Target>

Now, this will work once. The IIS ApplicationPool is keeping a handle on the native binary files and I can't overwrite or delete them on another build. So, I need to kill the pool before doing a build which is not really helpful. Also, I wrote the code with the using keyword too.