Skip to content

start-vs-VisualFSharpSln.ps1: build against a locally built hive Roslyn from its own packages - #20528

Open
xperiandri wants to merge 6 commits into
dotnet:mainfrom
xperiandri:start-vs-local-roslyn-packages
Open

xperiandri wants to merge 6 commits into
dotnet:mainfrom
xperiandri:start-vs-local-roslyn-packages

Conversation

@xperiandri

@xperiandri xperiandri commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Description

When the Roslyn deployed in the hive is a local build (a -dev version), the launcher now always overrides the flowed Roslyn packages and adds that checkout's Shipping and NonShipping package folders as restore sources, so F5 builds against the Roslyn it runs.

-RoslynRepo names the checkout (default: a sibling roslyn folder). The launch stops with a clear message when the checkout has no package for that version, or when the global packages folder still holds an earlier pack of it.

Shipped Roslyn versions behave as before.

-InstalledVersions takes the versions from the installed VS instead: Roslyn even when the hive has one of its own, and the System.* and MSBuild packages VS binds to its own copies, wherever the repo flows newer ones. An explicit -RoslynVersion still wins for Roslyn.

Tooling only; no compiler or IDE code path changes, so no release notes entry.

Checklist

  • Test cases added
  • Performance benchmarks added in case of performance changes
  • Release notes entry updated

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

✅ Release-note check exempted

The NO_RELEASE_NOTES label or localization-bot authorship exempts this pull request.

@github-actions github-actions Bot added ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure ⚠️ Affects-Restore Tooling check: PR touches NuGet packages or feeds labels Sep 11, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 🕵️ AI review — verify independently.

Comment thread start-vs-VisualFSharpSln.ps1 Resolved
Comment thread start-vs-VisualFSharpSln.ps1 Resolved
Comment thread start-vs-VisualFSharpSln.ps1 Resolved
Comment thread start-vs-VisualFSharpSln.ps1 Resolved
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 14, 2026
@T-Gro
T-Gro self-requested a review September 14, 2026 14:28
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added the ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager label Sep 24, 2026
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Build-Infra, Affects-Design-Time, Affects-Restore
Affects-Build-Infra: Changes generated MSBuild configuration.
Affects-Design-Time: Changes Visual Studio launch behavior.
Affects-Restore: Changes package restore sources.

Generated by PR Tooling Safety Check · gpt56 1.7M · ◷

Copilot AI balanced review requested due to automatic review settings September 29, 2026 16:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

A missing package folder can break restore, and clearing the shared NuGet cache can disrupt other builds.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

This PR lets the Visual Studio launch script build the F# extension against a locally built Roslyn deployed in the target hive, using that Roslyn build’s own packages.

Changes:

  • Override flowed Roslyn packages whenever the hive uses a -dev build.
  • Add the local Roslyn package folders as restore sources and clear cached copies before restoring.
File Description
start-vs-VisualFSharpSln.ps1 Locates local Roslyn packages and configures the override, restore, and launch.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread start-vs-VisualFSharpSln.ps1 Resolved
Comment thread start-vs-VisualFSharpSln.ps1 Resolved
@xperiandri
xperiandri force-pushed the start-vs-local-roslyn-packages branch from 84f3cdc to c820c98 Compare October 2, 2026 16:03

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 🕵️ Re-reviewed all recent changes as of c820c98; LGTM

@xperiandri
xperiandri force-pushed the start-vs-local-roslyn-packages branch from c820c98 to 4a165d7 Compare October 8, 2026 01:11

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 🕵️ Re-reviewed all recent changes as of 4a165d7; LGTM

@xperiandri
xperiandri force-pushed the start-vs-local-roslyn-packages branch from 4a165d7 to f0f219e Compare October 10, 2026 22:28

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 🕵️ Re-reviewed all recent changes as of f0f219e; LGTM

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The diff is still way too big - apply the NoBloat instructions and CodeCompaction skill.

Especially the massive code comments would benefit not just with drastic reduction, but ideally with well named functions/arguments/bindings/types making the intention clear.

If a well named piece of code just tells what the comment says - delete the comment.

  • This diff adds 18 new code comment lines and 1541 new code comment chars.
  • Comments become stale maintenance state. Keep only durable user-facing contracts; express script intent through names, functions, parameters, and validated data structures.

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Oct 13, 2026
@T-Gro T-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Oct 14, 2026
Comment thread start-vs-VisualFSharpSln.ps1 Resolved
xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 19, 2026
…est head

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 19, 2026
…est head

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 23, 2026
…est head

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 23, 2026
…est head

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@xperiandri
xperiandri force-pushed the start-vs-local-roslyn-packages branch from f0f219e to c9632eb Compare October 29, 2026 19:43
@xperiandri
xperiandri requested a review from T-Gro October 29, 2026 19:44
@xperiandri

Copy link
Copy Markdown
Contributor Author

Started again from main's script: +38/−8 (was +72/−16), with 3 new comment lines / 196 characters (was 18 / 1541). Intent now lives in the names Get-RoslynDevFeeds, Get-StalePackageFolders and ConvertTo-PropsPath.

Covered with 30 -DryRun cases on PowerShell 7 and Windows PowerShell 5.1, and with real NuGet restores against repository names containing %20, %41, $(...), @(...), &, [1], ' and parentheses.

xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 29, 2026
…est head

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 🕵️ Re-reviewed all recent changes as of 1a955d1; LGTM

@xperiandri
xperiandri requested a review from T-Gro November 2, 2026 12:23
xperiandri and others added 6 commits November 4, 2026 02:18
A Roslyn built locally and deployed into the hive is there because
its API surface differs from the flowed package: it is where an
ExternalAccess contract lives before it flows. Matching the minor the
repo flows therefore says nothing about whether the flowed packages
will do, and refusing the dev version left no way to build against
the Roslyn F5 actually runs.

A dev version now always overrides, and the override adds the package
folders of that Roslyn build to RestoreAdditionalProjectSources, since
no feed carries its version. -RoslynRepo names the repository, next to
this one by default, and the script checks that it actually holds the
package before restore fails halfway with NU1101.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d drop stale -dev packages

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Start again from main's script and add only what the feature needs: the nested restructure and the -LiteralPath edits on lines the script already had are gone, and the added comments drop from 18 lines to 3.

A repack that keeps the same -dev version is detected by comparing the restored nupkg with the feed's, and the launch stops with the folders to delete instead of deleting them under whatever still reads the cache.

Feed paths are MSBuild-escaped before the XML escape, so %20, %41, $(...) and @(...) in a repository path reach NuGet unchanged.

The -RoslynRepo default is resolved in the function body, because $PSScriptRoot is empty in a param() default under Windows PowerShell 5.1 when [CmdletBinding()] is present, and the parameter comes last so the existing positional parameters keep their positions.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A Roslyn deployed in the hive always won over the installed VS's, so a stale or locally built one could not be bypassed. With the switch the hive is skipped and the version comes from the installed VS's Microsoft.CodeAnalysis.dll; an explicit -RoslynVersion still takes precedence.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The switch took only the Roslyn version from the installed VS. A hive
deploy still failed when the repo flowed newer System.* or MSBuild
packages than VS binds to, because VS loads its own copies of them.

With -InstalledVersions the launcher also pins those libraries to the
installed VS: System.Collections.Immutable, System.Composition,
System.Diagnostics.DiagnosticSource and System.Reflection.Metadata from
the bindingRedirects in devenv.exe.config, Microsoft.Build, .Framework,
.Tasks.Core and .Utilities.Core from the version of the MSBuild it
ships. A library the repo does not flow newer is left alone, and an
explicit -RoslynVersion still wins for Roslyn.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@xperiandri
xperiandri force-pushed the start-vs-local-roslyn-packages branch from 1a955d1 to cefbe23 Compare November 5, 2026 00:24

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖🕵️ AI review — verify independently.

$assembly = $libraries[$property]
$redirect = if ($config) { $config.SelectSingleNode("//*[local-name()='dependentAssembly'][*[local-name()='assemblyIdentity' and @name='$assembly']]/*[local-name()='bindingRedirect']") }
$dll = Join-Path $msbuild "$assembly.dll"
$installed = if ($redirect) { $v = [version]$redirect.GetAttribute('newVersion'); '{0}.{1}.{2}' -f $v.Major, $v.Minor, $v.Revision }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖🕵️ [P2] MSBuild pinned to 15.1.0 instead of the installed package version when its assembly redirect is present. Use the MSBuild DLL's ProductVersion even when a redirect exists.

<!-- devenv.exe.config, with MSBuild 18.0.2 installed -->
<dependentAssembly>
  <assemblyIdentity name="Microsoft.Build" />
  <bindingRedirect oldVersion="0.0.0.0-15.1.0.0" newVersion="15.1.0.0" />
</dependentAssembly>
.\start-vs-VisualFSharpSln.ps1 -InstalledVersions -DryRun
# MicrosoftBuild 15.1.0; expected 18.0.2

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager ⚠️ Affects-Restore Tooling check: PR touches NuGet packages or feeds AI-reviewed PR reviewed by AI review council NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants