Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

IDEA: build FSAC standalone binaries for each OS/Arch/TFM #1258

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

baronfel
Copy link
Contributor

@baronfel baronfel commented Mar 31, 2024

Zed and other modern editors that use LSP servers for language support often have helpers that make it easy to acquire and launch LSP servers. For users that are happy with dotnet tools, the current publishing mechanism works great. But for users of these other tools, having a platform-specific binary available as part of our releases would make the task of acquiring FSAC easier and more natural.

This PR does the very first step: building FSAC as a standalone binary for each TFM and RID that we might want to support in this way. When invoking the BuildAllStandloneApplications target, MSBuild will invoke a platform-specific, self-contained build of FSAC for a given TFM and platform.

On my machine this takes ~4m, but it is very incremental - rebuilds with no changes take ~5s. I think we should consider enabling standlone binaries - this might also make it easier to test specific configurations of LSP, perhaps even sidestepping the odd global.json manipulation we have to do today to ensure isolated environments.

@@ -3,6 +3,7 @@
<PropertyGroup>
<Title>FsAutoComplete</Title>
<Product>FsAutoComplete</Product>
<ArtifactsPath>$(MSBuildThisFileDirectory)artifacts</ArtifactsPath>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think if we go down this path, we should move to requiring .NET 8 and use the artifacts layout to simplify out outputs.

Comment on lines 6 to 7
<TargetFrameworks Condition="'$(BuildNet7)' == 'true'">net6.0;net7.0</TargetFrameworks>
<TargetFrameworks Condition="'$(BuildNet8)' == 'true'">net6.0;net7.0;net8.0</TargetFrameworks>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

WIP: we could potentially remove all of the BuildNetX stuff if we built/tested standalone binaries.

@baronfel baronfel force-pushed the build-arch-specific-binaries branch from 54449c0 to 607bfcc Compare March 31, 2024 21:50
@baronfel
Copy link
Contributor Author

Note that currently these build, but currently make huge binaries if you don't trim. Trimming/single file usage is a bit rough across the FSAC/Ionide libraries at the moment - for example ionide/proj-info#204.

@baronfel baronfel force-pushed the build-arch-specific-binaries branch from 607bfcc to 798b5f4 Compare March 31, 2024 22:58
@baronfel
Copy link
Contributor Author

This would also open the door to making Ionide itself auto-acquire FSAC on load instead of bundling the tool inside the VSCode extension. This would reduce extension size a bit (though increase the LSP size) at the benefit of making more editors use the same acquisition method.

@greggyb
Copy link
Contributor

greggyb commented May 23, 2024

Sounding off here for a nonstandard environment. I develop F# on FreeBSD using Ionide-vim. There is a community port and package of dotnet to FreeBSD. dotnet tool works for me here.

Mechanism is relatively unimportant to me, but I want to make sure that there remains a path for using FSAC for myself and others that may not be on a tier 1 platform/OS from dotnet's perspective.

@baronfel
Copy link
Contributor Author

Totally agree - I think we'd want to keep the tool version in any case if only because of how convenient it is to acquire! This PR would all only be additive.

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.

2 participants