- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.2k
[release/7.0] Port workload fixes from 6.0 #75486
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
Conversation
| I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
worload changes lgtm, someone else should look over the arcade related stuff
| Tagging subscribers to this area: @dotnet/runtime-infrastructure Issue Details
 Most of the changes are in  
 | 
| @joeloff are these types of changes considered tell-mode or ask-mode? If it's the latter, we need M2 approval. If the former, I can sign-off. | 
| this is build infrastructure supporting workload insertion and is tell mode as discussed in tactics | 
| The failures aren't because of the workload stuff, it looks like it is the sdk bump causing problems. @marcpopMSFT is this because the SDK bumped the 6.0.x runtime? | 
| This can probably wait until servicing has gone out | 
| across multiple VS builds to support multi-targeting. --> | ||
| <ItemGroup> | ||
| <SwixWorkloadPackProjects Include="@(SwixProjects)" Condition="'%(PackageType)' == 'msi-pack'" | ||
| ManifestOutputPath="$(VStemp)\p\%(SwixProjects.SdkFeatureBand)" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come nested in a p directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it to avoid path length issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, p => packs, c => components... We've introduced additional naming elements for multitargeting and if we ever do SxS preview feature bands, then the package IDs can get quite long so I'm just avoiding things early on. And we already include the packs and component parts in the final artifact name, but we need separate folders to zip things up insolation.
| 
 Meaning this should wait until after the RC2 snap? | 
| 
 That's fine, as long as we get this into RC2 after the snap otherwise we're going to have to go through all the manual work again for VS. | 
| There's a conflict. @joeloff can you please resolve it? | 
| 
 Done. I picked the RC1 SDK since that's what  | 
| 
 Thanks @joeloff. I added the "no merge" label. Please remove it when this is ready to merge after the snap, and ping me. | 
| failure is unrelated | 
| Verified a signed internal build and everything looks as expected. | 
| failures are not related  | 
| Failures are #75391 and PinObj_neg doesn't seem to have an associated issue, but there's other runs with the same failure mode - the output doesn't get more descriptive than  | 
Most of the changes are in
engdue to the Arcade update to obtain the latest copy of the workload build tasks. Those changes are unrelated to the workload fix, but appears to be necessary.