Skip to content

Conversation

@ivanpovazan
Copy link
Member

Description

Microsoft.NET.Runtime.MonoTargets.Sdk package is built even when it is not required.

From recently we started seeing runtime official builds doing retries (which is a separate concerning problem).

This is problematic for the Build Workloads (or windows-x64 release Workloads) job of the runtime official build.
Build Workloads depends on the jobs that target mobile platforms, uses their outputs and produces its own artifacts which are later published and used in VS insertion stages. More specifically:

  • Build Workloads dependencies publish Microsoft.NET.Runtime.MonoTargets.Sdk.nupkg/.symbols, while
  • Build Workloads publishes .msi variants of this package

However, it can happen that a non-mobile targeted job failed on the first try of the official build e.g., linux_musl_x64 Mono​ and that Build Workload succeeded producing the .msi variants. However, when linux_musl_x64 Mono​ succeeds on the second try it again publishes a different Microsoft.NET.Runtime.MonoTargets.Sdk.nupkg/.symbols overwriting what was used in the first run of Build Workload, which makes the overall build non-deterministic, ie.:

  • Build Workload published .msi packages with one version of Microsoft.NET.Runtime.MonoTargets.Sdk
  • retry of linux_musl_x64 Mono​ published a different version of Microsoft.NET.Runtime.MonoTargets.Sdk.nupkg/.symbols

As an end result, VS insertion reports issues during Symbol Check as we end up having a miss match between published Microsoft.NET.Runtime.MonoTargets.Sdk.nupkg/.symbols and the .msi variant.

Changes

This PR limits building Microsoft.NET.Runtime.MonoTargets.Sdk package only when we target mobile which helps in prevention of having different versions of the same package as a result of an official build.

Testing

I manually ran an official build with this change in: https://dev.azure.com/dnceng/internal/_build/results?buildId=2493547 and verified that the build succeeds and that the MergedManifest.xml includes all the artifacts as without this change.

Follow-up

Once approved we should backport this to our servicing branches as we have the same issue there as well.

Finally, we should investigate why retries of the official builds are enabled as that seems like an undesired behavior.

@ivanpovazan
Copy link
Member Author

/cc: @vitek-karas

@lewing
Copy link
Member

lewing commented Jul 22, 2024

what is the status of this?

@ivanpovazan
Copy link
Member Author

what is the status of this?

CI failures are unrelated so this is ready to go.

@ivanpovazan ivanpovazan merged commit 951ed61 into dotnet:main Jul 24, 2024
@ivanpovazan
Copy link
Member Author

I will track official builds on main to verify that this change did not cause a regression. Once that is verified I will initiate backports to 8.0 and 6.0.

@ivanpovazan
Copy link
Member Author

Official builds including this change does not seem to have an issue with building required packages. Example of successful build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2502042&view=results

@ivanpovazan
Copy link
Member Author

/backport to release/8.0-staging

@github-actions
Copy link
Contributor

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/10090590446

@ivanpovazan
Copy link
Member Author

/backport to release/6.0-staging

@github-actions
Copy link
Contributor

Started backporting to release/6.0-staging: https://github.com/dotnet/runtime/actions/runs/10090593349

@github-actions
Copy link
Contributor

@ivanpovazan backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Build Mono build tasks packs only when targeting mobile
Using index info to reconstruct a base tree...
M	src/mono/nuget/mono-packages.proj
Falling back to patching base and 3-way merge...
Auto-merging src/mono/nuget/mono-packages.proj
CONFLICT (content): Merge conflict in src/mono/nuget/mono-packages.proj
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Build Mono build tasks packs only when targeting mobile
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions
Copy link
Contributor

@ivanpovazan an error occurred while backporting to release/8.0-staging, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@github-actions
Copy link
Contributor

@ivanpovazan backporting to release/6.0-staging failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Build Mono build tasks packs only when targeting mobile
Using index info to reconstruct a base tree...
M	src/mono/nuget/mono-packages.proj
Falling back to patching base and 3-way merge...
Auto-merging src/mono/nuget/mono-packages.proj
CONFLICT (content): Merge conflict in src/mono/nuget/mono-packages.proj
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Build Mono build tasks packs only when targeting mobile
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions
Copy link
Contributor

@ivanpovazan an error occurred while backporting to release/6.0-staging, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants