-
Notifications
You must be signed in to change notification settings - Fork 516
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
[dotnet] Create and ship a WorkloadDependencies.json file. Fixes #21768. #21779
Merged
Merged
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
6735240
[dotnet] Create and ship a WorkloadDependencies.json file. Fixes #21768.
rolfbjarne 2ddbe2a
Merge remote-tracking branch 'origin/main' into dev/rolf/workloaddepe…
rolfbjarne ebbc0be
Merge branch 'main' into dev/rolf/workloaddependencies-json
rolfbjarne 03475b1
Merge branch 'main' into dev/rolf/workloaddependencies-json
dalexsoto 7fc9591
Merge branch 'main' into dev/rolf/workloaddependencies-json
rolfbjarne File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| # generate-workload-dependencies |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| include $(TOP)/scripts/template.mk | ||
| $(eval $(call TemplateScript,GENERATE_WORKLOADDEPENDENCIES_JSON,generate-workloaddependencies-json)) |
39 changes: 39 additions & 0 deletions
39
scripts/generate-workloaddependencies-json/generate-workloaddependencies-json.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // arguments are: <platform> <version> <xcodeVersion> <sdkVersion> <outputPath> | ||
|
|
||
| using System.IO; | ||
| using System.Xml; | ||
|
|
||
| var expectedArgumentCount = 5; | ||
| if (args.Length != expectedArgumentCount) { | ||
| Console.WriteLine ($"Need {expectedArgumentCount} arguments, got {args.Length}"); | ||
| return 1; | ||
| } | ||
|
|
||
| var argumentIndex = 0; | ||
| var platform = args [argumentIndex++]; | ||
| var version = args [argumentIndex++]; | ||
| var xcodeVersion = args [argumentIndex++]; | ||
| var sdkVersion = args [argumentIndex++]; | ||
| var outputPath = args [argumentIndex++]; | ||
|
|
||
| var platformLowerCase = platform.ToLowerInvariant (); | ||
|
|
||
| using (var writer = new StreamWriter (outputPath)) { | ||
| writer.WriteLine ($"{{"); | ||
| writer.WriteLine ($" \"microsoft.net.sdk.{platformLowerCase}\": {{"); | ||
| writer.WriteLine ($" \"workload\": {{"); | ||
| writer.WriteLine ($" \"alias\": [ \"{platformLowerCase}\" ],"); | ||
| writer.WriteLine ($" \"version\": \"{version}\""); | ||
| writer.WriteLine ($" }},"); | ||
| writer.WriteLine ($" \"xcode\": {{"); | ||
| writer.WriteLine ($" \"version\": \"[{xcodeVersion},)\","); | ||
| writer.WriteLine ($" \"recommendedVersion\": \"{xcodeVersion}\""); | ||
| writer.WriteLine ($" }},"); | ||
| writer.WriteLine ($" \"sdk\": {{"); | ||
| writer.WriteLine ($" \"version\": \"{sdkVersion}\""); | ||
| writer.WriteLine ($" }}"); | ||
| writer.WriteLine ($" }}"); | ||
| writer.WriteLine ($"}}"); | ||
| } | ||
|
|
||
| return 0; | ||
5 changes: 5 additions & 0 deletions
5
scripts/generate-workloaddependencies-json/generate-workloaddependencies-json.csproj
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)</TargetFramework> | ||
| </PropertyGroup> | ||
| </Project> |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
microsoft.net.sdk.[platform]the right key? Or should it be the workload name?I think we want a mapping "required Xcode" -> iOS workload, and not a relationship between a "required Xcode" -> "workload manifest"?
Mainly thinking if we need .NET SDK team to review.
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.
Yeah, you could have multiple workloads within one manifest, that has different requirements per workload?
For example, we might have a
android-nativeaotworkload one day that requires the Android NDK and the plainandroidworkload doesn't.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.
I think we do want the actual workload identifier here as is in the PR. The aliases were meant to capture the name we might use in the CLI, but for looking up this type of info, for example,
C:\Program Files\dotnet\sdk-manifests\9.0.100\microsoft.net.sdk.ios\[version]\WorkloadDependencies.jsonis where i'd expect to find the info, so I think it makes sense for the key in this file to match the id here.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 would we define one workload as having different requirements as the other?
For example, here in .NET 9 Preview 6:
We would require API-34 for the
androidworkload and API-35 for the previewandroid-35workload.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.
I think we've mostly sorted this out for now. Android is listing these as optional, and we'll eventually add a way to query the context for a given project state (probably invoking a specific msbuild target on a project) at which point having a
net9.0-android35.0TFM for example could cause the returned info to require the sdk package for android 35 instead of the default 34 fornet9.0as per the file in the workload.In the case of macios, I don't think this will be a problem as the workload doesn't differentiate anywhere at this point that different xcode versions would be needed for different target platform versions for example.