[native_toolchain_c] Fix #3327 native_toolchain_c vswhere selection

[Chaos02]

Fixes Native_toolchain_c trying to use Microsoft SQL Server Management Studio as a msvc provider

Related Issues

Fixes #3327

PR Checklist

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.
• I've run dart tool/ci.dart --all locally and resolved all issues identified. This ensures the PR is formatted, has no lint errors, and ran all code generators. This applies to the packages part of the toplevel pubspec.yaml workspace.
• All existing and new tests are passing. I added new tests to check the change I am making.
• The PR is actually solving the issue. PRs that don't solve the issue will be closed. Please be respectful of the maintainers' time. If it's not clear what the issue is, feel free to ask questions on the GitHub issue before submitting a PR.
• I have updated CHANGELOG.md for the relevant packages. (Not needed for small changes such as doc typos).
• I have updated the pubspec package version if necessary.

[github-actions]

Package publishing

Package	Version	Status	Publish tag (post-merge)
package:code_assets	1.0.0	already published at pub.dev
package:data_assets	0.19.6	already published at pub.dev
package:ffi	2.2.0	already published at pub.dev
package:hooks	1.0.3	already published at pub.dev
package:hooks_runner	1.2.1	already published at pub.dev
package:jni_flutter	1.0.1	already published at pub.dev
package:native_toolchain_c	0.18.1	ready to publish	native_toolchain_c-v0.18.1
package:record_use	0.6.1-wip	WIP (no publish necessary)
package:swift2objc	0.2.0-wip	WIP (no publish necessary)
package:swiftgen	0.1.2	already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

[github-actions]

PR Health

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
native_toolchain_c	None	0.18.0	0.18.1	0.18.0	✔️

This check can be disabled by tagging the PR with skip-breaking-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbol	Leaking sources

This check can be disabled by tagging the PR with skip-leaking-check.

[dcharkes]

This is only for if the target is Architecture.x64. For Architecture.arm64 it should be Microsoft.VisualStudio.Component.VC.Tools.ARM64. But, VisualStudioResolver doesn't take a target architecture, we're trying to find one visual studio.

So maybe a better fix is to first try 'Microsoft.VisualStudio.Component.VC.Tools.x86.x64', then add all instances with parseVswhere and then do the same with Microsoft.VisualStudio.Component.VC.Tools.ARM64.

We should probably try to run a CI on runs-on: windows-11-arm in .github/workflows/native.yaml to cover the CI. (But we can do that as a separate PR.)

[Chaos02]

I agree, given that the actual choosing is done in the caller, we should return all of the possible instances and implement choosing logic for the architecture somewhere here perhaps?

https://github.com/Chaos02/native/blob/812bd1bd6376f0f898627a11ea4c935fe3a544c4/pkgs/native_toolchain_c/lib/src/native_toolchain/msvc.dart#L228-L257

or maybe even here?

https://github.com/Chaos02/native/blob/fix/native-toolchain-c-vswhere-requires-3327-clean/pkgs/native_toolchain_c/lib/src/native_toolchain/msvc.dart#L41-L127

I'll suggest something, let me know if it sounds good to you

[Chaos02]

Would you rename the x64 (old) resolver to VisualStudioX64 or leave it be and just suffix the newer ones?
https://github.com/Chaos02/native/blob/d4f3cb61eb8062802e48a2b9038868e4b243f363/pkgs/native_toolchain_c/lib/src/native_toolchain/msvc.dart#L317

like here:
https://github.com/Chaos02/native/blob/d4f3cb61eb8062802e48a2b9038868e4b243f363/pkgs/native_toolchain_c/lib/src/native_toolchain/msvc.dart#L304

[Chaos02]

Would you rename the x64 (old) resolver to VisualStudioX64 or leave it be and just suffix the newer ones?
https://github.com/Chaos02/native/blob/d4f3cb61eb8062802e48a2b9038868e4b243f363/pkgs/native_toolchain_c/lib/src/native_toolchain/msvc.dart#L317

like here:
https://github.com/Chaos02/native/blob/d4f3cb61eb8062802e48a2b9038868e4b243f363/pkgs/native_toolchain_c/lib/src/native_toolchain/msvc.dart#L304

[Chaos02]

Would you rename the x64 (old) resolver to VisualStudioX64 or leave it be and just suffix the newer ones?
https://github.com/Chaos02/native/blob/d4f3cb61eb8062802e48a2b9038868e4b243f363/pkgs/native_toolchain_c/lib/src/native_toolchain/msvc.dart#L317

like here:
https://github.com/Chaos02/native/blob/d4f3cb61eb8062802e48a2b9038868e4b243f363/pkgs/native_toolchain_c/lib/src/native_toolchain/msvc.dart#L304

[dcharkes]

Renaming sounds good.

Thanks for iterating on this PR @Chaos02 🙏

[dcharkes]

extending doesn't seem the right pattern if the x64 and arm64 are next to each other.

Can the VisualStudioResolver get a targetArchitecture param instead?

[dcharkes]

final Tool visualStudioX64? I don't think we export this variable outside the package, so it would not be breaking anything?

And name Visual Studio (x64)? Or does changing the name break things?