Skip to content

Poison work - reordering and exclusion of non-shipping packages#15347

Merged
NikolaMilosavljevic merged 4 commits intodotnet:mainfrom
NikolaMilosavljevic:poison.work2
Jan 26, 2023
Merged

Poison work - reordering and exclusion of non-shipping packages#15347
NikolaMilosavljevic merged 4 commits intodotnet:mainfrom
NikolaMilosavljevic:poison.work2

Conversation

@NikolaMilosavljevic
Copy link
Copy Markdown
Member

@NikolaMilosavljevic NikolaMilosavljevic commented Jan 25, 2023

Fixes the following issues:
dotnet/source-build#2997
dotnet/source-build#2579
dotnet/source-build#2577
dotnet/source-build#2576
dotnet/source-build#3000

Also removes most of poisoned artifacts, by reordering repo build - runtime will now build before roslyn and linker repos.

New patch is required for runtime repo. Patch backport is also in progress: dotnet/runtime#81180

Backport of arcade overrides: dotnet/arcade#12326

@NikolaMilosavljevic
Copy link
Copy Markdown
Member Author

Runtime patch backport: dotnet/runtime#81180

{
return NonShippingPackagesListFiles
.SelectMany(item => File.ReadAllLines(item.ItemSpec))
.Distinct();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you want to include a ToList() invocation here otherwise the entire linq query is re-evaluated on every iteration of the while loop in GetPoisonedFiles

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in commit 4.

Date: Tue, 24 Jan 2023 18:03:12 +0000
Subject: [PATCH] Allow source-build to set UsingToolMicrosoftNetCompilers
property

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please include the following comment. This makes it easier to manage patches.

backport: https://github.com/dotnet/runtime/pull/81180

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in commit 4.

@@ -0,0 +1,100 @@
<!-- Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. -->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please open a backport PR for the ArcadeOverrides and link to this PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Backport of arcade overrides: dotnet/arcade#12326

Lines="@(IntermediateNonShippingNupkgFile->'%(Filename)%(Extension)')"
Overwrite="true" />

<ItemGroup>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still don't understand what this item group does. Can you help me understand what it is needed for? Who consumes the Content?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nuget packaging consumes Content items. This is the same as old code, above, in GetSupplementalIntermediateNupkgManifest target.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, that is what I was missing. Thanks for explaining.

@mmitche
Copy link
Copy Markdown
Member

mmitche commented Jan 26, 2023

WOO!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants