RxSwiftCommunity / RxKeyboard

Reactive Keyboard in iOS
MIT License
1.6k stars 176 forks source link

Carthage Build Failed #45

Closed daidongon closed 6 years ago

daidongon commented 6 years ago
imairi commented 6 years ago

+1

freak4pc commented 6 years ago

Does this work on Xcode 9.0.1 GM?

devxoul commented 6 years ago

I pushed new tag(0.7.1) which supports Xcode 9.1. Travis CI will upload a new prebuilt binary for 0.7.1 then you'll be able to use RxKeyboard in Xcode 9.1 :)

moogle19 commented 6 years ago

@devxoul Travis CI is unfortunately still failing. I think it is because of the missing env-var $FRAMEWORK.

devxoul commented 6 years ago

@moogle19, I fixed that and it now succeeded!

moogle19 commented 6 years ago

@devxoul But this is only working if the target is building with XCode 9.1 and is using the binary framework. It still needs a .xcodeproj and a shared scheme to support Carthage.

devxoul commented 6 years ago

@moogle19 if you're using Xcode 9.0 you can use 0.7.0 instead. I manage RxKeyboard's dependencies with Swift Package Manager which is not compatible with Carthage.

moogle19 commented 6 years ago

@devxoul Okay. What is the reason that you are using SPM? I mean SPM currently does not supported iOS projects and I think RxKeyboard is targeted for iOS projects.

devxoul commented 6 years ago

I'd like to keep this repository clean and tidy by getting rid of .xcodeproj file. It was not comfortable for me to manage a library for both CocoaPods and Carthage with .xcodeproj file when the library has dependencies to other libraries. Swift Package Manager provides a really easy way to manage dependencies in a development stage.

Also, whether SPM supports iOS or not is an unrelated topic because I mainly distribute this library to CocoaPods and Carthage. SPM is just a tool for a dependency management in a development stage.

imairi commented 6 years ago

carthage update --platform iOS RxKeyboard , downloaded 0.7.1 binary. but the below errors occurred.

2017-11-02 9 05 55

I use Xcode Version 9.1 (9B55).

bsrz commented 6 years ago

@devxoul this is really harming the community and the popularity of this library. Supporting Carthage is easy and allows to have prebuilt libraries. With the amount of time it takes to compile CocoaPod projects, it's really not an option to not support Carthage. Otherwise, an incredible amount of productivity is lost.

If you still choose to not support Carthage, someone will fork this and support it very easily and then there is a potential to for this repo to not be the only official repo for RxKeyboard. I really hope you reconsider your position since Carthage isn't going away any time soon.

bsrz commented 6 years ago

Additionally, the README on master still explicitly says you support Carthage with the instructions. This feels closer to "well me, myself and I don't need it so I'm not going to support it" more than a good decision for the community.

devxoul commented 6 years ago

@imairi, I think the Xcode version in Travis is not compatible with the app store version. I just updated the binary with a new one. Could you please try again?

@bsarrazin, I have no idea why do you think that RxKeyboard doesn't support Carthage. The comment above is about why I don't keep .xcodeproj file. We have prebuilt binaries in each releases.

imairi commented 6 years ago

@devxoul thanks, it works on Version 9.1 (9B55).

devxoul commented 6 years ago

@imairi, I also submitted a report to Travis about the Xcode version issue. Thanks!

daidongon commented 6 years ago

@devxoul, Thank you!

bsrz commented 6 years ago

@devxoul getting rid of the .xcodeproj forces the community to jump through hoops to build the binary themselves. Distributing a prebuilt bin can be great, but it assumes/forces which version of Swift people are using. Carthage doesn'T support multiple versions of Swift in a prepackaged bin.

bsrz commented 6 years ago
*** Skipped installing RxKeyboard.framework binary due to the error:
    "Incompatible Swift version - framework was built with 4.0.2 (swiftlang-900.0.69.2 clang-900.0.38) and the local version is 4.0 (swiftlang-900.0.65.2 clang-900.0.37)."
devxoul commented 6 years ago

@bsarrazin, which Xcode version did you use?

bsrz commented 6 years ago

I'm running this with Xcode 9.0.1.

devxoul commented 6 years ago

@bsarrazin, you may use 0.7.0 instead.

bsrz commented 6 years ago

I understand the solution. My point is about what is assumed/forced by this library vs what is good for the community. Forcing folks to fork the repo, creating an Xcode project, changing their Cartfile to point to their forked repo, running the build and maintaining that forked repo with updates... it's a lot more work than keep a simple xcode project in this repo.

devxoul commented 6 years ago

I have never forced them to fork this repository. It's obvious that RxKeyboard supports Carthage in my preferred way. What's the problem?

bsrz commented 6 years ago

Claiming to support Carthage because you distribute a zip file in the GitHub release is not 100% correct. You're supporting only a sliver of what it means to support Carthage. Supporting Carthage means having an Xcode project with a shared scheme (maybe multiple schemes if the lib can run on more than on operating system, e.g. RxKeyboard-iOS, RxKeyboard-tvOS, etc...). Having this Xcode project allows for consumers of the library to build the lib according the specs of their project.

What if someone wants to use 0.7.2 to ensure using the best version of the library but their project hasn't been converted to Swift 4 yet? Even more specific, users can't use 0.7.2 because it has been compiled with Xcode 9.1 (with Swift 4.0.2) which is incompatible with their project using Xcode 9.0.1 (with Swift 4.0). In some cases, the people maintaining CI machines or workstations are the ones deciding when the upgrade of software like Xcode is done.

Right now, it would only be correct to say you support Carthage only for projects using Xcode 9.1 as a precompiled binary using Swift 4.0.2. Nothing more.

devxoul commented 6 years ago

Thanks for the detailed reply. Now I see what you're worrying about. I'll keep that in my mind. But it is still more fun to me to use SPM than using a xcodeproj file and creating a bunch of targets. I want to maintain this repository in my preferred way. If somebody doesn't agree with this, he/she could fork this repository anytime :)

bsrz commented 6 years ago

more fun to me

in my preferred way

could fork this repository anytime

Those are the points I was worrying about. The decision that was made to remove the xcodeproj is for yourself only, not for the community. Suggesting a fork with an Xcode project means there are no longer a single source of truth for this repo.

devxoul commented 6 years ago

Hmm, I thought an open source was for fun. What do you think is 'for community'? Is that more important than doing something interesting? And why does the single source of truth matter?

freak4pc commented 6 years ago

@devxoul I understand your POV here, but if this is a community project it's beneficial it would support as many installation methods as possible (SPM isn't really popular, yet)

Could you share your concerns on moving away from the xcodeproj solution ? I'm not a huge Carthage user but would love to understand

devxoul commented 6 years ago

@freak4pc, xcodeproj file has a large dependency to a Xcode version and it contains a lot of unnecessary information. I always had to worry about conflicts and unintended diffs. Also it was complicated for me to manage a library if it has dependencies to other libraries. For example, RxKeyboard has dependencies to RxSwift and RxCocoa. With a traditional way(with xcodeproj), I had to use only Carthage for a dependency management (even I'm not a Carthage user). Because CocoaPods-integrated xcodeproj file was not able to be built via Carthage installation. A Git submodule was tricky to manage a specific version. I had a bunch of open source projects so I needed a smarter way to manage whole projects. SPM was the most elegant way to achieve what I wanted to do.

And please note that RxKeyboard does support Carthage. There are many projects in RxSwiftCommunity which don't support Carthage at all.

daidongon commented 6 years ago

more fun to me

in my preferred way

I think that your selfish thinking is not good for this community.

but if this is a community project it's beneficial it would support as many installation methods as possible (SPM isn't really popular, yet)

That is exactly right. SPM is still very minority.

devxoul commented 6 years ago

@daidongon, Could you please explain me why that thinking is selfish? I am also a member of the community. I didn't harm anyone, and I was just doing my best to contribute to the community.

daidongon commented 6 years ago

Carthage and CocoaPods are the major package managers in the iOS community, while SPM is very minor and completely does not support iOS projects. Your idea of ​​adopting only such an incomplete method is selfish and it will not be the best way in this community.

devxoul commented 6 years ago

@daidongon, A prebuilt binary is an official way to support Carthage.

daidongon commented 6 years ago

By not publishing the xcodeproj file, every time the version of Xcode goes up, build error problems continue to occur, every time people other than you are in trouble. This issue is one of them.

freak4pc commented 6 years ago

@RxSwiftCommunity/contributors Any of the core contributors want to give their opinion to this discussion as well?

devxoul commented 6 years ago

@daidongon Hmm, isn't that an issue of Carthage? I think https://github.com/Carthage/Carthage/issues/2219 is a better place to discuss about that.

devxoul commented 6 years ago

@daidongon Why should I? I added a prebuilt binary support even I don't use Carthage. And fixed problems every issues you filed. I thought that I had a responsibility to do that at least. But open sourcing is not my primary job. I'm not a charity nor your employee :(

muukii commented 6 years ago

If we can find how to create .xcodeproj on build time. This problem will be solved.

devxoul commented 6 years ago

@muukii Yeah, actually I had a same idea and tried to add a SPM integration on Carthage (https://github.com/Carthage/Carthage/pull/1945) but I faced some problems. I hope someone could develop this idea.

groue commented 6 years ago

Fun is surely part of the game. Fun of a library's owner, and also fun for the library users.

Library users have no fun when a library doesn't make its requirements clear. Nobody likes working with an optimistic spirit, only to eventually discover that the whole task was bound to fail.

I thus think that it's good that a repo owner spends a little fun documenting the supported installation methods for the supported platforms. This, at least, makes it clear what is supported out of the box, and avoids user frustration.

On the other side, when a user wants to extend the installation methods (by adding an Xcode project in our particular case), then it's a good idea that he spends a little fun helping the repo owner on the maintenance of the added installation method. A great way to do this is to submit a PR that contains documentation and tests. This way, the repo owner doesn't have to worry much about supporting the new feature in the future.

@devxoul has no obligation supporting any feature request. Yet I suggest that RxKeyboard would extend its Carthage documentation with all subtleties about Xcode/Swift versions we have seen above.

If someone wants to ease Carthage integration for more Xcode versions, then I'm sure a motivated PR that comes with a clear howto section and automated tests will be examined with great care.

devxoul commented 6 years ago

@groue, thanks for suggestion. I added a quick guide about a contributing and a Carthage installation. (https://github.com/RxSwiftCommunity/RxKeyboard/commit/2b480d1248ea485b23ba212e1803d96c8d236597)

groue commented 6 years ago

I don't know if it is possible to automatically generate an Xcode project with a few shared schemes from an SPM package. But assuming it is possible, then one could help by supporting two Makefile targets:

make RxKeyboard.xcodeproj # Generates the Xcode project
make test                 # Tests both SPM and Carthage

The Makefile could look like below:

default: test

CARTHAGE := $(shell command -v carthage)
XCRUN := $(shell command -v xcrun)
SWIFT = $(shell $(XCRUN) --find swift 2> /dev/null)

test: testRxKeyboard testCarthage

testRxKeyboard:
    $(SWIFT) package clean
    $(SWIFT) build
    $(SWIFT) build -c release
    set -o pipefail && $(SWIFT) test

RxKeyboard.xcodeproj:
    # generate RxKeyboard.xcodeproj and schemes

testCarthage: RxKeyboard.xcodeproj
ifdef CARTHAGE
    rm -rf Carthage
    $(CARTHAGE) build --no-skip-current
else
    @echo Carthage must be installed for testCarthage
    @exit 1
endif

.PHONY: test xcode testRxKeyboard testCarthage

Making sure a release would please all users would then only cost a simple make test.

devxoul commented 6 years ago

Actually Carthage uses xcodebuild and I don't use swift test to test RxKeyboard so we may not need both of them. We can just do as I does in:

groue commented 6 years ago

If swift package generate-xcodeproj generates a project that is part of the release, then it looks like yes, we'd get top-notch Carthage support, and you'd still be able to keep on focusing on SPM 👍

groue commented 6 years ago

Actually Carthage uses xcodebuild

Yes, it does, but with subtle differences that go well beyond my Xcode skills. The net effect of those differences is that an xcodebuild success is not a guarantee that carthage build will work as well. I've spent enough hours trying to understand spurious and non-reproducible Carthage errors with GRDB to know that 1st hand, unfortunately (that was not fun at all ;-)). A good test for Carthage support does contain an explicit carthage build command.

devxoul commented 6 years ago

That makes sense. By the way I have to write tests for RxKeyboard first. It depends on a lot of singleton instances and UI stuffs. I should start from decoupling dependencies. Hmm, it'll be fun.

groue commented 6 years ago

One step after the other, isn't it?

Meanwhile, unless I'm mistaken, Xcode 9.1 users should use 0.7.1, and Xcode 9 users should use 0.7.0.

devxoul commented 6 years ago

@groue, I couldn't understand what you mean. 🤔

groue commented 6 years ago

Sorry, let me try again. It's OK if you improve RxKeyboard at your own pace, even if the library is pretty popular, and has rightfully demanding users. And since it looks like Carthage is available for both Xcode 9.0 and Xcode 9.1, I guess this issue is closed for good. Future improvements will come from you or from pull requests.

icanzilb commented 6 years ago

I don't want to spam this thread but I've been following it and @groue regarding "and has rightfully demanding users" I'd like to mention that RxKeyboard comes under the MIT license which states "THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT." therefore any demanding is not rightful. Using language like this does not imho add value to the discussion, but does put pressure on people who in their free time write software for everyone to use for free.