Skip to content
This repository has been archived by the owner on Feb 17, 2021. It is now read-only.

Remove shared example layouts scheme so as not to be built by carthage #131

Closed
wants to merge 1 commit into from

Conversation

stowy
Copy link

@stowy stowy commented Apr 19, 2017

@nicksnyder I would like to remove ExampleLayouts from the shared schemes, since it builds when you specify LayoutKit as a Carthage dependency, which is undesirable as it takes additional build time.

If we are to have LayoutKit, which is swift-based, as a dependency, when building on ci without a pre-built artifact, we would like to minimize the build time for swift dependencies.

Swift dependencies can't really be stored as pre-built artifacts AFAIK due to the lack of ABI stability.

@jingwei-huang1
Copy link
Contributor

I think that makes sense to remove from Carthage dependency. @nicksnyder Do you have any objection?

@nicksnyder
Copy link
Collaborator

I am not a fan of this. We shouldn't have to hide schemes just to prevent Carthage from building stuff. Are you sure that Carthage does not provide a way to depend on a specific scheme?

@jingwei-huang1
Copy link
Contributor

@stowy
Copy link
Author

stowy commented Apr 20, 2017 via email

@nicksnyder
Copy link
Collaborator

If Carthage just blindly builds all schemes, that is Carthage's fault. I am kind of surprised they don't want to fix this.

If necessary we will maintain our own fork and hide it there.

Probably the best solution.

May I ask, what is the rational behind having example layouts as a shared
scheme?

When developing an example layout it is nice to be able to build that independently.

takes additional build time.

How much time does it take? ExampleLayouts is really small so it should be very fast to compile.

Does Carthage also build the sample app?

@jingwei-huang1
Copy link
Contributor

@nicksnyder @stowy

How much time does it take? ExampleLayouts is really small so it should be very fast to compile.

It takes around 30 seconds to build.

Does Carthage also build the sample app?

No, it doesn't build application target.

@nicksnyder
Copy link
Collaborator

It takes around 30 seconds to build.

In what environment? How are you measuring?

It takes <8 seconds to build on my early 2015 dual core macbook pro

@jingwei-huang1
Copy link
Contributor

I tested 3x times and the average is 38 seconds to build ExampleLayout-iOS.

I tested on MacBook Pro (Retina, 15-inch, Mid 2015), 2.5 GHz Intel Core i7, 32G RAM

test

@nicksnyder
Copy link
Collaborator

I agree that it is a waste of time to build ExampleLayouts.

Not only is Carthage unnecessarily building ExampleLayouts-iOS when it shouldn't, but LayoutKit itself is getting built twice: once when LayoutKit-iOS is built and a second time because ExmapleLayouts-iOS depends on it (the build isn't cached).

I think this waste of time is caused by Carthage though. Library maintainers should be free to share whatever schemes they please. If Carthage wants to piggy back on shared schemes, then it should provide a way to make schemes as hidden from Carthage consumers.
Carthage/Carthage#1899

@dcaunt
Copy link

dcaunt commented Apr 25, 2017

@jingwei-huang1 you may find the --cache-builds flag helps alleviate the unnecessary rebuilding.

@nicksnyder you may consider attaching a prebuilt binary framework to your GitHub releases, which could avoid building at all for matching Swift versions.

@nicksnyder
Copy link
Collaborator

@dcaunt Thanks for the pointer to prebuilt artifacts doc.

Swift dependencies can't really be stored as pre-built artifacts AFAIK due to the lack of ABI stability.

@stowy For a given version of Swift we can provide a pre-built binary. I would prefer this solution.
https://github.com/Carthage/Carthage/#archive-prebuilt-frameworks-into-one-zip-file

@stowy
Copy link
Author

stowy commented Apr 25, 2017

@nicksnyder great, that sounds like a good solution, thanks. We can close this PR in that case. Thanks @dcaunt and @jingwei-huang1 for the extra input 👍

@staguer
Copy link
Contributor

staguer commented Apr 25, 2017

So the release checklist will be updated to include instructions for adding the pre-build binary? Who is the best person to do that?

@nicksnyder
Copy link
Collaborator

The linked instructions include steps for configuring Travis to do it automatically

@nicksnyder nicksnyder closed this May 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants