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

Fix build race #70261

Merged
merged 2 commits into from
Oct 12, 2023
Merged

Fix build race #70261

merged 2 commits into from
Oct 12, 2023

Conversation

jasonmalinowski
Copy link
Member

By setting RuntimeIdentifier= we were preventing MSBuild from only running that once, since the regular invocation of the build would set the TFM but not the RID. This doesn't seem necessary anymore, so we can just remove it.

@jasonmalinowski jasonmalinowski requested a review from a team as a code owner October 5, 2023 21:40
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 5, 2023
We were accidentally creating a number of duplicate builds for a few
reasons:

- When we invoked the build for the BuildHost, that set RuntimeIdentifer=
  which had the effect of meaning MSBuild did not consider that to
  be the same build as a "regular" build that sets just the TFM.
  By setting RemoveProperties instead of that property, we can
  unify builds better.

- When we are packing each RID separately, that was also triggering
  parallel builds of the other projects since each build was also
  passing PackRuntimeIdentifier to the dependencies of the language
  server. But that property doesn't impact anything there, so
  that's unnecessary and just creates additional races.
which causes a lot of duplicate building. This removes it. -->
<ItemDefinitionGroup>
<ProjectReference>
<GlobalPropertiesToRemove>PackRuntimeIdentifier</GlobalPropertiesToRemove>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know what is going on here. If you can verify that all the dependency dlls are R2R images in the pack output I'm good.

Without specifically looking for items under an ItemGroup, it was
tripped up by ItemDefinitionGroups.
@jasonmalinowski jasonmalinowski requested a review from a team as a code owner October 11, 2023 21:02
@jasonmalinowski jasonmalinowski self-assigned this Oct 11, 2023
@jasonmalinowski jasonmalinowski merged commit 17e4696 into dotnet:main Oct 12, 2023
24 checks passed
@jasonmalinowski jasonmalinowski deleted the fix-build-race branch October 12, 2023 05:01
@ghost ghost added this to the Next milestone Oct 12, 2023
@jjonescz jjonescz modified the milestones: Next, 17.9 P1 Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants