Msbuild: MSBuild-task equivalent of "msbuild.exe /restore /t:Target"

[natemcmaster]

https://github.com/Microsoft/msbuild/pull/2414 added support for msbuild /restore /t:Target which executes /t:Restore in a new build context before invoking /t:Target.

Is it possible to perform the same behavior from the MSBuild task?

For example, i'd love to be able to do this

<Project>
  <Target Name="CiBuild">
      <MSBuild Projects="MyProj.sln"
               Targets="Build"
               Restore="true" />
   </Target>
</Project>

cc @jeffkl

[jeffkl]

At this time, it is not possible to do this. The logic for /restore is pretty proprietary to MSBuild.exe while the <MSBuild /> task is using a completely different code path. That said, anything is possible, however we do not currently have plans to implement this.

[natemcmaster]

That's what I feared, after taking a cursory glance at #2414.

[AndyGerlicher]

Can you just turn that into two MSBuild task invocations and set different global properties? Targets="Restore" Properties="IsRestoring=true" then call Build.

[natemcmaster]

That's what we've done in the past to force project re-evaluation after restore. The main problem with that is mostly usability. It's not immediately obvious to devs new writing MSBuild--or even experienced ones--why you need separate MSBuild-task invocations, and why you need a "dummy" property on the restore one.

[dasMulli]

For CI projects, I've been moving them to having an explicit Restore target in them so I can call them through msbuild /restore ci-build.proj or just dotnet build ci-build.proj.
https://gist.github.com/dasMulli/fdc9bf5c433175f8feb638d3eed41b68

[nguerrera]

The main problem with that is mostly usability. It's not immediately obvious to devs new writing MSBuild--or even experienced ones--why you need separate MSBuild-task invocations, and why you need a "dummy" property on the restore one.

+100 and TIL that it's not correct either. I'll let @rainersigwald explain why. ;)

[rainersigwald]

The reason it's not sufficient to use a global property is that MSBuild maintains a ProjectRootElementCache of the parsed XML of imported files. It's there for performance (it would be slow and unnecessary to load Microsoft.Common.CurrentVersion.targets for every project in your build) but _also_ for consistency, ensuring that if you edit a common import mid-build you don't have half of your projects with the old version and half with the new version.

Self-contained demo of the problem:

<Project DefaultTargets="Build">
 <PropertyGroup>
  <Import>import.props</Import>
 </PropertyGroup>

 <Import Project="$(Import)" Condition="Exists($(Import))" />

 <PropertyGroup>
  <ReadValue Condition="'$(ReadValue)' == ''">0</ReadValue>
 </PropertyGroup>


 <Target Name="Restore">
  <PropertyGroup>
   <WriteValue>$([MSBuild]::Add($(ReadValue), 1))</WriteValue>
 </PropertyGroup>
  <ItemGroup>
   <IncludeLines Include="&lt;Project&gt;" />
   <IncludeLines Include=" &lt;PropertyGroup&gt;" />
   <IncludeLines Include="  &lt;ReadValue&gt;$(WriteValue)&lt;/ReadValue&gt;" />
   <IncludeLines Include=" &lt;/PropertyGroup&gt;" />
   <IncludeLines Include="&lt;/Project&gt;" />
  </ItemGroup>

  <WriteLinesToFile File="$(Import)"
                    Lines="@(IncludeLines)"
                    Overwrite="true" />

  <Message Importance="High" Text="Wrote value: $(WriteValue)" />
 </Target>

 <Target Name="Build">
  <Message Importance="High" Text="Read value: $(ReadValue)" />
 </Target>

 <Target Name="BuildWithDifferentGlobalProperties">
  <MSBuild Projects="$(MSBuildThisFileFullPath)"
           Targets="Restore"
           Properties="_DummyProperty=Restore" />
  <MSBuild Projects="$(MSBuildThisFileFullPath)"
           Targets="Build"
           Properties="_DummyProperty=Build" />
 </Target>
</Project>

s:\repro\Microsoft\msbuild\issues\2811>msbuild /nologo /v:m demo-global-props-insufficient.proj /t:Build;Restore
  Read value: 0
  Wrote value: 1

s:\repro\Microsoft\msbuild\issues\2811>msbuild /nologo /v:m demo-global-props-insufficient.proj /t:Build;Restore
  Read value: 1
  Wrote value: 2

s:\repro\Microsoft\msbuild\issues\2811>msbuild /nologo /v:m demo-global-props-insufficient.proj /t:BuildWithDifferentGlobalProperties
  Wrote value: 3
  Read value: 2

