Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[build] Fix local workload template creation #9250

Merged
merged 4 commits into from
Aug 28, 2024
Merged

Conversation

pjcollins
Copy link
Member

@pjcollins pjcollins commented Aug 27, 2024

The targets that set up the local workload have been moved into a
separate ConfigureLocalWorkload.targets, as the Directory.Build.targets
file was getting crowded.

The template pack is no longer deleted by DeleteExtractedWorkloadPacks,
allowing dotnet-local scripts to create new templates again.

The workload files and metadata created when installing the temporary
android-deps workload are now cleaned up, which should fix issues
where bin/Debug/dotnet/dotnet invocations could fail with:

System.Collections.Generic.KeyNotFoundException: The given key 'android-deps' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadResolver.GetManifestFromWorkload(WorkloadId workloadId)
   at Microsoft.DotNet.Cli.WorkloadCommandParser.ShowWorkloadsInfo(ParseResult parseResult, WorkloadInfoHelper workloadInfoHelper, IReporter reporter, String dotnetDir, Boolean showVersion)
   at Microsoft.DotNet.Cli.CommandLineInfo.PrintInfo()
   at Microsoft.DotNet.Cli.Program.ProcessArgs(String[] args, TimeSpan startupTime, ITelemetry telemetryClient)
   at Microsoft.DotNet.Cli.Program.Main(String[] args)

The temporary android-deps workload will now install all of the
runtime workloads that our workload extends, rather than a hard-coded
subset. This should allow the local workload setup to build projects
targeting both net9.0-android and net8.0-android.

The Windows build stage has been updated to use the local workload
configuration for all tests, including creating and building a template.

The targets that set up the local workload have been moved into a
separate ConfigureLocalWorkload.targets, as the Directory.Build.targets
file was getting crowded.

The template pack is no longer deleted by DeleteExtractedWorkloadPacks,
allowing dotnet-local scripts to create new templates again.

The workload files and metadata created when installing the temporary
`android-deps` workload are now cleaned up, which should fix issues
where bin/Debug/dotnet/dotnet invocations could fail with:

    System.Collections.Generic.KeyNotFoundException: The given key 'android-deps' was not present in the dictionary.
       at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
       at Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadResolver.GetManifestFromWorkload(WorkloadId workloadId)
       at Microsoft.DotNet.Cli.WorkloadCommandParser.ShowWorkloadsInfo(ParseResult parseResult, WorkloadInfoHelper workloadInfoHelper, IReporter reporter, String dotnetDir, Boolean showVersion)
       at Microsoft.DotNet.Cli.CommandLineInfo.PrintInfo()
       at Microsoft.DotNet.Cli.Program.ProcessArgs(String[] args, TimeSpan startupTime, ITelemetry telemetryClient)
       at Microsoft.DotNet.Cli.Program.Main(String[] args)

The temporary `android-deps` workload will now install all of the
runtime workloads that our workload extends, rather than a hard-coded
subset. This should allow the local workload setup to build projects
targeting both net9.0-android and net8.0-android.
@pjcollins pjcollins requested a review from jonpryor as a code owner August 27, 2024 17:47
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM. 👍

Comment on lines 65 to 71
if (startIndex != -1) {
startIndex += startElement.Length;
var endIndex = jsonContent.IndexOf("]", startIndex);
if (endIndex != -1) {
ExtendsElement = jsonContent.Substring(startIndex, endIndex - startIndex)?.Trim();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Should we maybe LogError() in the else block for both of these? I imagine some change in the .json could silently fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, I figured we would fail during the android-deps workload install in that case but better to error earlier

@pjcollins pjcollins merged commit 11fe204 into main Aug 28, 2024
57 of 58 checks passed
@pjcollins pjcollins deleted the dev/pjc/local-wl-cleanup branch August 28, 2024 00:05
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants