Closed palmerc closed 6 years ago
I am not seeing the point of this. I see you prefer CMake and that is fine. Of course you are free to maintain your own fork of Clutch for those who also like CMake. I think this makes the barrier to entry too high for non-programmers. I like CMake but I do not see why we need it since we only support building on macOS.
This also relies on your forks of ldid, libplist, OpenSSL (for a Linux build I presume), and your scripts. I prefer having the user run each step manually rather than run scripts. Once everything is cloned this may be easier but then you are the only maintainer for this build system (which is fine on your fork). Also I found that I see in your script you do not give the option to build a Release version with CMake. You should accept that argument and it is better to default to Release for non-programming users.
I trust Saurik to keep his copy of the project maintained for a long time to come. If we were to use ldid to sign, I would rather have ldid either come from his repository and building it or simply having the user provide ldid (they could use your fork). We do not support building Clutch on anything except macOS at this time so we have no need for an OpenSSL-based ldid. Saurik's code only calls upon OpenSSL functions if the system if __APPLE__
(defined on macOS) is not defined.
I can see the utility in signing with ldid instead of codesign as Xcode will not let us put the correct entitlements anyway (we could use that in the build process instead of codesign
). However, that would require installing ldid in PATH somewhere. However, any Mac with Xcode installed is going to have codesign
in PATH so we really do not need ldid
.
The build instructions we have are intended to be for a plain macOS+Xcode+Command Line Tools install (whether that is via the Apps Store or from Apple's developer site), and this would require the user to install CMake and have more unknowns on the user side. Our current build process makes the user run 5 steps by hand, especially since they are disabling the code-signing requirement in Xcode. The authority on software (besides us for our own code) is Apple in this case. This lowers the barriers to entry since the user has high trust in getting anything from Apple especially on a Mac (this includes Bash, Git, and SSH distributed by Apple). We only have to provide a few steps to get them going.
@ttwj @NinjaLikesCheez thoughts?
First off, thanks for your contribution :) I'll try carve out some time this weekend to get a device and run this
I've no problem with CMake per say - we used to use it, however we ended up with far more issues about 'how to build' as the xcodeproject and cmake diverged. As long as they are both maintained (+docs!) to the point where they build the same way I don't see a problem - but we have to have xcode support for the sake of newer users.
We should use standard implementations as far as we can - the simpler the build process the better, if we want to add a cmake flag for ldid, fine - but pulling and building it as par of the standard build process might be excessive.
My first choice is not always CMake and in many ways I don’t like it for iOS related things unless they are libraries. However, in this particular project, being command-line and never appearing in the App Store it has a certain appeal.
As for ldid it is totally optional. In theory one could even have CMake generate an Xcode (cmake -GXcode) project file and thereby still handle code signing and development with it.
On 15 Jan 2018, at 11:10, NinjaLikesCheez notifications@github.com wrote:
First off, thanks for your contribution :) I'll try carve out some time this weekend to get a device and run this
I've no problem with CMake per say - we used to use it, however we ended up with far more issues about 'how to build' as the xcodeproject and cmake diverged. As long as they are both maintained (+docs!) to the point where they build the same way I don't see a problem - but we have to have xcode support for the sake of newer users.
We should use standard implementations as far as we can - the simpler the build process the better, if we want to add a cmake flag for ldid, fine - but pulling and building it as par of the standard build process might be excessive.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/KJCracks/Clutch/pull/201#issuecomment-357637812, or mute the thread https://github.com/notifications/unsubscribe-auth/AAgifnvbWy69F1ccqnYl6-Xb6SNG65YZks5tKyQEgaJpZM4RdZLm.
I'm concerned about new guys contributing who aren't as familiar and within a PR may need some guidance. For this it's easier if everyone is using Xcode.
Like I said before, CMake would be more useful if it is for Clutch to have one build system on all platforms. Right now we don't have a build process for Linux which is almost certainly possible to do but we do not support that yet. But then I still don't want shell scripts. Anything done in shell scripts should be in the CMake files themselves. Being more limited, a CMake script is almost always more portable than a shell script (or Makefile).
Also on standards, please no extra spaces for control structures like if. It should be IF (CONDITION)
even in CMake code.
I also propose putting all CMake items in a directory cmake
in the project root if we do merge this. For the time being my vote is no.
See branch palmerc-master
for some changes I've commited. (can't push to your fork)
Please include the compiler flags we are using in your CMakeLists.txt
file(s). We have a lot enabled that are not normally enabled. This is a good start.
https://cmake.org/cmake/help/v3.0/command/target_compile_options.html
And generally using the target_* versions is better form. I’ll have to look at your makefiles to see what options you’re passing
On 1 Apr 2018, at 12:06, Andrew Udvare notifications@github.com wrote:
Please include the compiler flags we are using in your CMakeLists.txt file(s). We have a lot enabled that are not normally enabled. This is a good start.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
Will take a look however that project only has one target for the time being. It's a very simple project.
The flags we use are specified in the Xcode project but they are in the format Xcode wants. You have to consult the man pages for clang
, clang++
, gcc
. Many of the Clang flags are not compatible with GCC, so they should not be passed to that if you detect GCC. I would prefer not to pass the argument to ignore unknown arguments in either case. We technically would be supporting Apple's last build of GCC 4.2.1 and the latest Clang versions (5 and 6). Normally in autotools you test each flag by doing a test compile. I am pretty sure CMake can do this too for ones that we are not sure on.
These are the Clang flags:
-fmodules-prune-after=345600
-fmodules-prune-interval=86400
-DOBJC_OLD_DISPATCH_PROTOTYPES=0
-Warc-repeated-use-of-weak
-Wassign-enum
-Wconditional-uninitialized
-Wdeprecated-declarations
-Wdeprecated-implementations
-Wdocumentation
-Wduplicate-method-match
-Wempty-body
-Wenum-conversion
-Werror
-Werror=block-capture-autoreleasing
-Werror=bool-conversion
-Werror=comma
-Werror=constant-conversion
-Werror=conversion
-Werror=deprecated-objc-isa-usage
-Werror=float-conversion
-Werror=implicit-function-declaration
-Werror=incompatible-pointer-types
-Werror=non-modular-include-in-framework-module
-Werror=objc-literal-conversion
-Werror=objc-root-class
-Werror=return-type
-Werror=shorten-64-to-32
-Werror=sign-conversion
-Werror=strict-prototypes
-Wexplicit-ownership-type
-Wfour-char-constants
-Wimplicit-atomic-properties
-Wimplicit-retain-self
-Winfinite-recursion
-Wint-conversion
-Wmissing-braces
-Wmissing-prototypes
-Wnewline-eof
-Wno-arc-maybe-repeated-use-of-weak
-Wno-gnu-statement-expression
-Wno-missing-field-initializers
-Wno-selector
-Wno-trigraphs
-Wnon-literal-null-conversion
-Wnon-modular-include-in-framework-module
-Wparentheses
-Wpointer-sign
-Wprotocol
-Wshadow
-Wsign-compare
-Wstrict-selector-match
-Wswitch
-Wundeclared-selector
-Wuninitialized
-Wunknown-pragmas
-Wunreachable-code-aggressive
-Wunused-function
-Wunused-label
-Wunused-parameter
-Wunused-value
-Wunused-variable
-fcolor-diagnostics
-fdiagnostics-show-note-include-stack
-fembed-bitcode-marker
-fmacro-backtrace-limit=0
-fmessage-length=110
-fmodules
-fobjc-arc
-fpascal-strings
-fstrict-aliasing
-fvisibility=hidden
-miphoneos-version-min=8.0
-pedantic
-std=gnu11
Most of these will work with GCC but not all.
@palmerc Going to move ahead on this? I am very open to having this in since with CMake the Xcode project data won't have to be versioned and we can remove those extra lines in .gitignore
. CMake can do everything in the generated Makefile (including install dependencies if we choose to use some from CocoaPods or similar).
I'm looking through the comments to see where this stands.
Since I was in a hurry I created a new PR. #220
Since the output is a command-line tool I've generated a CMake build for the project.