Skip to content

Fix Carthage project setup#35

Merged
freak4pc merged 9 commits into
masterfrom
fix/carthage-project-setup
Jul 10, 2018
Merged

Fix Carthage project setup#35
freak4pc merged 9 commits into
masterfrom
fix/carthage-project-setup

Conversation

@mkko

@mkko mkko commented Jun 25, 2018

Copy link
Copy Markdown
Contributor

This PR fixes project structure to work with Carthage.

The original issue is described here. To resolve it, I added a workspace file to the root of the project. While at it, I also took liberty of moving source files out of Pods directory, upgrading the project for Xcode 9.4 and cleaning up a bit in podspec.

@freak4pc

freak4pc commented Jul 8, 2018

Copy link
Copy Markdown
Member

@mkko Can you further explain why this is needed? "Prefers xcworkspace" makes sense, but there isn't one so I would expect it to just build xcproj in that case. Is this a bug in Carthage or this repo?

@mkko

mkko commented Jul 8, 2018

Copy link
Copy Markdown
Contributor Author

There's one under examples. Carthage doesn't care about the layout of the project which is why we need one workspace at the top level.

@freak4pc

freak4pc commented Jul 8, 2018

Copy link
Copy Markdown
Member

It's a bit strange but "it is what it is", as they say.
I think we should add Carthage build test and also build the workspace as part of CI in the circle.yml file potentially. What do you think?

@mkko

mkko commented Jul 8, 2018

Copy link
Copy Markdown
Contributor Author

In general that would be a good idea. However, for this issue it wouldn't have helped since there was no build errors. I don't know what went wrong with the build, though. All was good until running the project, I'm guessing something weird with the linker.

This is horribly implicit feature with Carthage and I'm a little worried that at some point someone might clean up the practically empty workspace file out of the project. Building the workspace in CI would at least give us some way of giving an error about possibly broken Carthage builds.

I'm new to Circle, but I'll try to add a build phase there and see how it looks.

@freak4pc

freak4pc commented Jul 8, 2018

Copy link
Copy Markdown
Member

I'll try to help out with this tomorrow

@freak4pc freak4pc force-pushed the fix/carthage-project-setup branch 3 times, most recently from b03f401 to 1a6e00d Compare July 10, 2018 08:48
@freak4pc freak4pc force-pushed the fix/carthage-project-setup branch from 1a6e00d to b8f5d2d Compare July 10, 2018 08:50
@freak4pc freak4pc force-pushed the fix/carthage-project-setup branch from af48e96 to 9fdc436 Compare July 10, 2018 09:00
@freak4pc

Copy link
Copy Markdown
Member

Ok this should be good to go + Migrated to Circle CI v2.
Thanks for your help!

@freak4pc freak4pc merged commit 979173c into master Jul 10, 2018
@freak4pc freak4pc deleted the fix/carthage-project-setup branch July 10, 2018 09:05
@mkko

mkko commented Jul 10, 2018

Copy link
Copy Markdown
Contributor Author

Thanks! Didn't even have the time to react on this.

However, I'm wondering the purpose of bootstrap.sh. I think copying the .resolved file over the original Cartfile will break the specified version requirements (i.e. ~> 1.0 becomes "1.0" which refers to a specific commit). That is, unless I misunderstood the purpose of the bootstrap in this case.

@freak4pc

freak4pc commented Jul 10, 2018

Copy link
Copy Markdown
Member

Hey @mkko - sorry for not leaving open, I thought we're just trying to fix the CI and merge as the fix itself was clear to go.

The entire script is supposed to cache the Carthage folder - so if you now create another PR or branch, you'll see that it says its already cached and won't have to rebuild it (which is a very lengthy thing with Carthage).

The way Circle CI work is that it can cache specific folders - in our case we're caching Carthage. But Carthage.resolved isn't part of that folder, as it lives in the root folder of the project. So the script really goes:

Initial attempt: Carthage/Cartfile.resolved doesn't exist, so no cache to check against. Going to build. After build copy the root Cartfile.resolved to Carthage/Cartfile.resolved so its cached along with that folder in the CI.

Next attempt: Cache is pulled. Carthage/Cartfile.resolved exists and we compare it with the "root" Cartfile.resolved. If they're equal, the cached build can be used without rebuilding the whole thing. If the files are different (like with a version change), it does the "initial attempt" flow again (rebuild, copy resolved, cache).

Hope this makes sense!

@mkko

mkko commented Jul 10, 2018

Copy link
Copy Markdown
Contributor Author

No worries, I meant it in a good way. Good to get things delivered.

Thanks for the clarification! I actually misread the copy phase and had the impression that the .resolved was used as the new Cartfile and that sounded alarming to me.

Anyways, thank you for the CI part, would've taken me a lot longer to make it work. Also, care to do the next release with this in it? I would gladly do it but I can't push this to CocoaPods.

@freak4pc freak4pc changed the title Fix/carthage project setup Fix Carthage project setup Jul 11, 2018
@freak4pc

Copy link
Copy Markdown
Member

All done!

🎉 Congrats

🚀 RxMKMapView (4.2.0) successfully published
📅 July 11th, 11:23
🌎 https://cocoapods.org/pods/RxMKMapView
👍 Tell your friends!

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.

2 participants