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.

Sign up for GitHub

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

Jump to bottom

[release/7.0-rc1] Make two manifests one for net7 and one for net6 in 7.0.100 sdk band #163

Merged
merged 22 commits into from
Aug 17, 2022
Merged

[release/7.0-rc1] Make two manifests one for net7 and one for net6 in 7.0.100 sdk band #163

merged 22 commits into from
Aug 17, 2022

Conversation

lewing
Copy link
Member

@lewing lewing commented Aug 1, 2022 •
edited
Loading

@lewing lewing requested a review from steveisok August 1, 2022 21:05
@lewing lewing changed the title 😬 Make two manifests one for net7 and one for net6 in 7.0.100 sdk band Aug 1, 2022
@lewing lewing requested a review from joeloff August 1, 2022 21:12
@joeloff
Copy link
Member

joeloff commented Aug 2, 2022

I'd imagine the tooling doesn't handle this well. We're waiting on porting the new new tooling to Arcade/main after we get runtime/release-6.0 updated because porting the Arcade changes will trigger automatic code flows, but you will need it to separate the generated manifests I think to avoid weirdness in VS authoring.

@Redth Redth mentioned this pull request Aug 4, 2022
Closed
@marcpopMSFT @akoeplinger
Try to fix up the multi-targeted build. This will likely need an arca…
baccbfa 
…de change to enable manifests withtout packs and an additional change once arcade is updated.
@akoeplinger akoeplinger mentioned this pull request Aug 9, 2022
Closed
akoeplinger
akoeplinger reviewed Aug 11, 2022
View reviewed changes
eng/emsdk.proj Outdated Show resolved Hide resolved
@lewing lewing marked this pull request as ready for review August 16, 2022 18:08
@lewing
Copy link
Member Author

lewing commented Aug 16, 2022

This should probably go straight into release/7.0 at this point

@lewing
Copy link
Member Author

lewing commented Aug 16, 2022

@joeloff what is left here? I assume we still need a newer arcade, what changes do the tasks need?

@joeloff
Copy link
Member

joeloff commented Aug 16, 2022

@joeloff what is left here? I assume we still need a newer arcade, what changes do the tasks need?

This probably has the best example: https://github.com/dotnet/runtime/pull/72947

The manifest task goes away and some of the metadata we separated out can all go on the same items. Note the MsiVersion parameter that's required (it's a bug I plan on fixing).

We can also better support building manifest for multiple feature bands. Don't think EMSDK has done that, but the new task is better at accidentally building the same MSI twice when processing multiple manifest files.

@lewing lewing requested a review from radical August 16, 2022 21:07
@radical radical changed the base branch from main to release/7.0-rc1 August 16, 2022 22:40
@lewing
Copy link
Member Author

lewing commented Aug 17, 2022

@radical while the size issue is blocking I don't think we can add the check before we fix the size ;)

@radical radical changed the title Make two manifests one for net7 and one for net6 in 7.0.100 sdk band [release/7.0-rc1] Make two manifests one for net7 and one for net6 in 7.0.100 sdk band Aug 17, 2022
@lewing
Copy link
Member Author

lewing commented Aug 17, 2022 •
edited
Loading

@joeloff ok now this looks like it is failing because the .net6 packs don't exist as part of this build which is true

@lewing lewing requested a review from hoyosjs August 17, 2022 17:15
@lewing
Copy link
Member Author

lewing commented Aug 17, 2022 •
edited
Loading

without AllowMissingPacks we get

D:\a\1\s\eng\workloads\workloads.csproj(79,5): error : System.IO.FileNotFoundException: NuGet package not found
D:\a\1\s\eng\workloads\workloads.csproj(79,5): error : File name: 'D:\a\1\s\artifacts\packages\Release\Shipping/Microsoft.NET.Runtime.Emscripten.2.0.23.Node.win-x64.6.0.9.nupkg'
D:\a\1\s\eng\workloads\workloads.csproj(79,5): error :    at Microsoft.DotNet.Build.Tasks.Workloads.CreateVisualStudioWorkload.Execute() in /_/src/Microsoft.DotNet.Build.Tasks.Workloads/src/CreateVisualStudioWorkload.wix.cs:line 272
##[error]eng\workloads\workloads.csproj(79,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) System.IO.FileNotFoundException: NuGet package not found

With it:

eng\workloads\workloads.csproj(79,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) System.Collections.Generic.KeyNotFoundException: The given key 'Microsoft.NET.Runtime.Emscripten.Node' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Microsoft.DotNet.Build.Tasks.Workloads.CreateVisualStudioWorkload.Execute() in /_/src/Microsoft.DotNet.Build.Tasks.Workloads/src/CreateVisualStudioWorkload.wix.cs:line 250

@lewing lewing merged commit db10127 into dotnet:release/7.0-rc1 Aug 17, 2022
@lewing lewing deleted the gar branch August 17, 2022 19:39
lewing added a commit that referenced this pull request Aug 18, 2022
@lewing @hoyosjs
@marcpopMSFT @radical @akoeplinger
[release/7.0] Sync 7.0-rc1 -> 7.0 (#173)
2d5d286 
* Workload changes and arcade update (#171) (#172)

* Update arcade to 7.0.0-beta.22416.1

* Workload changes

Co-authored-by: Larry Ewing <[email protected]>

Co-authored-by: Juan Hoyos <[email protected]>

* [release/7.0-rc1] Make two manifests one for net7 and one for net6 in 7.0.100 sdk band (#163)

* Here goes nothing

* Update WorkloadManifest.json.in

* Update WorkloadManifest.json.in

* Try to fix up the multi-targeted build. This will likely need an arcade change to enable manifests withtout packs and an additional change once arcade is updated.

* Update eng/emsdk.proj

Co-authored-by: Alexander Köplinger <[email protected]>

* Rename .pkgproj files

* Update PackageVersionNet6 to 6.0.9

* Fix the location of the .net7 pkgproj

Co-authored-by: Marc Paine <[email protected]>
Co-authored-by: Ankit Jain <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>

* [release/7.0-rc1] Shorten the name more (#174)

* Shorten the name more

* [release/7.0-rc1] Check nuget sizes and fix them (#175)

* yml: Fail if nuget size exceeds 250MiB, on linux, and macos

* Fix the case for no errors

* Remove unused binaries

Co-authored-by: Ankit Jain <[email protected]>

* Add PowerShell-based NuGet size check script and use it on Windows (#176)

* Add PowerShell-based NuGet size check script and use it on Windows

* Update azure-pipelines.yml

Co-authored-by: Juan Hoyos <[email protected]>
Co-authored-by: Marc Paine <[email protected]>
Co-authored-by: Ankit Jain <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
lewing added a commit that referenced this pull request Aug 22, 2022
@lewing @hoyosjs
@marcpopMSFT @radical @akoeplinger
Sync rc1 again (#188)
81cc0e4 
* Workload changes and arcade update (#171) (#172)

* Update arcade to 7.0.0-beta.22416.1

* Workload changes

Co-authored-by: Larry Ewing <[email protected]>

Co-authored-by: Juan Hoyos <[email protected]>

* [release/7.0-rc1] Make two manifests one for net7 and one for net6 in 7.0.100 sdk band (#163)

* Here goes nothing

* Update WorkloadManifest.json.in

* Update WorkloadManifest.json.in

* Try to fix up the multi-targeted build. This will likely need an arcade change to enable manifests withtout packs and an additional change once arcade is updated.

* Update eng/emsdk.proj

Co-authored-by: Alexander Köplinger <[email protected]>

* Rename .pkgproj files

* Update PackageVersionNet6 to 6.0.9

* Fix the location of the .net7 pkgproj

Co-authored-by: Marc Paine <[email protected]>
Co-authored-by: Ankit Jain <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>

* [release/7.0-rc1] Shorten the name more (#174)

* Shorten the name more

* [release/7.0-rc1] Check nuget sizes and fix them (#175)

* yml: Fail if nuget size exceeds 250MiB, on linux, and macos

* Fix the case for no errors

* Remove unused binaries

Co-authored-by: Ankit Jain <[email protected]>

* Add PowerShell-based NuGet size check script and use it on Windows (#176)

* Add PowerShell-based NuGet size check script and use it on Windows

* Update azure-pipelines.yml

* Roll back 6.0.x to 6.0.8 so that packages exist (#177)

* Revert "Roll back 6.0.x to 6.0.8 so that packages exist (#177)" (#178)

This reverts commit b7a5ae2.

* make a 6.0.4 build (#180)

* Revert "make a 6.0.4 build (#180)" (#181)

This reverts commit eb3232e.

* [release/7.0] Update arcade to 7.0.0-beta.22418.4 (#182)

* WorkloadManifest.targets: fix condition (#183)

Co-authored-by: Juan Hoyos <[email protected]>
Co-authored-by: Marc Paine <[email protected]>
Co-authored-by: Ankit Jain <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akoeplinger akoeplinger akoeplinger left review comments

@steveisok steveisok Awaiting requested review from steveisok

@joeloff joeloff Awaiting requested review from joeloff

@radical radical Awaiting requested review from radical

@hoyosjs hoyosjs Awaiting requested review from hoyosjs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lewing @joeloff @akoeplinger @marcpopMSFT @radical