Skip to content

chore: various finish line tasks #23

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

Merged
merged 4 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,15 @@ jobs:
cache-dependency-path: '**/packages.lock.json'
- name: dotnet restore
run: dotnet restore --locked-mode
#- name: dotnet publish
# run: dotnet publish --no-restore --configuration Release --output .\publish
#- name: Upload artifact
# uses: actions/upload-artifact@v4
# with:
# name: publish
# path: .\publish\
# This doesn't call `dotnet publish` on the entire solution, just the
# projects we care about building. Doing a full publish includes test
# libraries and stuff which is pointless.
- name: dotnet publish Coder.Desktop.Vpn.Service
run: dotnet publish .\Vpn.Service\Vpn.Service.csproj --configuration Release --output .\publish\Vpn.Service
- name: dotnet publish Coder.Desktop.App
run: dotnet publish .\App\App.csproj --configuration Release --output .\publish\App
- name: Upload artifact
uses: actions/upload-artifact@v4
with:
name: publish
path: .\publish\
61 changes: 61 additions & 0 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
name: Release

on:
push:
tags:
- '*'

permissions:
contents: write

jobs:
build:
runs-on: windows-latest

steps:
- uses: actions/checkout@v4

- name: Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: '8.0.x'

- name: Get version from tag
id: version
shell: pwsh
run: |
$tag = $env:GITHUB_REF -replace 'refs/tags/',''
if ($tag -notmatch '^v\d+\.\d+\.\d+$') {
throw "Tag must be in format v1.2.3"
}
$version = $tag -replace '^v',''
$assemblyVersion = "$version.0"
echo "VERSION=$version" >> $env:GITHUB_OUTPUT
echo "ASSEMBLY_VERSION=$assemblyVersion" >> $env:GITHUB_OUTPUT

- name: Build and publish x64
run: |
dotnet publish src/App/App.csproj -c Release -r win-x64 -p:Version=${{ steps.version.outputs.ASSEMBLY_VERSION }} -o publish/x64
dotnet publish src/Vpn.Service/Vpn.Service.csproj -c Release -r win-x64 -p:Version=${{ steps.version.outputs.ASSEMBLY_VERSION }} -o publish/x64

- name: Build and publish arm64
run: |
dotnet publish src/App/App.csproj -c Release -r win-arm64 -p:Version=${{ steps.version.outputs.ASSEMBLY_VERSION }} -o publish/arm64
dotnet publish src/Vpn.Service/Vpn.Service.csproj -c Release -r win-arm64 -p:Version=${{ steps.version.outputs.ASSEMBLY_VERSION }} -o publish/arm64

- name: Create ZIP archives
shell: pwsh
run: |
Compress-Archive -Path "publish/x64/*" -DestinationPath "./publish/CoderDesktop-${{ steps.version.outputs.VERSION }}-x64.zip"
Compress-Archive -Path "publish/arm64/*" -DestinationPath "./publish/CoderDesktop-${{ steps.version.outputs.VERSION }}-arm64.zip"

- name: Create Release
uses: softprops/action-gh-release@v1
with:
files: |
./publish/CoderDesktop-${{ steps.version.outputs.VERSION }}-x64.zip
./publish/CoderDesktop-${{ steps.version.outputs.VERSION }}-arm64.zip
name: Release ${{ steps.version.outputs.VERSION }}
generate_release_notes: true
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -403,5 +403,7 @@ FodyWeavers.xsd
.idea/**/shelf

publish
WindowsAppRuntimeInstall-x64.exe
WindowsAppRuntimeInstall-*.exe
windowsdesktop-runtime-*.exe
wintun.dll
wintun-*.dll
32 changes: 26 additions & 6 deletions App/App.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,42 @@
<PublishProfile>Properties\PublishProfiles\win-$(Platform).pubxml</PublishProfile>
<UseWinUI>true</UseWinUI>
<Nullable>enable</Nullable>
<EnableMsixTooling>false</EnableMsixTooling>
<EnableMsixTooling>true</EnableMsixTooling>
<WindowsPackageType>None</WindowsPackageType>
<RestorePackagesWithLockFile>true</RestorePackagesWithLockFile>
<!-- To use CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute: -->
<LangVersion>preview</LangVersion>
<!-- We have our own implementation of main with exception handling -->
<DefineConstants>DISABLE_XAML_GENERATED_MAIN</DefineConstants>
</PropertyGroup>

<PropertyGroup Condition="$(Configuration) == 'Release'">
<PublishTrimmed>true</PublishTrimmed>
<TrimMode>CopyUsed</TrimMode>
<PublishReadyToRun>true</PublishReadyToRun>
<SelfContained>true</SelfContained>
</PropertyGroup>

<ItemGroup>
<Manifest Include="$(ApplicationManifest)" />
</ItemGroup>

<ItemGroup>
<Content Include="Images\SplashScreen.scale-200.png" />
<Content Include="Images\Square150x150Logo.scale-200.png" />
<Content Include="Images\Square44x44Logo.scale-200.png" />
</ItemGroup>
<!--
Clean up unnecessary files (including .xbf XAML Binary Format files)
and (now) empty directories from target. The .xbf files are not
necessary as they are contained within resources.pri.
-->
<Target Name="CleanupTargetDir" AfterTargets="Build;_GenerateProjectPriFileCore" Condition="$(Configuration) == 'Release'">
<ItemGroup>
<FilesToDelete Include="$(TargetDir)**\*.xbf" />
<FilesToDelete Include="$(TargetDir)createdump.exe" />
<DirsToDelete Include="$(TargetDir)Controls" />
<DirsToDelete Include="$(TargetDir)Views" />
</ItemGroup>

<Delete Files="@(FilesToDelete)" />
<RemoveDir Directories="@(DirsToDelete)" />
</Target>

<ItemGroup>
<PackageReference Include="CommunityToolkit.Mvvm" Version="8.4.0" />
Expand Down
21 changes: 16 additions & 5 deletions App/App.xaml.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;
using System.Diagnostics;
using System.Threading.Tasks;
using Coder.Desktop.App.Services;
using Coder.Desktop.App.ViewModels;
using Coder.Desktop.App.Views;
Expand All @@ -13,6 +13,8 @@ public partial class App : Application
{
private readonly IServiceProvider _services;

private bool _handleWindowClosed = true;

public App()
{
var services = new ServiceCollection();
Expand All @@ -36,18 +38,27 @@ public App()

_services = services.BuildServiceProvider();

#if DEBUG
UnhandledException += (_, e) => { Debug.WriteLine(e.Exception.ToString()); };
#endif

InitializeComponent();
}

public async Task ExitApplication()
{
_handleWindowClosed = false;
Exit();
var rpcManager = _services.GetRequiredService<IRpcController>();
// TODO: send a StopRequest if we're connected???
await rpcManager.DisposeAsync();
Environment.Exit(0);
}

protected override void OnLaunched(LaunchActivatedEventArgs args)
{
var trayWindow = _services.GetRequiredService<TrayWindow>();

// Prevent the TrayWindow from closing, just hide it.
trayWindow.Closed += (sender, args) =>
{
if (!_handleWindowClosed) return;
args.Handled = true;
trayWindow.AppWindow.Hide();
};
Expand Down
Binary file removed App/Images/SplashScreen.scale-200.png
Binary file not shown.
Binary file removed App/Images/Square150x150Logo.scale-200.png
Binary file not shown.
Binary file removed App/Images/Square44x44Logo.scale-200.png
Binary file not shown.
83 changes: 83 additions & 0 deletions App/Program.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
using System;
using System.Runtime.InteropServices;
using System.Threading;
using Microsoft.UI.Dispatching;
using Microsoft.UI.Xaml;
using Microsoft.Windows.AppLifecycle;
using WinRT;

namespace Coder.Desktop.App;

#if DISABLE_XAML_GENERATED_MAIN
public static class Program
{
private static App? app;
#if DEBUG
[DllImport("kernel32.dll")]
private static extern bool AllocConsole();
#endif

[DllImport("user32.dll", CharSet = CharSet.Unicode)]
private static extern int MessageBoxW(IntPtr hWnd, string text, string caption, int type);

[STAThread]
private static void Main(string[] args)
{
try
{
ComWrappersSupport.InitializeComWrappers();
if (!CheckSingleInstance()) return;
Application.Start(p =>
{
var context = new DispatcherQueueSynchronizationContext(DispatcherQueue.GetForCurrentThread());
SynchronizationContext.SetSynchronizationContext(context);

app = new App();
app.UnhandledException += (_, e) =>
{
e.Handled = true;
ShowExceptionAndCrash(e.Exception);
};
});
}
catch (Exception e)
{
ShowExceptionAndCrash(e);
}
}

[STAThread]
private static bool CheckSingleInstance()
{
#if !DEBUG
const string appInstanceName = "Coder.Desktop.App";
#else
const string appInstanceName = "Coder.Desktop.App.Debug";
#endif

var instance = AppInstance.FindOrRegisterForKey(appInstanceName);
return instance.IsCurrent;
}

[STAThread]
private static void ShowExceptionAndCrash(Exception e)
{
const string title = "Coder Desktop Fatal Error";
var message =
"Coder Desktop has encountered a fatal error and must exit.\n\n" +
e + "\n\n" +
Environment.StackTrace;
MessageBoxW(IntPtr.Zero, message, title, 0);

if (app != null)
app.Exit();

// This will log the exception to the Windows Event Log.
#if DEBUG
// And, if in DEBUG mode, it will also log to the console window.
AllocConsole();
#endif
Environment.FailFast("Coder Desktop has encountered a fatal error and must exit.", e);
}
}
#endif
4 changes: 1 addition & 3 deletions App/Properties/PublishProfiles/win-arm64.pubxml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<!--
https://go.microsoft.com/fwlink/?LinkID=208121.
-->
Expand All @@ -8,7 +8,5 @@ https://go.microsoft.com/fwlink/?LinkID=208121.
<Platform>ARM64</Platform>
<RuntimeIdentifier>win-arm64</RuntimeIdentifier>
<PublishDir>bin\$(Configuration)\$(TargetFramework)\$(RuntimeIdentifier)\publish\</PublishDir>
<SelfContained>true</SelfContained>
<PublishSingleFile>False</PublishSingleFile>
</PropertyGroup>
</Project>
4 changes: 1 addition & 3 deletions App/Properties/PublishProfiles/win-x64.pubxml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<!--
https://go.microsoft.com/fwlink/?LinkID=208121.
-->
Expand All @@ -8,7 +8,5 @@ https://go.microsoft.com/fwlink/?LinkID=208121.
<Platform>x64</Platform>
<RuntimeIdentifier>win-x64</RuntimeIdentifier>
<PublishDir>bin\$(Configuration)\$(TargetFramework)\$(RuntimeIdentifier)\publish\</PublishDir>
<SelfContained>true</SelfContained>
<PublishSingleFile>False</PublishSingleFile>
</PropertyGroup>
</Project>
4 changes: 1 addition & 3 deletions App/Properties/PublishProfiles/win-x86.pubxml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<!--
https://go.microsoft.com/fwlink/?LinkID=208121.
-->
Expand All @@ -8,7 +8,5 @@ https://go.microsoft.com/fwlink/?LinkID=208121.
<Platform>x86</Platform>
<RuntimeIdentifier>win-x86</RuntimeIdentifier>
<PublishDir>bin\$(Configuration)\$(TargetFramework)\$(RuntimeIdentifier)\publish\</PublishDir>
<SelfContained>true</SelfContained>
<PublishSingleFile>False</PublishSingleFile>
</PropertyGroup>
</Project>
22 changes: 14 additions & 8 deletions App/Services/CredentialManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Runtime.InteropServices;
using System.Text;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading;
using System.Threading.Tasks;
using Coder.Desktop.App.Models;
Expand All @@ -10,6 +11,17 @@

namespace Coder.Desktop.App.Services;

public class RawCredentials
{
public required string CoderUrl { get; set; }
public required string ApiToken { get; set; }
}

[JsonSerializable(typeof(RawCredentials))]
public partial class RawCredentialsJsonContext : JsonSerializerContext
{
}

public interface ICredentialManager
{
public event EventHandler<CredentialModel> CredentialsChanged;
Expand Down Expand Up @@ -123,7 +135,7 @@ private void UpdateState(CredentialModel newModel)
RawCredentials? credentials;
try
{
credentials = JsonSerializer.Deserialize<RawCredentials>(raw);
credentials = JsonSerializer.Deserialize(raw, RawCredentialsJsonContext.Default.RawCredentials);
}
catch (JsonException)
{
Expand All @@ -138,16 +150,10 @@ private void UpdateState(CredentialModel newModel)

private static void WriteCredentials(RawCredentials credentials)
{
var raw = JsonSerializer.Serialize(credentials);
var raw = JsonSerializer.Serialize(credentials, RawCredentialsJsonContext.Default.RawCredentials);
NativeApi.WriteCredentials(CredentialsTargetName, raw);
}

private class RawCredentials
{
public required string CoderUrl { get; set; }
public required string ApiToken { get; set; }
}

private static class NativeApi
{
private const int CredentialTypeGeneric = 1;
Expand Down
9 changes: 8 additions & 1 deletion App/Services/RpcController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public VpnLifecycleException(string message) : base(message)
}
}

public interface IRpcController
public interface IRpcController : IAsyncDisposable
{
public event EventHandler<RpcModel> StateChanged;

Expand Down Expand Up @@ -224,6 +224,13 @@ public async Task StopVpn(CancellationToken ct = default)
new InvalidOperationException($"Service reported failure: {reply.Stop.ErrorMessage}"));
}

public async ValueTask DisposeAsync()
{
if (_speaker != null)
await _speaker.DisposeAsync();
GC.SuppressFinalize(this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand why this is necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

You don't understand why the entire DisposeAsync implementation is necessary or why the GC.SuppressFinalize(this); is?

If you mean the whole implementation, we're calling it when the app closes to close the named pipe and remove the async receive tasks from the background.

If you mean the GC.SuppressFinalize(this) thing, it prevents the ~RpcController() finalizer method from being called. Since we're not using it and we don't need to clean up any unmanaged resources here, we just tell the GC to not bother calling it during a GC run later on.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I meant SuppressFinalize.

I don't see an explicit ~RpcController() method--I assume this is implicit? Is the need to suppress it required for correctness (i.e. it is not idempotent), or is suppressing it just a performance optimization?

I ask because I'm concerned that this is somewhat fragile. _speaker is the only resource that needs to be cleaned up today, but if we add a new resource and forget to include it in this method, presumably we'd leak it by supressing the implicit finalizer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly I don't know that much about how the C# GC works, so I'm not sure. From what I can tell in most cases where you don't explicitly write a finalizer it won't try to run it, so it's probably fine without it.

The main reason this is here is because the linter said we should include it even though we don't have a finalizer:

To prevent derived types with finalizers from having to reimplement IDisposable and to call it, unsealed types without finalizers should still call GC.SuppressFinalize.

So it seems like it's more about convention

}

private void MutateState(Action<RpcModel> mutator)
{
RpcModel newState;
Expand Down
Loading
Loading