s:\repro\Microsoft\msbuild\issues\2811>msbuild /nologo /v:m demo-global-props-insufficient.proj /t:BuildWithDifferentGlobalProperties
  Wrote value: 4
  Read value: 3

s:\repro\Microsoft\msbuild\issues\2811>msbuild /nologo /v:m demo-global-props-insufficient.proj /restore /t:Build
  Wrote value: 5
  Read value: 5

s:\repro\Microsoft\msbuild\issues\2811>msbuild /nologo /v:m demo-global-props-insufficient.proj /restore /t:Build
  Wrote value: 6
  Read value: 6

I'm not totally sure I buy the consistency argument for requiring the cache within a build, but I also don't know how to validate that relaxing it _won't_ break some VS customer somewhere.

[rainersigwald]

We had a bug for that I just ran across: https://github.com/Microsoft/msbuild/issues/1185, including a case where the "snapshot" behavior is required for correctness.

[msshapira]

Any update on this issue? Is there a good workaround?

[PhilipDaniels]

Just wanted to say that I have wasted 2 days of time because /t:restore;build does not work reliably, before coming across this amazing comment https://github.com/Microsoft/msbuild/issues/3000#issuecomment-417675215 Judging by queries on Stack Overflow I am not the only one. Incidentally, I have not seen this documented anywhere in the official docs.

The suggested workaround of using /restore did not work either, I had to add a specific old-style build step of calling nuget.exe to do the restore. This was on a Bamboo CI server.

If /t:restore and /restore cannot be made to work properly then you should remove them or print out a message recommending alternatives and then fail the build. At least that way people won't waste their time.

[aolszowka]

@PhilipDaniels the problem is /t:restore DOES work properly; however it must be the only task called during that process; /t:restore;build DOES NOT work.

That is why we have several tasks that look like this:

  <Target Name="TIMSNETNuGetRestore" DependsOnTargets="GetTIMSNETSolution">
    <Message Text="Restoring NuGet Packages for TIMSNET" Importance="high"/>

    <!-- You're going to be super temped to combine this with BuildTIMSNET -->
    <!-- However you cannot because of bugs in the context; see the common -->
    <!-- "Restore" target in ComputersUnlimited.Build.All.msbuild          -->
    <MSBuild
      Projects="@(ProjectsToBuild)"
      Properties="PostBuildEvent="
      Targets="Restore"
      BuildInParallel="true" />
  </Target>

And its caller

  <!--**********************************************************************-->
  <!--* Restore (DO NOT CHANGE THIS NAME)                                  *-->
  <!--*   This is the target that will be executed in its own context when *-->
  <!--* you do an "msbuild /restore" and this is where all of the NuGet    *-->
  <!--* packages should be restored.                                       *-->
  <!--*                                                                    *-->
  <!--* See the Following:                                                 *-->
  <!--*    - https://github.com/Microsoft/msbuild/issues/2811              *-->
  <!--*    - https://github.com/Microsoft/msbuild/issues/3000              *-->
  <!--*      Specifically the @aolszowka comments                          *-->
  <!--**********************************************************************-->
  <Target Name="Restore">
    <CallTarget Targets="TIMSNETNuGetRestore" />
  </Target>

[rainersigwald]

The suggested workaround of using /restore did not work either, I had to add a specific old-style build step of calling nuget.exe to do the restore. This was on a Bamboo CI server.

@PhilipDaniels Can you please open a new issue describing what you did and what went wrong? That's expected to work.

[PhilipDaniels]

The suggested workaround of using /restore did not work either, I had to add a specific old-style build step of calling nuget.exe to do the restore. This was on a Bamboo CI server.

@PhilipDaniels Can you please open a new issue describing what you did and what went wrong? That's expected to work.

@rainersigwald Done https://github.com/Microsoft/msbuild/issues/4071

[ViktorHofer]

@rainersigwald @AndyGerlicher @jeffkl

At this time, it is not possible to do this. The logic for /restore is pretty proprietary to MSBuild.exe while the task is using a completely different code path. That said, anything is possible, however we do not currently have plans to implement this.

Are we now, three years later, in a better position to support this? I just stumbled upon this as I was trying to avoid an unnecessary glob (which matters in large repos like dotnet/runtime).

[rainersigwald]

@ViktorHofer No, the essential complexity remains. Can you elaborate on your "unnecessary glob" scenario? I don't see how that ties in to this.