chore: use slim binary over dylib #210
Open
+530
−246
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 30, 2025
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Jul 30, 2025
7ef8131 to
8f00c6a
Compare
547fd97 to
6687411
Compare
8f00c6a to
479576a
Compare
ethanndickson
commented
Jul 31, 2025
| @@ -69,7 +69,7 @@ actor Receiver<RecvMsg: Message> { | |||
| }, | |||
| onCancel: { | |||
| self.logger.debug("async stream canceled") | |||
| self.dispatch.close() | |||
| self.dispatch.close(flags: [.stop]) | |||
There was a problem hiding this comment.
This causes any currently running dispatch.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
| private static func validateInfo(infoPlist: [String: AnyObject], expectedVersion: String) throws(ValidationError) { | ||
| guard let plistIdent = infoPlist[infoIdentifierKey] as? String, plistIdent == expectedIdentifier else { | ||
| throw .invalidIdentifier(identifier: infoPlist[infoIdentifierKey] as? String) | ||
| } | ||
|
|
||
| guard let plistName = infoPlist[infoNameKey] as? String, plistName == expectedName else { | ||
| throw .invalidIdentifier(identifier: infoPlist[infoNameKey] as? String) | ||
| } | ||
|
|
||
| // Downloaded dylib must match the version of the server | ||
| guard let 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 | ||
| guard let dylibVersion = infoPlist[infoShortVersionKey] as? String, | ||
| // x.compare(y) is .orderedDescending if x > y | ||
| minimumCoderVersion.compare(dylibVersion, options: .numeric) != .orderedDescending | ||
| else { | ||
| throw .belowMinimumCoderVersion | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I've added another PR to check the version by exec'ing.
6687411 to
ef370db
Compare
479576a to
e9e15db
Compare
e9e15db to
c7602c2
Compare
ef370db to
55319f4
Compare
c7602c2 to
f008801
Compare
8670f11 to
be347a8
Compare
f008801 to
847006f
Compare
be347a8 to
e6a3578
Compare
847006f to
d9255bc
Compare
e6a3578 to
a1864f6
Compare
d9255bc to
4f29ff3
Compare
a1864f6 to
8b4c8cd
Compare
4f29ff3 to
5840bce
Compare
8b4c8cd to
78fd6c0
Compare
5840bce to
3f7f6b6
Compare
This was referenced Aug 4, 2025
deansheather
approved these changes
Aug 4, 2025
3f7f6b6 to
42b8f0b
Compare
78fd6c0 to
a5d5337
Compare
a5d5337 to
c450bd4
Compare
42b8f0b to
b716554
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #201.
This PR:
TunnelHandlewithTunnelDaemon, an abstraction which runsposix_spawnon the slim binary, and manages the spawned process.TunnelDaemon.In a future PR: