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

allow frameworks to specify which schemes Carthage should build #1616

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@jpsim
Copy link
Contributor

jpsim commented Nov 29, 2016

By specifying an optional .carthage_schemes file at the root of a framework repo, Carthage can skip much of the work involved in evaluating which schemes to build in a directory.

If present, the .carthage_schemes file should be a newline-separated list of scheme names that Carthage should build.

The work involved in evaluating which schemes to build can be substantial due to requiring xcodebuild invocations. In the case of the Realm frameworks, for example, using this approach shaves minutes off the total update time.

Since frameworks are sometimes useful for purposes internal to a repo and not meant to be consumed externally, specifying a subset of schemes to build in .carthage_schemes can also great speed up build times.

This can also be useful to work around certain Xcode bugs, such as when Xcode considers unit test bundle products to actually be framework products rdar://29407087.

allow frameworks to specify which schemes Carthage should build
By specifying an optional `.carthage_schemes` file at the root of a
framework repo, Carthage can skip much of the work involved in
evaluating which schemes to build in a directory.

If present, the `.carthage_schemes` file should be a newline-separated
list of scheme names that Carthage should build.

The work involved in evaluating which schemes to build can be substantial
due to requiring `xcodebuild` invocations. In the case of the Realm
frameworks, for example, using this approach shaves minutes off the total
update time.

Since frameworks are sometimes useful for purposes internal to a repo
and not meant to be consumed externally, specifying a subset of schemes
to build in `.carthage_schemes` can also great speed up build times.
@jpsim

This comment has been minimized.

Copy link
Contributor Author

jpsim commented Nov 29, 2016

Building both Realm.framework and RealmSwift.framework is ~40% faster with this PR (499s vs 839s), since it works around rdar://29407087.


Current master:

$ time carthage update --platform osx
*** Fetching realm-cocoa
*** Checking out realm-cocoa at "a772de1626c9e6b3279c508375a539c29c77244c"
*** xcodebuild output can be found in /var/folders/np/5y4sgt8x3r13sk7x0n31z_3m0000gn/T/carthage-xcodebuild.JBCzD7.log
*** Building scheme "Realm" in Realm.xcworkspace
*** Building scheme "Object Server Tests" in Realm.xcworkspace
*** Building scheme "RealmSwift" in Realm.xcworkspace
carthage update --platform osx  839.58s user 86.48s system 262% cpu 5:52.89 total

This PR:

$ time carthage update --platform osx
*** Fetching realm-cocoa
*** Checking out realm-cocoa at "a772de1626c9e6b3279c508375a539c29c77244c"
*** xcodebuild output can be found in /var/folders/np/5y4sgt8x3r13sk7x0n31z_3m0000gn/T/carthage-xcodebuild.VXjlfY.log
*** Building scheme "Realm" in Realm.xcworkspace
*** Building scheme "RealmSwift" in Realm.xcworkspace
carthage update --platform osx  499.41s user 45.25s system 238% cpu 3:48.07 total

Unfortunately, Carthage still builds Realm.framework once too many, since RealmSwift.framework uses it as a dependency, and Carthage always cleans before building, but that's a different battle.

@younata

This comment has been minimized.

Copy link
Contributor

younata commented Nov 29, 2016

That looks like a pretty good idea, and has been something I've been wanting to do myself.

Would it be possible to add some test coverage for this?

@Pr0Ger

This comment has been minimized.

Copy link

Pr0Ger commented Nov 29, 2016

I think it's better to give this kind of control to project instead of framework. For example project can build only 'Realm' scheme, or only 'RealmSwift' dependes on what it is required.

Relevant issue: #1227

@jpsim

This comment has been minimized.

Copy link
Contributor Author

jpsim commented Nov 29, 2016

I think it's better to give this kind of control to project instead of framework.

That's a nice idea, but orthogonal to this feature. It doesn't address preventing internal-only frameworks from being exposed externally.

For example, Realm uses a framework called PlaygroundFrameworkWrapper to enable using a prebuilt framework in a playground (typically only allowed for framework targets defined in an Xcode workspace), which is only used for packaging purposes, and should never be exposed externally. We've had to unshare this framework scheme to cater to Carthage's behavior (realm/realm-cocoa@9c38192), which isn't ideal and shouldn't be required of any project maintainer since it adversely affects non-Carthage use cases.

@jpsim

This comment has been minimized.

Copy link
Contributor Author

jpsim commented Nov 29, 2016

Would it be possible to add some test coverage for this?

I'm happy to add tests and documentation, but before I invest time into that, I'd like to get a sense from Carthage maintainers whether or not this is an idea they're generally comfortable with.

@mdiep

This comment has been minimized.

Copy link
Member

mdiep commented Dec 1, 2016

Thanks for the pull request. I always really appreciate when people open a PR instead of expecting maintainers to do all the work. ☺️

I do feel a little uncomfortable with this because it opens the door to a lot of additional configuration and complexity—and Carthage has eschewed all configuration. I feel this both on an implementation level (where the codebase needs to be refactored to handle the existing complexity) and conception level (where we'd probably want a larger vision for what configuration we want to allow and how it works).

But I also respect that this is a very pragmatic choice. That's part of why I've let this sit for a couple days before responding. Idealism vs. pragmatism is a hard line to manage.

I noticed that your radar had a response. Have you tried out the solution they gave? Knowing whether that solves the issue at hand seems like the next step.

@jpsim

This comment has been minimized.

Copy link
Contributor Author

jpsim commented Dec 1, 2016

I always really appreciate when people open a PR instead of expecting maintainers to do all the work.

Same! And this is a better way to spawn a more concrete conversation than by having a hand-wavy feature request issue.

I do feel a little uncomfortable with this because it opens the door to a lot of additional configuration and complexity—and Carthage has eschewed all configuration. I feel this both on an implementation level (where the codebase needs to be refactored to handle the existing complexity) and conception level (where we'd probably want a larger vision for what configuration we want to allow and how it works).

I can appreciate that. However in practice, as a library maintainer with a fairly complex repo structure, I can safely say that conforming to Carthage's expectations has been more of a burden, and needed more refactoring than this simple configuration option involves.

For example, there's no way currently to have internal-only frameworks (like the PlaygroundFrameworkWrapper example above) without also making compromises, which in some cases aren't viable, such as unsharing the framework scheme in that specific example.

If we explicitly controlled which schemes we wanted Carthage to consider building, there would be less of a burden on library maintainers to consider the Carthage implications of adding another framework in their repo, sometimes deeply nested in an examples/ subdirectory.

Please keep in mind as well that this file is entirely optional and won't affect the majority of library maintainers with very simple project structures.

As for wanting to explore a more general strategy for configuration, that's a fair argument and I respect if that's the outcome of this. However, it is disappointing. I'd much rather see this implemented with a warning saying that it's a temporary solution until a more comprehensive configuration approach can be designed and implemented. As you can tell, it's also not a feature that will require a high maintenance burden (it's dead simple for this reason), so it could conceivably be preserved even after a different configuration strategy is introduced.

I noticed that your radar had a response. Have you tried out the solution they gave? Knowing whether that solves the issue at hand seems like the next step.

Yes, but that's only a partial solution to one of the motivating factors for this feature (that Xcode behavior). It doesn't address the "internal-only framework" scenario presented above.

@Antondomashnev Antondomashnev referenced this pull request Jan 4, 2017

Open

Skip schemes #1227

@guidomb

This comment has been minimized.

Copy link
Contributor

guidomb commented Jan 4, 2017

This could be a solution to the problem of working with dependencies like https://github.com/aws/aws-sdk-ios. They have lots of scheme and the entire project takes around 30 minutes to compile. Even if as a library consumer I only want to use AWS S3 I need to be build all their services frameworks. Users of AWS iOS SDK that include it via CocoaPods don't have this issue because they can use subspec but this is not an option with Carthage.

Although we can discuss if the way AWS SDK is structure is adequate or not, having the solution that @jpsim propose will make our life easier and will lower the barrier for library maintainers to adopt Carthage.

@guidomb

This comment has been minimized.

Copy link
Contributor

guidomb commented Jan 4, 2017

@jpsim Is this an option only for the library maintainer or for libraries consumers too? Because if this is only for maintainers then what I said before about AWS SDK does not apply and maybe a solution like #1227 would be better.

@jpsim

This comment has been minimized.

Copy link
Contributor Author

jpsim commented Jan 4, 2017

As-is, this would only be for library maintainers.

@mdiep

This comment has been minimized.

Copy link
Member

mdiep commented Jan 5, 2017

Since frameworks are sometimes useful for purposes internal to a repo and not meant to be consumed externally

Do you have any examples of this? Internal frameworks that are part of shared schemes? (The Object Server Tests case above doesn't fit since it was an Xcode/configuration issue.)

(Sorry for the delay. But now that ReactiveSwift 1.0 has shipped, I should be able to wrap up this discussion.)

@jpsim

This comment has been minimized.

Copy link
Contributor Author

jpsim commented Jan 5, 2017

An example is the PlaygroundFrameworkWrapper that I mentioned in a previous issue.

@jpsim

This comment has been minimized.

Copy link
Contributor Author

jpsim commented Jan 5, 2017

*previous comment

@mdiep

This comment has been minimized.

Copy link
Member

mdiep commented Jan 8, 2017

Unfortunately, I don't think that example warrants adding this to Carthage.

This feature brings a fair bit of risk to Carthage. Although the feature itself isn't complicated, the configuration issue could quickly become a large breaking change in the community. If authors start relying on it to hide broken schemes, then we can't break that support.

Given that this is a minor case, has a workaround, and should really be a enhancement request for Xcode, I don't think it's a good idea to accept it. Carthage has to walk a very fine line here. Since the impact and reach are both small, I don't think it's a good idea.

@jpsim

This comment has been minimized.

Copy link
Contributor Author

jpsim commented Jan 10, 2017

Understood.

@jpsim jpsim closed this Jan 10, 2017

@jpsim jpsim deleted the jpsim:jp-carthage-schemes branch Jan 10, 2017

@arkdan

This comment has been minimized.

Copy link

arkdan commented Jul 16, 2017

@mdiep i deeply regret you judged the question this way.

  1. since the change is optional, there is a long way before the authors start checking in "broken schemes" because of it.
  2. can we really consider carthage stopping authors from "hiding broken schemes"?
  3. on one side we have real value to developers ("pragmatism") vs your opinion ("idealism"), which, i'm sorry to say, can not be enough to stop this discussion. Rather, you could suggest another approach around the problem, or maybe keep the discussion open for the community and other maintainers.
  4. i would definitely benefit if carthage supported specifying a scheme.
  5. defer {} is not needed most of the time - but when it is, it brings real value. I sure wasn't sad when i got defer {} at my disposal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.