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

Incremental build with MsBuild #1471

Closed
Vilmir opened this issue Feb 12, 2016 · 14 comments
Closed

Incremental build with MsBuild #1471

Vilmir opened this issue Feb 12, 2016 · 14 comments

Comments

@Vilmir
Copy link

Vilmir commented Feb 12, 2016

We have recently updated our product codebase to use Paket instead of Nuget for managing our package dependencies.
Our developers are quite pleased by this move, but we discovered a cumbersome behavior of MsBuild in incremental builds.

Our build script builds all our csproj in a single MsBuild task. It is used as an incremental build, meaning that for most of the situation we do not clean anything before building. This was working well with NuGet. But with Paket, csproj may not be rebuilt while they shall.

It happens when someone updates an existing library package to a new version, without modifying anything in a given project using this lib. Paket updates the paket.lock file and does not modify the csproj using the library, because the unzipped path does not change.

Visual Studio handles this case well, it understands that the referenced assembly did get an update and rebuilds the csproj.
MsBuild does not handle this case, and does not rebuild the csproj. It seems that MsBuild does not consider referenced assemblies as inputs to decide whether or not to rebuild.
The assembly produced by the csproj still uses the old, now depreciated library assembly. If this assembly is called, it fails with a dependency not found error.

I have created a small project here to repro the problem with the library Autofac:
https://github.com/Vilmir/IncrementalBuildCheckWithPaket

Repro steps:

  • Open Developper Command Prompt for VS 2015
  • msbuild.exe build.proj
  • Run bin/Debug/IncrementalBuildCheck.exe it works
  • ".paket/paket.exe" update nuget Autofac version 3.5.2
  • msbuild.exe build.proj
  • Run bin/Debug/IncrementalBuildCheck.exe, you shall see an exception:
System.IO.FileLoadException: Could not load file or assembly 'Autofac, Version=3.0.0.0, Culture=neutral, PublicKeyToken=17863af14b0044da' or
 one of its dependencies. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131
040)
File name: 'Autofac, Version=3.0.0.0, Culture=neutral, PublicKeyToken=17863af14b0044da'
   at IncrementalBuildCheck.Program.TestLibrary()
   at IncrementalBuildCheck.Program.Main(String[] args) in C:\Users\11mvazquez\Desktop\IncrementalBuildCheckWithPaket\Program.cs:line 12
@forki
Copy link
Member

forki commented Feb 12, 2016

That's an interesting point, but I'm not sure what we can do here. The goal was always to keep the csproj as stable as possible.
What do you suggest?

@cdrnet
Copy link
Member

cdrnet commented Feb 12, 2016

An approach that might help here could work along the lines of

paket.exe restore
|> gather list of packages that paket did actually touch while restoring
|> collect projects using these packages using paket find-refs
|> touch all these projects

@Vilmir
Copy link
Author

Vilmir commented Feb 12, 2016

I see 2 options to solve this issue:

  • Cdrnet's solution
  • Ask MsBuild team to add referenced assemblies to the list of files to watch for incremental builds

To me, this is a MsBuild bug, as Visual Studio is smart enough to rebuild.

@forki
Copy link
Member

forki commented Feb 12, 2016

question is if we can do something in paket!?

@Vilmir
Copy link
Author

Vilmir commented Feb 15, 2016

As cdrnet proposed, paket could touch on the file system projects impacted by a new package dependency update. That would force MsBuild to rebuild all those csproj.

@forki
Copy link
Member

forki commented Feb 15, 2016

I guess an easy solution would be to touch all projects when update changed at least one dependency. Of course that is touching more files than needed, but would probably work.

@forki forki closed this as completed in c6bfd75 Feb 15, 2016
@forki
Copy link
Member

forki commented Feb 15, 2016

please try latest

@cdrnet
Copy link
Member

cdrnet commented Feb 15, 2016

It seems to me there are two different situations where MsBuild fails to detect changed dependencies:

A) Active: I make the change to paket.lock myself (paket.exe install, or via add, remove, update)
B) Passive: The change to paket.lock is made by the version control system (git checkout paket.exe restore)

With the latest version, in case A) incremental builds do indeed detect that a rebuild is required, so the fix does work. Thanks!

Case B) is a bit more involved and I don't think touching project files on restore should be the default behavior, but it might be optional (e.g. something along the lines of --touch-affected-refs?).

@Vilmir
Copy link
Author

Vilmir commented Feb 15, 2016

👍 cdrnet

@jschroed
Copy link

Hi guys,

This change causes a bit of an issue with our team. Our default usage for our SCM is to check out files as read-only files. This works great with paket since most of the time, the project files do not need to change. We only check out the files when we have a real change (paket or otherwise).

We do run "paket update" fairly frequently but most of the packages that we use are fixed at a version. Would it be possible to implement a change such that project files are only touched when a project that they referenced has been updated? (I understand that this could be a quite a bit of work to do.) Probably better for us would be to have the default behavior not do this but be possible with a flag (like cdrnet suggested).

We do use MSBuild on our project and have started using incremental build too. I think this would be a good feature to have for our team.

Short version: 1 to cdrnet's Case B suggestion (optional flag - default behavior does not touch project files)

@Vilmir
Copy link
Author

Vilmir commented Feb 16, 2016

FYI I have opened another issue in MsBuild:
dotnet/msbuild#489

@jschroed cdrnet and I are working in the same team. We are interested in implementing cdrnet's proposal that would touch csproj files for projects impacted by an updated library. This behavior would be activated by an optional parameter. Would that work for your case?

@forki let us know if you are OK with this plan.

@jschroed
Copy link

@Vilmir, yes that would be perfect for us.

@forki
Copy link
Member

forki commented Feb 17, 2016

Yes every help would be appreciated. But I'm not near a pc for 2 weeks.
Merge would be afterwards.
On Feb 16, 2016 09:19, "Miguel Vazquez" [email protected] wrote:

FYI I have opened another issue in MsBuild:
dotnet/msbuild#489 dotnet/msbuild#489

@jschroed https://github.com/jschroed cdrnet and I are working in the
same team. We are interested in implementing cdrnet's proposal that would
touch csproj files for projects impacted by an updated library. This
behavior would be activated by an optional parameter. Would that work for
your case?

@forki https://github.com/forki let us know if you are OK with this
plan.


Reply to this email directly or view it on GitHub
#1471 (comment).

@Vilmir
Copy link
Author

Vilmir commented Feb 17, 2016

Happy holiday Forki!

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

4 participants