Use 64-bit Cocoa instead of 32-bit Carbon for macOS builds #1224
Conversation
This file contains 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
leonsoft-kras
requested changes
Jun 24, 2019
leonsoft-kras
previously approved these changes
Jun 25, 2019
|
on a fresh install of 10.15 public beta I get |
This appears to be caused by a known issue in Lazarus that will be fixed in the next version. See https://bugs.freepascal.org/view.php?id=35702 I think our best bet here will be to just wait for the next version before supporting 10.15. The application could be built using an SVN pull of Lazarus to work around this, but that would add a fair amount of complexity to the build process and I don't know what sort of issues it could potentially cause. I think it's also worth noting that 10.15 is still in the prerelease stage, so supporting it at this stage probably isn't critical. |
|
@someperson thanks again, I agree with you about just wait for the next version, and I also need more time to test it! cc #1150 |
|
no mind, just triggered the build again, those for raspberry pi fails frequently on Travis CI |
someperson
force-pushed
the
cocoa
branch
5 times, most recently
from
5a76765 to
38f3e51
Compare
|
@bertybassett Could you please retest using my latest build? It should run in macOS 10.15 Public Beta 2 (if you're still running Public Beta 1 or an older developer release, you might need to update). |
|
Look like the issue was resolved from upstream, I don't have macOS v10.15 beta yet, so can't test it right now, but please feel free to let me know when it's ready to be merged! Thanks. |
|
@PeterDaveHello My concern with merging this in as things stand is that I haven't tested (and am not even sure I know how to test in some cases) all features in the application. I only discovered the other day that the "New connection" dialog was broken, for example, and it wasn't hard to fix, but I don't know if there are more instances like this. This is why I have been calling my builds alpha versions. I have confirmed that the application runs in my 10.15 VM, but 10.15 isn't required to test it. I don't know how far back it supports, but it definitely runs in 10.14, and Travis CI compiles it in a 10.13 environment, so I'd expect it to work there as well. |
|
@someperson yes, we should test it more 😙 btw, may I know your development environment? Is it also macOS v10.14? Thanks |
|
I'm asking this because I can't install fpc v3.0.4 on macOS v10.14, but v3.0.4a can 🤔 maybe need to upgrade fpc here. but even with fpc 3.0.4a, I still failed at final(?) linking: didn't realize v10.14 will need more effort 😱 maybe I'll need to downgrade to v10.13 as workaround, but eventually I prefer to upgrade both development environment and CI to macOS 10.14, after I can build it on v10.14 |
|
@PeterDaveHello My development environment is 10.14, though admittedly, I installed FPC and Lazarus on it manually, rather than using the script. I tried bumping FPC to 3.0.4a the other day, but for reasons I didn't understand, this resulted in problems compiling it, whereas 3.0.4 worked. If you want, I can look more into this. 3.0.4 is the version I have installed on my machine though, so I'm not sure why you're having problems. |
|
@PeterDaveHello Something also worth noting is that I've noticed that the builds I've produced on my machine in 10.14 are Dark Mode-aware, but Lazarus' support for Dark Mode seems to be incomplete, resulting in some UI elements being unreadable if the user has Dark Mode enabled. The builds from Travis CI don't have this issue since the application produced runs solely in Light Mode regardless of the user's settings. Do we need to find a workaround for this now? |
|
@someperson feel free to help improve any part of this project, including dependency upgrade like macOS, fpc, lazarus, etc, and also features like the dark mode. I'm still trying to understand why I'm facing the issues on macOS 10.14, already tried on both model of 2014 and 2018, thanks for your help anyway! |
|
I tried to compile this branch but it still outputs a 32bit executable on Catalina... am I missing something? |
|
I am on the latest Catalina Beta - any chance of a DMG file so I can test it also? |
His builds are at: https://github.com/someperson/transgui/releases Not sure if he'd prefer issues listed here, or somewhere else. I noticed his repo has issues turned off, so can't put them there. |
|
I just turned on Issues in my repo, so feel free to file anything specific to the 64-bit builds there. Thanks for the heads-up; I didn't even know Issues could be turned off. |
Thanks, briefly installed it and it works fine - once I got past the security 'integrity' warning apple display. I will try and test it more fully in the next few days but it looks great. |
|
@PeterDaveHello I just merged in a number of improvements, including fixes for building on macOS 10.14. I also bumped the Travis CI environment to Xcode 10.3 on macOS 10.14. Would you mind retesting with these changes? |
|
@someperson Sure, I will take a look soon, thanks! |
transgui.lpi
Outdated
| @@ -211,7 +211,7 @@ | |||
| <Verbosity> | |||
| <ShowHints Value="False"/> | |||
| </Verbosity> | |||
| <CustomOptions Value="-Sa"/> | |||
| <CustomOptions Value="-Sa -CX -XX"/> | |||
There was a problem hiding this comment.
I'm not sure but wondering if this one should be separated into another commit as it affects not only macOS build?
There was a problem hiding this comment.
Unfortunately, this change is kind of necessary unless certain references to the macOS framework headers are removed from transgui. Otherwise, there is a dependency on Carbon that cannot be resolved in the x86_64 macOS world, causing problems during compilation. See https://wiki.freepascal.org/univint
These flags enable smart linking, which causes anything that isn't used to be dropped at compile-time. Importantly, that includes the Carbon dependency when compiling for Cocoa.
To be honest with you, I don't know how to figure out exactly what would need to be removed to get rid of the Carbon dependency, so this was the easier solution. I know it's not ideal since it affects the other builds, although it shouldn't break anything in theory. Feel free to let me know if you have a better idea.
There was a problem hiding this comment.
Do you think it's possible to separate it into another PR which should be merged before this one? I think we can do some tests to make sure it won't bring some negative affects to the current builds. Thanks.
There was a problem hiding this comment.
I can definitely put it on a branch by itself and create a PR for it. I'll go ahead and do that. Thanks!
There was a problem hiding this comment.
I've merged #1252 , so let's remove this diff here?
Reverted VERSION.txt and changed compilers.sh to build correct version of Lazarus Use x86_64 compiler when building Lazarus Rebased cocoa branch onto latest upstream master Bumped FPC to 3.0.4a and Lazarus to 2.0.2a Reverted FPC to 3.0.4 Fixed "New connection" dialog on macOS Fixed "Show advanced options" checkbox on macOS Bumped Lazarus to 2.0.4 and FPC to 3.0.4a Bumped build environment to xcode10.3 Added flags to lazbuild command in order to build the application fully using x86_64 cocoa Enabled smart linking in order to allow compilation fully without Carbon to succeed Set resulting DMG filesystem to HFS+
PeterDaveHello
approved these changes
Sep 13, 2019
|
Just downloaded transgui-5.17.0-cocoa-alpha6 on latest Catalina beta (19A558d) and so far all is fine. |
|
Just FYI on Catalina 19A582a I've found that the file listings display peculiarly when dark mode is enabled. see screenshot for details |
|
Thank you so much @someperson for porting to 64bits and Cocoa, and thanks @PeterDaveHello for the quick merge. I had the build from Sam installed for two weeks, had no issue with it even after the upgrade to Catalina, and I just upgraded to the new 5.18.0 official version, no problem either so far. |
|
Is this still active? |
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.
I was able to compile a functional version of Transmission Remote GUI for macOS that uses Cocoa instead of Carbon and is 64-bit instead of 32-bit, which I have released on my fork. This is rather experimental and not well-tested, but I thought it might be a good idea to create a pull request now in order to raise awareness. Here be dragons!