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

Switch batching method in order to get all build errors from multi-targeted projects #10872

Open
Tracked by #10846
dsplaisted opened this issue May 16, 2021 · 1 comment
Labels
Category:Quality Week Issues that should be considered for quality week Functionality:Restore Priority:2 Issues for the current backlog. Type:Bug

Comments

@dsplaisted
Copy link

When you try to build a project that requires a workload you don’t have, you will get an error like the following:

NETSDK1147: To build this project, the following workloads must be installed: microsoft-android-sdk-full

For multi-targeted projects, we should report all the missing workloads. Currently, a project multi-targeting to net6.0-android and net6.0-ios will generate separate errors for each inner build:

NETSDK1147: To build this project, the following workloads must be installed: microsoft-android-sdk-full
NETSDK1147: To build this project, the following workloads must be installed: microsoft-ios-sdk-full

This is probably OK, though it might be nice to combine these into a single error that lists both missing workloads.

However, you have to restore before you can build, and currently restore errors out when the first inner build fails and so doesn’t report both missing workloads:

NETSDK1147: To build this project, the following workloads must be installed: microsoft-android-sdk-full

We should fix this so that you get both errors from restore.

Looking at binlogs, I think the reason for the difference is that the _GenerateProjectRestoreGraphAllFrameworks uses the MSBuild task’s Properties parameter with a batched value:

    <MSBuild
      BuildInParallel="$(RestoreBuildInParallel)"
      Projects="$(MSBuildProjectFullPath)"
      Targets="_GenerateProjectRestoreGraphPerFramework"
      Properties="TargetFramework=%(_RestoreTargetFrameworkItems.Identity);
                  $(_GenerateRestoreGraphProjectEntryInputProperties)">
 
      <Output
          TaskParameter="TargetOutputs"
          ItemName="_RestoreGraphEntry" />
    </MSBuild>

On the other hand, to do the inner builds, the DispatchToInnerBuilds target doesn’t use the Properties parameter at all:

    <MSBuild Projects="@(_InnerBuildProjects)"
             Condition="'@(_InnerBuildProjects)' != '' "
             Targets="$(InnerTargets)"
             BuildInParallel="$(BuildInParallel)">
      <Output ItemName="InnerOutput" TaskParameter="TargetOutputs" />

Rather, the Projects items it passes in have AdditionalProperties metadata, which is set in the _ComputeTargetFrameworkItems target:

      <_TargetFramework Include="$(TargetFrameworks)" />
      <!-- Make normalization explicit: Trim; Deduplicate by keeping first occurrence, case insensitive -->
      <_TargetFrameworkNormalized Include="@(_TargetFramework->Trim()->Distinct())" />
      <_InnerBuildProjects Include="$(MSBuildProjectFile)">
        <AdditionalProperties>TargetFramework=%(_TargetFrameworkNormalized.Identity)</AdditionalProperties>
      </_InnerBuildProjects>

Could we update the NuGet targets to use AdditionalProperties metadata instead of the Properties MSBuild task parameter, and see if that results in errors from all of the inner builds being reported?

@nkolev92 nkolev92 added Functionality:Restore Priority:2 Issues for the current backlog. Type:Bug Category:Quality Week Issues that should be considered for quality week labels May 20, 2021
@zkat zkat mentioned this issue Jul 1, 2021
5 tasks
@zkat zkat self-assigned this Jul 6, 2021
@zkat
Copy link
Contributor

zkat commented Jul 12, 2021

Pushing this into #10846 but we'll likely get it done before then. It just won't go into net6.0 GA, most likely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Quality Week Issues that should be considered for quality week Functionality:Restore Priority:2 Issues for the current backlog. Type:Bug
Projects
None yet
Development

No branches or pull requests

4 participants