Skip to content

Conversation

@joeloff
Copy link
Member

@joeloff joeloff commented Sep 23, 2024

Description

Visual Studio publishes all the component and component group IDs from their catalog to support extension developers. The norm is to follow their naming convention and include a prefix like Microsoft.VisualStudio.Component in the ID. .NET optional workloads (including abstract workloads) generate components and component groups that matches the .NET workload ID. This enables features like IPA to easily locate components when SDK resolvers report missing optional workloads.

Visual Studio asked that we follow their guidelines so starting in .NET 9, we will prefix our components with 'Microsoft.NET.Component'. Repos will be able to opt-in and generate new IDs.

Risk

Medium - Runtime, EMSDK, SDK, Maui all need to use the new feature simultaneously and we won't discover all the issues until we can insert new workloads and do full E2E testing. Risk is mitigated by having this as an opt-in feature.

Testing

We have sufficient unit tests to feel confident about the change. We'll be able to further assess problems once the first repos onboard to it and again when we have full E2E testing.

@joeloff joeloff added the servicing-consider Servicing ask-mode label Sep 23, 2024
@joeloff joeloff requested a review from a team September 23, 2024 21:12
Comment on lines 80 to 81
safeId = safeId.StartsWith("Microsoft.NET.", StringComparison.OrdinalIgnoreCase) ? safeId.Substring(14) :
safeId.StartsWith("Microsoft.", StringComparison.OrdinalIgnoreCase) ? safeId.Substring(10) :
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to see this as a loop over the well-known prefixes, without hard-coding the length of each one (ie the 10 and 14 constants).

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, thank you.

Comment on lines +98 to +99
i => i.ItemSpec.Contains("Microsoft.NET.Component.sdk.emscripten.5.6.swixproj")).ItemSpec), "component.swr"));
Assert.Contains("package name=Microsoft.NET.Component.sdk.emscripten", componentSwr);
Copy link
Member

Choose a reason for hiding this comment

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

Does this casing make sense? The first half uses proper upper case and the second half uses lowercase only. Is that how VS does it too?

Copy link
Member Author

Choose a reason for hiding this comment

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

.NET workload IDs are case-sensitive - And while we can probably fix this for some, others might prove hard, e.g. maccatalyst. If we followed the VS model 100%, we'd probably want to end up with MacCatalyst.

Comment on lines +180 to +181
// Component prefix is defaults to null when creating a SWIX component, so these values remain
// unchanged.
Copy link
Member

Choose a reason for hiding this comment

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

nit: Just a random question. Is the split in the comment line something done automatically or something you manually pressed "Enter" and then started a new line?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's me

@rbhanda rbhanda added servicing-approved Approved for servicing and removed servicing-consider Servicing ask-mode labels Sep 24, 2024
@rbhanda rbhanda modified the milestones: 9.0.9, 9.0.0 Sep 24, 2024
@joeloff joeloff merged commit b2d3a3a into dotnet:release/9.0 Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

servicing-approved Approved for servicing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants