Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Carthage support #262

Closed
wants to merge 2 commits into from
Closed

Conversation

darrenclark
Copy link
Contributor

Carthage support

Depends on:

Basic gist of it was, Xcode & xcodebuild seemed to have slightly different behavior when it comes to building the amalgamation target in those submodules:

  • Xcode would always (even when building for iOS) run the run script build phase as if it was being compiled for macOS
  • xcodebuild would run the run script build phase as if it was being built for whatever SDK was specified on the command line

Since the configure script & Makefile rely on building & running an executable as part of the build process, builds via xcodebuild would fail (issues related to trying to run an iOS executable on a Mac).


With the changes in the above PRs, make test_CarthageBuild seems to be working locally for me, so wanted to give it a run on Travis.

Due to the nature of this PR (updating submodules to my own forks) I don't think it can be merged as is, I just wanted to verify that everything was passing on Travis

@groue
Copy link
Owner

groue commented Oct 10, 2017

Hello @darrenclark

Thank you very much! I'm very looking forward to merge this PR.

Due to the nature of this PR (updating submodules to my own forks) I don't think it can be merged as is, I just wanted to verify that everything was passing on Travis

Tests pass: https://travis-ci.org/groue/GRDB.swift/builds/285813421

What is the next step? Shouldn't we wait for @swiftlyfalling? He did all the hard work bringing custom SQLite builds to GRDB, and his opinion is very valuable to me.

@darrenclark
Copy link
Contributor Author

You're welcome! Sounds good re: getting @swiftlyfalling's opinion on this.

@swiftlyfalling
Copy link
Collaborator

@darrenclark: Very nice. Previously, SQLiteLib worked with xcodebuild if the -destination parameter was specified. This extends support to the -sdk parameter, and it seems that Carthage specifies -sdk instead of -destination.

@groue: I merged the PR into swiftlyfalling/SQLiteLib, (and also updated to SQLite 3.20.1), so that side of things is good to go.

@groue
Copy link
Owner

groue commented Oct 11, 2017

All right, I carry on. Many thanks, for both your analysis and work!

@darrenclark
Copy link
Contributor Author

@swiftlyfalling Ahhh, I was wondering what was causing the difference between Xcode and xcodebuild, good to know about the -destination parameter, I totally missed that.

No problem, thank you & swiftlyfalling for your work on GRDB!

@groue
Copy link
Owner

groue commented Oct 12, 2017

Hello @darrenclark,

On top of your improvements, I'm adding a few tests that check that frameworks built with Carthage can actually be included in applications.

This work in progress is in the development branch.

For the context, I want you to understand why GRDB does not currently support Carthage. It is because I could never achieve reliable Carthage builds. Carthage has a build process that makes it very hard to reproduce errors. The Carthage team is cruelly under-staffed, and can't provide efficient support, on a topic that is difficult (Xcode mastery is not an easily found skill). Support to end users has thus to be provided by third-party libraries like GRDB. Since I don't know anyone that could reliably provide good support for GRDB users that open Carthage-related issues, my choice was eventually to remove Carthage from the supported installation methods, and stop worrying about it. I could not resist writing a rant just before I made this decision (it looks like I had a bad day, since my tone is not very nice).

After this shift, it was no longer GRDB that would not support Carthage. Instead, from now on, it is Carthage that does not support GRDB, and I'm perfectly fine with this way of thinking about it.

Now Carthage build errors are not always due to Carthage, as your PR shows. You do bring a new hope.

But if GRDB has improved, thanks to you, Carthage is still the same. We're not quite there yet: I still sometimes get an error when Carthage builds the GRDB frameworks. This reminds me of the days before I decided that this wasn't my problem any longer.

For example:

$ make test_CarthageBuild
echo '/* Makefile generated */' > SQLiteCustom/GRDBCustomSQLite-USER.h
echo '#define SQLITE_ENABLE_PREUPDATE_HOOK' >> SQLiteCustom/GRDBCustomSQLite-USER.h
echo '#define SQLITE_ENABLE_FTS5' >> SQLiteCustom/GRDBCustomSQLite-USER.h
echo '// Makefile generated' > SQLiteCustom/GRDBCustomSQLite-USER.xcconfig
echo 'CUSTOM_OTHER_SWIFT_FLAGS = -D SQLITE_ENABLE_PREUPDATE_HOOK -D SQLITE_ENABLE_FTS5' >> SQLiteCustom/GRDBCustomSQLite-USER.xcconfig
echo '// Makefile generated' > SQLiteCustom/src/SQLiteLib-USER.xcconfig
echo 'CUSTOM_SQLLIBRARY_CFLAGS = -DSQLITE_ENABLE_PREUPDATE_HOOK -DSQLITE_ENABLE_FTS5' >> SQLiteCustom/src/SQLiteLib-USER.xcconfig
rm -rf Carthage
/usr/local/bin/carthage build --no-skip-current
*** xcodebuild output can be found in /var/folders/5p/dm0blh350879m1kd0d98vbnr0000gn/T/carthage-xcodebuild.HWrwxE.log
*** Building scheme "GRDBCipheriOS" in GRDB.xcworkspace
*** Building scheme "GRDBOSX" in GRDB.xcworkspace
*** Building scheme "GRDBWatchOS" in GRDB.xcworkspace
*** Building scheme "GRDBCipherOSX" in GRDB.xcworkspace
*** Building scheme "GRDBiOS" in GRDB.xcworkspace
*** Building scheme "GRDBCustomSQLiteOSX" in GRDB.xcworkspace
*** Building scheme "GRDBCustomSQLiteiOS" in GRDB.xcworkspace
Build Failed
	Task failed with exit code 65:
	/usr/bin/xcrun xcodebuild -workspace /Users/groue/Documents/git/groue/GRDB.swift/GRDB.xcworkspace -scheme GRDBCustomSQLiteiOS -configuration Release -sdk iphonesimulator -destination platform=iOS\ Simulator,id=2049D1E4-79E6-4492-947F-AEA54B2B6032 -destination-timeout 3 ONLY_ACTIVE_ARCH=NO CODE_SIGNING_REQUIRED=NO CODE_SIGN_IDENTITY= CARTHAGE=YES build (launched in /Users/groue/Documents/git/groue/GRDB.swift)

This usually indicates that project itself failed to compile. Please check the xcodebuild log for more details: /var/folders/5p/dm0blh350879m1kd0d98vbnr0000gn/T/carthage-xcodebuild.HWrwxE.log
make: *** [test_CarthageBuild] Error 1

It looks like the initial build always succeed, but further builds sometimes end with an error. When they do, they fail on a random framework:

make distclean # Restore repo to pristine state
make test_CarthageBuild # Could not make it fail yet
make test_CarthageBuild # Sometimes OK, sometimes ends in error

What do you think?

@darrenclark
Copy link
Contributor Author

Ah I see, makes a lot of sense! And totally understandable why Carthage isn't supported.

I'll do a little more fiddling around over the weekend and see if I turn up anything regarding the random failures.

@groue
Copy link
Owner

groue commented Oct 12, 2017

Yes, @darrenclark, I'm not against a little more help before Carthage doors are opened again.

Whatever the results of your investigations, your improvements to sqlcipher and custom SQLite builds will be merged in: consider your PR accepted. Welcome to GRDB contributors! Even if Carthage remains "not officially supported", so that I can avoid the burden of providing support for a flaky installation method, it will be "more" available to die-hard hackers who are ready to spend their own time fixing bizarre build errors.

@groue groue added the help wanted Extra attention is needed label Oct 13, 2017
@groue
Copy link
Owner

groue commented Oct 23, 2017

@darrenclark I'm about to ship the next GRDB version with your enhancements included, @swiftlyfalling's updated SQLiteLib, and still no "official" Carthage support. I'll close this PR then. Should you eventually grab more information, I'll happily discuss with you again, in a new issue or pull request. Again, thanks a lot for your contribution, and happy GRDB!

@groue
Copy link
Owner

groue commented Oct 24, 2017

v2.1.0 has shipped.

@groue groue closed this Oct 24, 2017
@darrenclark
Copy link
Contributor Author

@groue sounds good!

I haven't had a chance to dig into it a little further yet, but will keep you updated if I eventually discover anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants