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

Use Carthage rather than CocoaPods to manage dependencies #169

Merged
merged 5 commits into from
Oct 2, 2018

Conversation

tonyarnold
Copy link
Contributor

I saw a few comments that you were having difficulties with CocoaPods and getting an update into Brew.

You don't have to use this as-is (or at all!) but I thought it might be helpful. I've tested that this builds, and that the tests pass, but you should review these changes closely.

@todo
Copy link

todo bot commented Sep 4, 2018

eventually migrate the global to this environment value

// TODO: eventually migrate the global to this environment value
var assertionHandler: AssertionHandler {
get { return NimbleAssertionHandler }
set { NimbleAssertionHandler = newValue }
}


This comment was generated by todo based on a TODO comment in 7532bc4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 7532bc4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

eventually migrate the global to this environment value

// TODO: eventually migrate the global to this environment value
var assertionHandler: AssertionHandler {
get { return NimbleAssertionHandler }
set { NimbleAssertionHandler = newValue }
}


This comment was generated by todo based on a TODO comment in 7532bc4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 7532bc4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

eventually migrate the global to this environment value

// TODO: eventually migrate the global to this environment value
var assertionHandler: AssertionHandler {
get { return NimbleAssertionHandler }
set { NimbleAssertionHandler = newValue }
}


This comment was generated by todo based on a TODO comment in 7532bc4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 7532bc4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

eventually migrate the global to this environment value

// TODO: eventually migrate the global to this environment value
var assertionHandler: AssertionHandler {
get { return NimbleAssertionHandler }
set { NimbleAssertionHandler = newValue }
}


This comment was generated by todo based on a TODO comment in 7532bc4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 7532bc4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 57057f5 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 57057f5 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 57057f5 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 57057f5 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in fba3ad4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in fba3ad4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in fba3ad4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in fba3ad4 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 3b1d786 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 3b1d786 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 3b1d786 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 3b1d786 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 7f00eba in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 7f00eba in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 7f00eba in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 4, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in 7f00eba in #169. cc @tonyarnold.

@masclibot
Copy link

1 Warning
⚠️ 😵 Big PR
1 Message
📖 This PR might seem trivial, but every contribution counts. 😘

Generated by 🚫 Danger

@phatblat
Copy link
Member

phatblat commented Sep 5, 2018

LOL, sorry about the todobot spam. I'm not sure, but the config change might not take effect until it's merged to master. I'm laughing because I also work on Quick and Nimble but haven't set up the bot on that org, but it looks like I should.

This is fantastic and something that I was begrudgingly going to do but was tired from all the work to get CocoaPods integrated and the brew formula to build. Thank you so much! I'll get back to you with feedback once I get a chance to look it over.

@phatblat phatblat mentioned this pull request Sep 5, 2018
4 tasks
script/build Outdated Show resolved Hide resolved
@todo
Copy link

todo bot commented Sep 5, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in e7b4b48 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 5, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in e7b4b48 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 5, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in e7b4b48 in #169. cc @tonyarnold.

@todo
Copy link

todo bot commented Sep 5, 2018

test & verify correct behavior

// TODO: test & verify correct behavior
internal func prepended(message: String) -> ExpectationMessage {
return .prepends(message, self)
}
/// Converts the tree of ExpectationMessages into a final built string.


This comment was generated by todo based on a TODO comment in e7b4b48 in #169. cc @tonyarnold.

@phatblat
Copy link
Member

phatblat commented Sep 5, 2018

Now I remember why I was putting this off, CocoaPods solved one thing by statically compiling the swift dependencies. The binary built from this PR blows up with this dyld error:

./mas version
dyld: Library not loaded: @rpath/Commandant.framework/Commandant
Referenced from: /Users/phatblat/dev/macos/mas/build/./mas
Reason: image not found
fish: './mas version' terminated by signal SIGABRT (Abort)

Lo and behold Carthage 0.30 also supports building static frameworks. However, it looks like the frameworks need to support static linking. I wonder whether the MACH_O_TYPE build setting could be forced to "static library" (or whatever the value really is).

@tonyarnold
Copy link
Contributor Author

I did wonder about that. The Carthage binary ships with a Frameworks directory that contains its dependencies:

¶ Repositories/mas at carthage – otool -L /usr/local/bin/carthage 
/usr/local/bin/carthage:
	@rpath/CarthageKit.framework/CarthageKit (compatibility version 1.0.0, current version 1.0.0)
	/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1452.23.0)
	/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.50.4)
	@rpath/Commandant.framework/Commandant (compatibility version 1.0.0, current version 1.0.0)
	@rpath/Curry.framework/Versions/A/Curry (compatibility version 1.0.0, current version 1.0.0)
	@rpath/PrettyColors.framework/Versions/A/PrettyColors (compatibility version 0.0.0, current version 0.0.0)
	@rpath/ReactiveSwift.framework/ReactiveSwift (compatibility version 1.0.0, current version 1.0.0)
	@rpath/ReactiveTask.framework/ReactiveTask (compatibility version 1.0.0, current version 1.0.0)
	@rpath/Result.framework/Versions/A/Result (compatibility version 1.0.0, current version 1.0.0)
	@rpath/Tentacle.framework/Versions/A/Tentacle (compatibility version 1.0.0, current version 1.0.0)
	@rpath/XCDBLD.framework/XCDBLD (compatibility version 1.0.0, current version 1.0.0)
	@rpath/libswiftAppKit.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftCore.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftCoreData.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftCoreFoundation.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftCoreGraphics.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftCoreImage.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftDarwin.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftDispatch.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftFoundation.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftIOKit.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftMetal.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftObjectiveC.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftQuartzCore.dylib (compatibility version 1.0.0, current version 902.0.54)
	@rpath/libswiftXPC.dylib (compatibility version 1.0.0, current version 902.0.54)
¶ Repositories/mas at carthage – find /usr/local -name CarthageKit.framework
/usr/local/Frameworks/CarthageKit.framework
/usr/local/Cellar/carthage/0.30.1/Frameworks/CarthageKit.framework

I wonder if that's acceptable for mas?

@phatblat
Copy link
Member

phatblat commented Sep 5, 2018

Yeah, that's acceptable. The binary is probably sensitive to the relative path to that Frameworks folder, but as the vast majority of mas users install through Homebrew (I believe) this should not be an issue.

@tonyarnold
Copy link
Contributor Author

Ok, so it appears that the way Carthage achieves this is by providing both a binary (carthage) and a framework (CarthageKit.framework) - the framework contains the dependent frameworks, as well as the Swift dylibs. The binary then sets its LD_RUNPATH_SEARCH_PATHS to include both @executable_path/CarthageKit.framework/Versions/Current/Frameworks and /Library/Frameworks/CarthageKit.framework/Versions/Current/Frameworks.

It looks like it might be necessary to split the tool into a backing framework, and a binary.

Do you know what? This might be easier using Swift Package Manager - let me do some quick reading and I'll get back to you.

@phatblat
Copy link
Member

phatblat commented Sep 5, 2018

I do have a WIP in the maskit branch to do the same thing for easier testing of the logic. However, if SwiftPM makes this easier, that might be worth a go. I'm thinking we need to solve this either way, but if the package manager can do some of the work that makes it easier. CocoaPods did a bunch of ✨ but ultimately the master spec repo ended up being a real pain when building within the Homebrew sandbox.

@phatblat phatblat merged commit 49a1784 into mas-cli:master Oct 2, 2018
phatblat added a commit that referenced this pull request Oct 2, 2018
@tonyarnold tonyarnold deleted the carthage branch October 4, 2018 05:46
@tonyarnold
Copy link
Contributor Author

I had a good play with SwiftPM, but I don't think it's ready for this (yet) sorry!

Gosh it'll be nice when we have vendor-supported tooling for things like mas!

@phatblat phatblat mentioned this pull request Oct 14, 2018
@phatblat
Copy link
Member

Thanks for the tip about how Carthage is packaged @tonyarnold. It's split now into a tiny executable and a framework. Now just need to update the Homebrew formula to put everything into place on install.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants