DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Carthage Support #305

Closed
hartbit wants to merge 2 commits from carthage-support into master
pull from: carthage-support
merge into: DaddyFrosty:master
Conversation 9 Commits 2 Files Changed 22 +840 -434
hartbit commented 2016-12-26 08:25:31 -08:00 (Migrated from github.com)
Copy Link

This is a pull request to support the Carthage dependency manager for Objective-C and Swift development. Unfortunately, doing this is causing a few issues:

  1. The buck build is now broken: I have the impression we can't #import <yoga/Yoga.h> from a public header of the framework because the importing module does not know the yoga module. This was the case in UIView+YogaKit.h. Fixing this requires adding the C headers as standard headers of the framework and #import <YogaKit/Yoga.h> instead. Unfortunately, this has broken the buck build. I don't know enough about buck to fix this. Any ideas?

  2. The tests fail in Xcode: Weirdly enough, two tests which run fine when run with buck test //YogaKit:YogaKitTests --config cxx.default_platform=iphonesimulator-x86_64 --config cxx.cflags=-DTRAVIS_CI fail when run inside an Xcode test target. Those are testSizeThatFitsSmoke and
    testFrameAndOriginPlacement. There's a test target in the Xcode projet of this Pull Request to play around with. Any ideas?

This is a pull request to support the Carthage dependency manager for Objective-C and Swift development. Unfortunately, doing this is causing a few issues: 1. **The buck build is now broken:** I have the impression we can't `#import <yoga/Yoga.h>` from a public header of the framework because the importing module does not know the `yoga` module. This was the case in `UIView+YogaKit.h`. Fixing this requires adding the C headers as standard headers of the framework and `#import <YogaKit/Yoga.h>` instead. Unfortunately, this has broken the buck build. I don't know enough about buck to fix this. Any ideas? 2. **The tests fail in Xcode**: Weirdly enough, two tests which run fine when run with `buck test //YogaKit:YogaKitTests --config cxx.default_platform=iphonesimulator-x86_64 --config cxx.cflags=-DTRAVIS_CI` fail when run inside an Xcode test target. Those are `testSizeThatFitsSmoke` and `testFrameAndOriginPlacement`. There's a test target in the Xcode projet of this Pull Request to play around with. Any ideas?
d16r commented 2016-12-27 07:22:12 -08:00 (Migrated from github.com)
Copy Link

Yeah, we were running into a similar problem internally. I'm going to take another look as soon as I have time, thanks for the PR!

Yeah, we were running into a similar problem internally. I'm going to take another look as soon as I have time, thanks for the PR!
guidomb commented 2016-12-27 10:03:56 -08:00 (Migrated from github.com)
Copy Link

@hartbit Remember that in order for the project to work with Carthage you need to make the framework scheme shared.

@hartbit Remember that in order for the project to work with Carthage you need to make the framework scheme shared.
hartbit commented 2016-12-27 10:05:55 -08:00 (Migrated from github.com)
Copy Link

@guidomb Yep, I saw I had forgotten that. But haven't fixed this yet because of the other roadblocks. And by the time they are cleared, I think I might as well do a PR to support Cocoapods at the same time. But that requires getting a good idea how to handle semver for the project.

@guidomb Yep, I saw I had forgotten that. But haven't fixed this yet because of the other roadblocks. And by the time they are cleared, I think I might as well do a PR to support Cocoapods at the same time. But that requires getting a good idea how to handle semver for the project.
guidomb commented 2016-12-27 10:09:40 -08:00 (Migrated from github.com)
Copy Link

@hartbit @dshahidehpour Yeah, what do you guys think is the best approach moving forward? I am happy to help, I can add CocoaPods support but I think it would be better for now to wait, as @hartbit said, for a semver policy and ideally for #289 to be merged. Regarding buck I don't have any experience with it.

@hartbit @dshahidehpour Yeah, what do you guys think is the best approach moving forward? I am happy to help, I can add CocoaPods support but I think it would be better for now to wait, as @hartbit said, for a semver policy and ideally for #289 to be merged. Regarding buck I don't have any experience with it.
guidomb commented 2016-12-27 10:18:04 -08:00 (Migrated from github.com)
Copy Link

@hartbit I was trying to test the Carthage build. I've created a single view blank iOS project, linked YogaKit.framework (which I've built using carthage build --no-skip-current after making YogaKit scheme shared) but when I try to use I get an error.

I've tried importing it from a Swift file using import YogaKit or from a bridging header using #import<YogaKit/YogaKit.h> neither is working. The first one fails with No such module 'YogaKit' and the second one with YogaKit/YogaKit.h file not found. Could this be related to the issue you pointed out in this PR's description about the buck build being broken?

@hartbit I was trying to test the Carthage build. I've created a single view blank iOS project, linked YogaKit.framework (which I've built using `carthage build --no-skip-current` after making YogaKit scheme shared) but when I try to use I get an error. I've tried importing it from a Swift file using `import YogaKit` or from a bridging header using `#import<YogaKit/YogaKit.h>` neither is working. The first one fails with `No such module 'YogaKit'` and the second one with `YogaKit/YogaKit.h file not found`. Could this be related to the issue you pointed out in this PR's description about the buck build being broken?
hartbit commented 2016-12-27 13:21:20 -08:00 (Migrated from github.com)
Copy Link

@guidomb I've updated the PR with the shared framework and its working for me. Did you setup the test project as detailed in Carthage's README?

@guidomb I've updated the PR with the shared framework and its working for me. Did you setup the test project as detailed in Carthage's README?
emilsjolander commented 2016-12-28 03:52:45 -08:00 (Migrated from github.com)
Copy Link

What is the status of this? 1. should be able to be fixed using header maps in xcode i think?

What is the status of this? 1. should be able to be fixed using header maps in xcode i think?
guidomb commented 2016-12-28 08:39:20 -08:00 (Migrated from github.com)
Copy Link

@hartbit Yes it worked. I must have done something wrong. I also added support for CocoaPods in #309 but for some reason I couldn't pick your fork as the PR's target.

@hartbit Yes it worked. I must have done something wrong. I also added support for CocoaPods in #309 but for some reason I couldn't pick your fork as the PR's target.
hartbit commented 2017-01-07 08:16:56 -08:00 (Migrated from github.com)
Copy Link

Closing so it can be implemented in #309 instead.

Closing so it can be implemented in #309 instead.

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
Algorithm
Build and Infrastructure
C#
C++ Enhancement
CLA Signed
Discussion
Documentation
Feature Request
Java
JavaScript
Merged
Question
Reverted
Shared with Meta
Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
YogaKit
YogaLayout ViewGroup
bug
dependencies
Pull requests that update a dependency file
duplicate
enhancement
fb-exported
fh:direct-merge-enabled
invalid
ruby
Pull requests that update Ruby code
wontfix
No Label
CLA Signed
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
DaddyFrosty
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#305
No description provided.
Delete Branch "carthage-support"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?