Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: use slim binary over dylib#210

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ethanndickson merged 1 commit intomainfromethan/slim-over-dylib
Aug 6, 2025

Conversation

ethanndickson
Copy link
Member

@ethanndicksonethanndickson commentedJul 30, 2025•
edited
Loading

Closes#201.

This PR:

  • Replaces the dylib download with a slim binary download.
  • Removes signature validation checks that are inappropriate for the slim binary.
  • ReplacesTunnelHandle withTunnelDaemon, an abstraction which runsposix_spawn on the slim binary, and manages the spawned process.
  • Adds tests forTunnelDaemon.

In a future PR:

  • Bump the minimum server version for Coder Desktop for macOS (to v2.25 once it's released)

@ethanndicksonGraphite App
Copy link
MemberAuthor

ethanndickson commentedJul 30, 2025•
edited
Loading

onCancel:{
self.logger.debug("async stream canceled")
self.dispatch.close()
self.dispatch.close(flags:[.stop])
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

This causes any currently runningdispatch.read calls to supply their callback with an error which lets us actually end the read loop and cause the manager and all of it's children to be deallocated, including pipe file descriptors.
Previously we would leak pipes, but it didn't matter because the process would die each time.

Comment on lines -104 to -127
privatestaticfunc validateInfo(infoPlist:[String:AnyObject], expectedVersion:String)throws(ValidationError){
guardlet plistIdent=infoPlist[infoIdentifierKey]as?String, plistIdent== expectedIdentifierelse{
throw.invalidIdentifier(identifier:infoPlist[infoIdentifierKey]as?String)
}

guardlet plistName=infoPlist[infoNameKey]as?String, plistName== expectedNameelse{
throw.invalidIdentifier(identifier:infoPlist[infoNameKey]as?String)
}

// Downloaded dylib must match the version of the server
guardlet dylibVersion=infoPlist[infoShortVersionKey]as?String,
expectedVersion== dylibVersion
else{
throw.invalidVersion(version:infoPlist[infoShortVersionKey]as?String)
}

// Downloaded dylib must be at least the minimum Coder server version
guardlet dylibVersion=infoPlist[infoShortVersionKey]as?String,
// x.compare(y) is .orderedDescending if x > y
minimumCoderVersion.compare(dylibVersion, options:.numeric)!=.orderedDescending
else{
throw.belowMinimumCoderVersion
}
}
Copy link
MemberAuthor

@ethanndicksonethanndicksonJul 31, 2025•
edited
Loading

Choose a reason for hiding this comment

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

Unfortunate, but because we don't build the slim binary with an external linker (previously xcode via cgo for the dylib) we can no longer create an info.plist section in the binary with-sectcreate. If we ever need to build the slim binary with cgo then we can add this back.

Copy link
MemberAuthor

@ethanndicksonethanndicksonJul 31, 2025•
edited
Loading

Choose a reason for hiding this comment

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

I've added another PR to check the version by exec'ing.

@ethanndicksonethanndickson marked this pull request as ready for reviewJuly 31, 2025 04:44
@ethanndicksonethanndickson self-assigned thisJul 31, 2025
@ethanndicksonethanndicksonforce-pushed theethan/xpc-validation branch 2 times, most recently from8670f11 tobe347a8CompareAugust 4, 2025 03:03
Copy link

@CopilotCopilotAI 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

This PR replaces the dylib-based tunnel implementation with a slim binary approach for the VPN functionality. It removes dylib-specific code and signature validation, introducingTunnelDaemon as a new abstraction that spawns and manages a binary process.

  • ReplacesTunnelHandle withTunnelDaemon that usesposix_spawn to run a slim binary
  • Updates download and validation logic to work with binaries instead of dylibs
  • Removes dylib-specific signature validation and quarantine removal processes

Reviewed Changes

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

Show a summary per file
FileDescription
scripts/update-cask.shUpdates cleanup paths to include new binary locations
Coder-Desktop/project.ymlRemoves bridging header reference for dylib integration
Coder-Desktop/VPNLibTests/TunnelDaemonTests.swiftAdds comprehensive tests for the new TunnelDaemon functionality
Coder-Desktop/VPNLib/XPC.swiftUpdates progress descriptions from "library" to "binary"
Coder-Desktop/VPNLib/Validate.swiftSimplifies validation by removing dylib-specific checks
Coder-Desktop/VPNLib/TunnelDaemon.swiftNew implementation for managing tunnel binary process
Coder-Desktop/VPNLib/System.swiftAdds system utilities for process spawning and monitoring
Coder-Desktop/Coder-DesktopHelper/TunnelHandle.swiftRemoves old dylib-based tunnel implementation
Coder-Desktop/Coder-DesktopHelper/Manager.swiftMajor refactor to use TunnelDaemon instead of TunnelHandle

@ethanndicksonGraphite App
Copy link
MemberAuthor

ethanndickson commentedAug 6, 2025•
edited
Loading

Merge activity

  • Aug 6, 3:39 AM UTC: A user started a stack merge that includes this pull request viaGraphite.
  • Aug 6, 3:48 AM UTC:Graphite rebased this pull request as part of a merge.
  • Aug 6, 3:50 AM UTC:@ethanndickson merged this pull request withGraphite.

@ethanndicksonethanndickson changed the base branch fromethan/xpc-validation tographite-base/210August 6, 2025 03:45
@ethanndicksonethanndickson changed the base branch fromgraphite-base/210 tomainAugust 6, 2025 03:47
@ethanndicksonethanndickson merged commit8c08563 intomainAug 6, 2025
4 checks passed
@ethanndicksonethanndickson deleted the ethan/slim-over-dylib branchAugust 6, 2025 03:50
ethanndickson added a commit that referenced this pull requestAug 6, 2025
This requirement is introduced by#210, as the `vpn-daemon` command on the macOS CLI isn't present on prior versions.(2.24.3 isn't out yet, but when it does come out, it'll be compatible. 2.25 is out, and it's compatible)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

Copilot code reviewCopilotCopilot left review comments

@deansheatherdeansheatherdeansheather approved these changes

@spikecurtisspikecurtisAwaiting requested review from spikecurtis

Assignees

@ethanndicksonethanndickson

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Coder Connect cannot reach a Coder deployment behind a VPN
2 participants
@ethanndickson@deansheather
[8]ページ先頭
©2009-2025 Movatter.jp