dotnet / dotnet/arcade

Build scripts do not separate Restore from Build, causing confusing behavior on builds after package updates

Open
#12,799 10 comments 1 reaction 0 assignees View on GitHub
up-for-grabs
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.