Build scripts do not separate Restore from Build, causing confusing behavior on builds after package updates
- Dominant language
- C#
- Stars
- 729
- Forks
- 397
- Avg merge
- 3d 15m
- Merged PRs (30d)
- 149
Description
- [ ] This issue is blocking
- [x] This issue is causing unreasonable pain
This is the root cause of https://github.com/NuGet/Home/issues/12437, which has caused @jaredpar and the Roslyn team a lot of pain recently.
The Arcade `build.proj` can invoke NuGet restore and build in the same build request, often when running `build.ps1 -restore -build`:
https://github.com/dotnet/arcade/blob/3840d434323ccbfc085809e730a3093356450832/src/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj#L254-L270
**This is not correct.** Specifically, it will cause incorrect builds when
* Projects reference NuGet packages
* Those NuGet packages deliver build logic (`.props`/`.targets` files)
* the references to those packages have changed since the last build (removed or version changed).
In that case, MSBuild will use _stale_ versions of the imported build logic in the build, because they are imported in the restore operation and MSBuild tries to retain a coherent set of imports for the duration of a build.
It's not possible to do this correctly today (https://github.com/dotnet/msbuild/issues/2811). However, the Arcade/restore integration can be altered to behave correctly by hooking into MSBuild's `-restore` behavior.
Specifically, instead of passing Restore as a property to `Build.proj`
https://github.com/dotnet/arcade/blob/3840d434323ccbfc085809e730a3093356450832/eng/common/build.ps1#L116
the wrapper script should pass (or not pass) `-restore`, so MSBuild can flush caches at the appropriate point between restore and build.
For that to work, the restore logic would need to be separated from `Execute` into its own target, which must be named `Restore`. MSBuild would then call that target, flush caches, then call `Execute` to do the "build" portion of the build.
This is highly relevant to developer-desktop scenarios where a dev might update a package reference version, and to Roslyn's two-phase build that builds the compiler then rebuilds the repo with it. It is **not** an issue for from-clean builds like most CI/PR/official builds, which I think is why it has been able to linger for so long.
Contributor guide
Assessment
This issue has not been assessed yet.