Skip to content

Upgrade Ionide.ProjInfo to 0.74.2#1055

Open
Smaug123 wants to merge 2 commits intofsprojects:mainfrom
Smaug123:upgrade-projinfo
Open

Upgrade Ionide.ProjInfo to 0.74.2#1055
Smaug123 wants to merge 2 commits intofsprojects:mainfrom
Smaug123:upgrade-projinfo

Conversation

@Smaug123
Copy link

@Smaug123 Smaug123 commented Feb 28, 2026

PR by Claude (Opus 4.6).

Fixes #1054 .

My knowledge of the MsBuild packages is exceptionally bad; I've tried understanding how it works in the past and simply failed. Claude already got this wrong once in a way CI caught but which I couldn't predict would fail.

@Smaug123 Smaug123 changed the title Upgrade Ionide.ProjInfo to 0.74.1 Upgrade Ionide.ProjInfo to 0.74.2 Feb 28, 2026
<PackageReference Include="CommandLineParser" />
<PackageReference Include="Ionide.ProjInfo" />
<PackageReference Include="Ionide.ProjInfo.Sln" />
<PackageReference Include="Microsoft.Build" IncludeAssets="compile" ExcludeAssets="runtime" PrivateAssets="all" />
Copy link
Author

Choose a reason for hiding this comment

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

| Ok loadedProject ->
match ProjectLoader.getLoadedProjectInfo projectFile customProperties loadedProject with
| Ok(ProjectLoader.LoadedProjectInfo.StandardProjectInfo projOptions) -> Ok projOptions
| Ok _ -> Error $"project '%s{projectFile}' is not a standard project"
Copy link
Author

@Smaug123 Smaug123 Feb 28, 2026

Choose a reason for hiding this comment

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

The other cases would have failed anyway, says Claude; we should just be moving the error earlier. Claude is confident of that statement, but I haven't completely tracked everything down. Here's a partial analysis, with additional comments inline in code snippets from Claude.

The type definition is https://github.com/ionide/proj-info/blob/35d34d3e2181e476d3c008c291670253eee35afc/src/Ionide.ProjInfo/Library.fs#L997 .

  type LoadedProjectInfo =
      | StandardProjectInfo of ProjectOptions   // Normal .fsproj/.csproj with design-time build
      | TraversalProjectInfo of ProjectReference list  // IsTraversal=true (e.g., dirs.proj aggregators)
      | OtherProjectInfo of ProjectInstance     // No design-time targets (e.g., .shproj)

proj-info classifies them at https://github.com/ionide/proj-info/blob/35d34d3e2181e476d3c008c291670253eee35afc/src/Ionide.ProjInfo/Library.fs#L661

  match pi with
  | IsTraversal -> Ok(TraversalProject pi)           // Has IsTraversal=true property
  | DesignTimeCapable designTimeTargets -> doDesignTimeBuild ()  // Has all design-time build targets
  | _ -> Ok(Other pi)                                // Lacks design-time targets

On main which uses v0.62.0 of proj-info, loadProject runs design-time build unconditionally: https://github.com/ionide/proj-info/blob/v0.62.0/src/Ionide.ProjInfo/Library.fs#L542

Claude claims that a traversal project or a shared project wouldn't have ResolveAssemblyReferencesDesignTime, so the Build call would error on main; we're just making the error more explicit. It's not clear to me that this is actually true.



let result =
// Needs to be done before anything else
Copy link
Author

Choose a reason for hiding this comment

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

Claude moved this to earlier in the flow (i.e. the start of crackProjects).

@nojaf
Copy link
Collaborator

nojaf commented Feb 28, 2026

@Smaug123 did you try this out somewhere?
Happy to take this one over #1056 , @TheAngryByrd could you take a look at this PR as well please.

If we have this, it would unblock you if we have a new version right?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the fsdocs-tool project-cracking implementation to work with Ionide.ProjInfo v0.74.2 (needed to address the VisualTree.relativePathOf issue referenced in #1054).

Changes:

  • Bumps Ionide.ProjInfo from 0.62.0 to 0.74.2 (and removes the separate Ionide.ProjInfo.Sln package).
  • Refactors ProjectCracker to use the newer ProjectLoader.loadProject + getLoadedProjectInfo API shape.
  • Adds a Microsoft.Build compile-time reference in fsdocs-tool to support the updated loading flow.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/fsdocs-tool/fsdocs-tool.fsproj Removes Ionide.ProjInfo.Sln reference and adds Microsoft.Build reference needed by new cracking approach.
src/fsdocs-tool/ProjectCracker.fs Switches project evaluation to new ProjInfo APIs; moves MSBuild init earlier in cracking.
Directory.Packages.props Updates central package version for Ionide.ProjInfo to 0.74.2 and removes Ionide.ProjInfo.Sln.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +290 to +294
let loggedMessages = System.Collections.Concurrent.ConcurrentQueue<string>()


let result =
// Needs to be done before anything else
let cwd = System.Environment.CurrentDirectory |> System.IO.DirectoryInfo
let dotnetExe = getDotnetHostPath () |> Option.map System.IO.FileInfo
let _toolsPath = Init.init cwd dotnetExe
ProjectLoader.getProjectInfo projectFile extraMsbuildProperties BinaryLogGeneration.Off customProperties
//file |> Inspect.getProjectInfos loggedMessages.Enqueue msbuildExec [gp] []
let projectCollection = new Microsoft.Build.Evaluation.ProjectCollection(dict extraMsbuildProperties)
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

loggedMessages is no longer populated after switching to ProjectLoader.loadProject/getLoadedProjectInfo, so the msgs list used in error reporting will always be empty. Either remove this queue/log plumbing, or wire it up to the logging/event sink exposed by the new ProjInfo/MSBuild evaluation path so failures still include actionable diagnostics.

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +16
<PackageVersion Include="Microsoft.Build" Version="" PrivateAssets="all" />
<PackageVersion Include="Microsoft.Build.Framework" Version="" PrivateAssets="all" />
<PackageVersion Include="Microsoft.Build.Utilities.Core" Version="" PrivateAssets="all" />
<PackageVersion Include="Microsoft.Build.Tasks.Core" Version="" PrivateAssets="all" />
<PackageVersion Include="Ionide.ProjInfo" Version="0.62.0" />
<PackageVersion Include="Ionide.ProjInfo.Sln" Version="0.62.0" />
<PackageVersion Include="Ionide.ProjInfo" Version="0.74.2" />
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

Microsoft.Build is now referenced by fsdocs-tool, but its central package version is currently set to an empty string. With ManagePackageVersionsCentrally=true, restoring a PackageReference Include="Microsoft.Build" will typically fail because NuGet requires a valid version here. Please set an explicit Version for Microsoft.Build (and any related MSBuild packages if needed), or move the version to a VersionOverride on the consuming project if you intentionally want a specific SDK-aligned version.

See below for a potential fix:

    <PackageVersion Include="Microsoft.Build" Version="17.11.3" PrivateAssets="all" />
    <PackageVersion Include="Microsoft.Build.Framework" Version="17.11.3" PrivateAssets="all" />
    <PackageVersion Include="Microsoft.Build.Utilities.Core" Version="17.11.3" PrivateAssets="all" />
    <PackageVersion Include="Microsoft.Build.Tasks.Core" Version="17.11.3" PrivateAssets="all" />

Copilot uses AI. Check for mistakes.
let _toolsPath = Init.init cwd dotnetExe
ProjectLoader.getProjectInfo projectFile extraMsbuildProperties BinaryLogGeneration.Off customProperties
//file |> Inspect.getProjectInfos loggedMessages.Enqueue msbuildExec [gp] []
let projectCollection = new Microsoft.Build.Evaluation.ProjectCollection(dict extraMsbuildProperties)
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

ProjectCollection implements IDisposable, but it’s currently created without being disposed/unloaded. When cracking multiple projects this can retain evaluated projects, hold file handles, and increase memory usage. Use a use binding (or try/finally) to dispose the ProjectCollection after loadProject/getLoadedProjectInfo completes.

Suggested change
let projectCollection = new Microsoft.Build.Evaluation.ProjectCollection(dict extraMsbuildProperties)
use projectCollection = new Microsoft.Build.Evaluation.ProjectCollection(dict extraMsbuildProperties)

Copilot uses AI. Check for mistakes.
github-actions bot added a commit that referenced this pull request Feb 28, 2026
…paceLoader

Adopt the simpler lower-level API from Ionide.ProjInfo (as in PR #1055):
- Replace WorkspaceLoader.Create + Notifications + LoadProjects with
  ProjectLoader.loadProject + ProjectLoader.getLoadedProjectInfo
- Add explicit Microsoft.Build reference (compile-time only, ExcludeAssets=runtime)
  since ProjectCollection is now directly instantiated in user code
- Fewer lines of code, no async notification subscription needed

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Smaug123
Copy link
Author

I was out this morning, haven't had a chance to try it out plugged into the FSharp.Analyzers.SDK build; might not get a chance to do that for a few hours. I didn't actually include the bugfix I specifically want in this PR; that would be an additional two-line change to convert "find .sln files" into "find .sln and .slnx files", but I thought it would be more hygienic to isolate the two changes.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update ProjInfo to v0.74.2 to fix VisualTree.relativePathOf issue

3 participants