Skip to content

[release/7.0] Port workload fixes from 6.0 - #75486

Merged
hoyosjs merged 3 commits into
dotnet:release/7.0from
joeloff:workload-update7
Sep 14, 2022
Merged

[release/7.0] Port workload fixes from 6.0#75486
hoyosjs merged 3 commits into
dotnet:release/7.0from
joeloff:workload-update7

Conversation

@joeloff

Copy link
Copy Markdown
Member
  • The updated task now provides metadata that will allow us to split the VSDROP generation into two separate archives. One which only contains workload packs (needed to share DROPs across VS versions to support multi-targeting) and another that only contains the manifest installers and workload components.
  • The ZIP archive names have been updated to disambiguate its contents and support automatic drop creation during staging.

Most of the changes are in eng due 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.

@ghost

Copy link
Copy Markdown

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.

@ghostghost assigned joeloffSep 12, 2022
@joeloff
joeloff requested a review from lewingSeptember 12, 2022 21:31
@lewinglewing changed the title Port workload fixes from 6.0[release/7.0] Port workload fixes from 6.0Sep 12, 2022

@lewinglewing left a comment

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.

worload changes lgtm, someone else should look over the arcade related stuff

@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details
  • The updated task now provides metadata that will allow us to split the VSDROP generation into two separate archives. One which only contains workload packs (needed to share DROPs across VS versions to support multi-targeting) and another that only contains the manifest installers and workload components.
  • The ZIP archive names have been updated to disambiguate its contents and support automatic drop creation during staging.

Most of the changes are in eng due 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.

Author:joeloff
Assignees:joeloff
Labels:

area-Infrastructure

Milestone:-

@carlossanlop

carlossanlop commented Sep 12, 2022

Copy link
Copy Markdown
Contributor

@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.

@lewing

Copy link
Copy Markdown
Member

this is build infrastructure supporting workload insertion and is tell mode as discussed in tactics

@carlossanlop

carlossanlop commented Sep 12, 2022

Copy link
Copy Markdown
Contributor

Ok thanks, @lewing (I wasn't sure, I had not seen changes related to distros before).

There are a lot of failures. Are any of them related to this? @joeloff

@lewing

lewing commented Sep 12, 2022

Copy link
Copy Markdown
Member

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?

@lewing

Copy link
Copy Markdown
Member

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)"

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.

How come nested in a p directory?

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.

Is it to avoid path length issues?

Copy link
Copy Markdown
MemberAuthor

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.

@carlossanlop

Copy link
Copy Markdown
Contributor

This can probably wait until servicing has gone out

Meaning this should wait until after the RC2 snap?

@joeloff

Copy link
Copy Markdown
MemberAuthor

This can probably wait until servicing has gone out

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.

@carlossanlop

Copy link
Copy Markdown
Contributor

There's a conflict. @joeloff can you please resolve it?

@carlossanlopcarlossanlop added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 13, 2022
@joeloff

Copy link
Copy Markdown
MemberAuthor

There's a conflict. @joeloff can you please resolve it?

Done. I picked the RC1 SDK since that's what darc pulled in as part of the Arcade update.

@carlossanlop

Copy link
Copy Markdown
Contributor

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.

Thanks @joeloff. I added the "no merge" label. Please remove it when this is ready to merge after the snap, and ping me.

@lewing

Copy link
Copy Markdown
Member

failure is unrelated

@joeloffjoeloff removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 14, 2022
@joeloff

Copy link
Copy Markdown
MemberAuthor

Verified a signed internal build and everything looks as expected.

@lewing

Copy link
Copy Markdown
Member

failures are not related

 MSBuild version 17.4.0-preview-22416-02+5d102ae37 for .NET
/datadisks/disk1/work/B5AA0966/p/wasm-test-runner/WasmTestRunner.proj : error : Failed to resolve SDK 'Microsoft.Build.NoTargets/3.5.0'. Package restore was successful but a package with the ID of "Microsoft.Build.NoTargets" was not installed.
/datadisks/disk1/work/B5AA0966/p/wasm-test-runner/WasmTestRunner.proj : error MSB4236: The SDK 'Microsoft.Build.NoTargets/3.5.0' specified could not be found.
Test Harness Exitcode is : 1
To run the test:
> set CORE_ROOT=/datadisks/disk1/work/B5AA0966/p
> /datadisks/disk1/work/B5AA0966/w/A7060920/e/JIT/HardwareIntrinsics/X86/Avx2/LoadAlignedVector256NonTemporal_ro/LoadAlignedVector256NonTemporal_ro.sh

@hoyosjs

Copy link
Copy Markdown
Member

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 Test Harness Exitcode is : 1 and there's not much to go by nonetheless... Merging this as tell mode for workloads work. cc: @jeffschwMSFT@marek-safar

@hoyosjs
hoyosjs merged commit 04d61ca into dotnet:release/7.0Sep 14, 2022
@ghostghost locked as resolved and limited conversation to collaborators Oct 15, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@joeloff@carlossanlop@lewing@hoyosjs@ViktorHofer@teo-tsirpanis