Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Support Buck build#2849

Merged
Adlai-Holler merged 4 commits intofacebookarchive:masterfrom
nguyenhuy:buck_final
Jan 4, 2017
Merged

Support Buck build#2849
Adlai-Holler merged 4 commits intofacebookarchive:masterfrom
nguyenhuy:buck_final

Conversation

@nguyenhuy
Copy link
Copy Markdown
Contributor

@nguyenhuy nguyenhuy commented Dec 31, 2016

First attempt to support Buck 🐇 🏎 💨

How to use:

# Run this once on the root directory to resolve dependencies (i.e PINRemoteImage, PINCache, OCMock, etc)
# and populate their BUCK files (more on this later)
pod install

buck build lib
# Same as above because 'lib' is an alias for //:AsyncDisplayKit
buck build //:AsyncDisplayKit
buck build //:AsyncDisplayKit-Core

buck test

Dependency resolution:

  • Currently I build dependencies from source files downloaded by CocoaPods.
  • There is a post_install hook added to the Podfile in root dir. The hook copies all BUCK templates needed to build the pods. Our main BUCK file (in the root dir) has hardcoded paths that point to these dependent BUCK targets. That means pod install needs to be run at least once.
  • Buck can already build static and shared prebuilt frameworks (prebuilt_apple_framework). So we can rely on Carthage for dependency resolution if needed to. I used it to build FLAnimatedImage for a while, but decided to abandon this approach because prebuilt_apple_framework is still under development and poorly documented.
  • A BUCK template has just enough information to build source files of a downloaded Pod. That means it either is an exact copy of the BUCK file from original repo (in case of PINCache), or is a stripped down version of the original BUCK file (in case of PINRemoteImage if we use pod 'PINRemoteImage/iOS', ' 3.0.0-beta.7' and pod 'PINRemoteImage/PINCache' in Podfile), or only lives in our repo (in case of FBSnapshotTestCase, OCMock, etc).

Remaining/followed-up tasks:

  • BUCK files for all examples
  • HTTP cache for build artifacts 🏎 💨
  • Benchmarks
  • CI integration

This PR depends on #2844.

Let me know if you have any questions or suggestions!

@nguyenhuy
Copy link
Copy Markdown
Contributor Author

Here are screenshots of 2 consecutive test passes (bct is my alias for buck clean && rm -rf buck-out && buck test). The first clean build and test took 52.8 seconds in total: 22.4s for building (without HTTP cache) and 30.2s for testing. The second pass took 700ms because everything is cached, including test results!

screen shot 2016-12-31 at 11 11 31 pm

screen shot 2016-12-31 at 11 12 08 pm

screen shot 2016-12-31 at 11 12 38 pm

screen shot 2016-12-31 at 11 12 51 pm

Copy link
Copy Markdown
Contributor

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know much about Buck but this looks beautiful! Faster builds and more smarter build system = 👍I support landing this whenever you feel it appropriate.

I would be good if possible to separate out the Buck support from the ASVideoNode delegate change, no?

@nguyenhuy
Copy link
Copy Markdown
Contributor Author

nguyenhuy commented Jan 2, 2017

@Adlai-Holler: Thanks for the review. The ASVideoNode delegate change was merged in #2844. I've rebased this PR to remove that commit. Let's wait for another day so others have a chance to raise any questions/concerns. Also, I can't land this myself (no GitHub permission) :)

@Adlai-Holler
Copy link
Copy Markdown
Contributor

👍

@Adlai-Holler
Copy link
Copy Markdown
Contributor

Here we go!

@Adlai-Holler Adlai-Holler merged commit be6faa1 into facebookarchive:master Jan 4, 2017
@nguyenhuy nguyenhuy deleted the buck_final branch January 4, 2017 22:43
@nguyenhuy
Copy link
Copy Markdown
Contributor Author

Very excited. Thanks!

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.

3 participants