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

Comments

Projects
None yet
4 participants
@Vilmir

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

This comment has been minimized.

Show comment
Hide comment
@forki

forki Feb 12, 2016

Member

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?

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

This comment has been minimized.

Show comment
Hide comment
@cdrnet

cdrnet Feb 12, 2016

Member

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
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

This comment has been minimized.

Show comment
Hide comment
@Vilmir

Vilmir 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.

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

This comment has been minimized.

Show comment
Hide comment
@forki

forki Feb 12, 2016

Member

question is if we can do something in paket!?

Member

forki commented Feb 12, 2016

question is if we can do something in paket!?

@Vilmir

This comment has been minimized.

Show comment
Hide comment
@Vilmir

Vilmir 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.

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

This comment has been minimized.

Show comment
Hide comment
@forki

forki Feb 15, 2016

Member

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.

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 in c6bfd75 Feb 15, 2016

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Feb 15, 2016

Member

please try latest

Member

forki commented Feb 15, 2016

please try latest

@cdrnet

This comment has been minimized.

Show comment
Hide comment
@cdrnet

cdrnet Feb 15, 2016

Member

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?).

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

This comment has been minimized.

Show comment
Hide comment
@Vilmir

Vilmir Feb 15, 2016

👍 cdrnet

Vilmir commented Feb 15, 2016

👍 cdrnet

@jschroed

This comment has been minimized.

Show comment
Hide comment
@jschroed

jschroed Feb 16, 2016

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)

jschroed commented Feb 16, 2016

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

This comment has been minimized.

Show comment
Hide comment
@Vilmir

Vilmir Feb 16, 2016

FYI I have opened another issue in MsBuild:
Microsoft/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.

Vilmir commented Feb 16, 2016

FYI I have opened another issue in MsBuild:
Microsoft/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

This comment has been minimized.

Show comment
Hide comment
@jschroed

jschroed Feb 17, 2016

@Vilmir, yes that would be perfect for us.

jschroed commented Feb 17, 2016

@Vilmir, yes that would be perfect for us.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Feb 17, 2016

Member

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" notifications@github.com wrote:

FYI I have opened another issue in MsBuild:
Microsoft/msbuild#489 Microsoft/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).

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" notifications@github.com wrote:

FYI I have opened another issue in MsBuild:
Microsoft/msbuild#489 Microsoft/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

This comment has been minimized.

Show comment
Hide comment
@Vilmir

Vilmir Feb 17, 2016

Happy holiday Forki!

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