Upgrade Tor bundle to 0.4.8.19 #874
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <key>CFBundleShortVersionString</key> | ||
| <string>0.4.8.19</string> | ||
| <key>CFBundleSupportedPlatforms</key> | ||
| <array> | ||
| <string>iPhoneOS</string> | ||
| </array> | ||
| <key>CFBundleVersion</key> | ||
| <string>0.4.8.19</string> |
There was a problem hiding this comment.
The iOS slice Info.plist now sets CFBundleShortVersionString and CFBundleVersion to 0.4.8.19. Apple requires these values to contain at most three period‑separated integers, otherwise codesign/App Store validation fails when embedding the framework. Consider using a three-component semantic version (for example 0.4.8) and a numeric build number instead. The same four-component string was added to the simulator and macOS slice Info.plists as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
seems to pass testflight upload just fine
There was a problem hiding this comment.
There was a similar issue at WhatsApp and the team decided to drop the first digit (if I remember correctly), but it was about the actual app version, not an embedded framework.
Given the sheer amount of changes + the fact that each time we update this binary it bloats the repo size, I'm going to remind about hosting the zipped version of the library outside of the repo and just pointing to it (which would also simplify pointing to different versions / reverting if needed)
There was a problem hiding this comment.
Does Tor team provide a ready-to-use xcframework for different releases/versions?
There was a problem hiding this comment.
unfortunately no. it's also very fragile on app background/foreground. i can't figure out the most robust way to handle yet.
There was a problem hiding this comment.
does the bg/fg issue depend on the version of Tor? do you think we could create a dedicated repo for the different versions of Tor xcframeworks to keep the binary vs our encapsulation logic separate?
| <dict> | ||
| <key>BinaryPath</key> | ||
| <string>tor-nolzma.framework/tor-nolzma</string> | ||
| <string>tor-nolzma.framework/Versions/A/tor-nolzma</string> |
There was a problem hiding this comment.
What's the difference between tor-nolzma.framework/tor-nolzma and tor-nolzma.framework/Versions/A/tor-nolzma? shouldn't they all have the same folder structure?
There was a problem hiding this comment.
good question.
| self.didStart = false | ||
| self.controlMonitorStarted = false |
There was a problem hiding this comment.
maybe instead of all of these bools we could use an enum to manage the state
Summary
Testing