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

Support Static Libraries for Carthage #1940

Merged
merged 5 commits into from
Apr 21, 2019
Merged

Support Static Libraries for Carthage #1940

merged 5 commits into from
Apr 21, 2019

Conversation

freak4pc
Copy link
Member

Following the discussion in #1799, this seems to work now. The only downside here is that the "validate-playground" script fails, and I'm not sure if there's an easy way to fix it here. Any suggestions welcomed.

cc/ @bobgodwinx @mfcollins3

@freak4pc freak4pc added this to the RxSwift 5 milestone Apr 18, 2019
@freak4pc freak4pc requested a review from kzaher April 18, 2019 11:35
@freak4pc freak4pc self-assigned this Apr 18, 2019
@mfcollins3
Copy link
Contributor

@freak4pc It looks like the swift command is trying to load RxSwift as if it's a dynamic framework and is failing. It's interpreting the code and trying to use dyld to dynamically make the calls to the framework's methods. I switched to using swiftc to build the program and link with the RxSwift framework, then run the program and that works:

. scripts/common.sh

PLAYGROUND_CONFIGURATIONS=(Release)

# make sure macOS builds
for scheme in "RxSwift"
do
  for configuration in ${PLAYGROUND_CONFIGURATIONS[@]}
  do
    PAGES_PATH=${BUILD_DIRECTORY}/Build/Products/${configuration}/all-playground-pages.swift
    rx ${scheme} ${configuration} "" build
    cat Rx.playground/Sources/*.swift Rx.playground/Pages/**/*.swift > ${PAGES_PATH}
    swiftc -v -D NOT_IN_PLAYGROUND -target x86_64-apple-macosx10.10 -F ${BUILD_DIRECTORY}/Build/Products/${configuration} -framework RxSwift ${PAGES_PATH}   
    ./all-playground-pages
  done
done

I added the -framework RxSwift to the command line for swiftc. I then added the call to execute the all-playground-pages program. I'm assuming that you're just looking to make sure that the playgrounds output without failing, correct?

@kzaher
Copy link
Member

kzaher commented Apr 18, 2019

@freak4pc I think that besides making out tests pass we should figure out how to make playgrounds work with these static libraries.

I think that we can add a dynamic framework called Playgrounds in RxExample which imports and reexports all static frameworks and use that for playgrounds.

I'm thinking of putting it to RxExample because otherwise Carthage will build it.

@freak4pc
Copy link
Member Author

@kzaher Absolutely. Actually @mfcollins3's solution seems to work locally for me, which is awesome! I didn't think it would be that simple to fix. Let's wait to see if CI agrees with this :)

@freak4pc
Copy link
Member Author

@kzaher this seems good to go.

I think maybe we should update the README to mention we support Static Libraries with Carthage 0.30.1 and up.

@freak4pc
Copy link
Member Author

I've updated the README.
I added a CHANGELOG entry in the mani CHANGELOG PR - #1933

@kzaher
Copy link
Member

kzaher commented Apr 19, 2019

Hi @freak4pc ,

It looks to me like playgrounds are still broken.

@freak4pc
Copy link
Member Author

@kzaher Seems to be running just fine ... what am I missing ?

https://imgur.com/hKwRg7R

@kzaher
Copy link
Member

kzaher commented Apr 20, 2019

@freak4pc The purpose of playground tests is not for the CI to pass, but to make sure that we didn't break playgrounds accidentally :)

The fact that CI passes is great, but users aren't able to use them because playgrounds don't use the command that you are using.

Screen Shot 2019-04-20 at 12 13 40 PM

@freak4pc
Copy link
Member Author

Ah, I missed that. I tried playing with this a bit in cea92be but couldn't get it to work with a wrapper dynamic framework.

I won't have much time to dig into this in the following days, so if anyone can chime in and has any thoughts, that'd be great.

@kzaher
Copy link
Member

kzaher commented Apr 20, 2019

For some reason I also couldn't get it to work with a dynamic framework, but that's the only plausible idea I have.

☹️

@kzaher
Copy link
Member

kzaher commented Apr 20, 2019

I can't even get the example app to work. I think that if we get RxExample to work, playgrounds will probably work :)

-all_load is useless as far as I can tell. Xcode 10.2.1 (10E1001)

@mfcollins3 Any ideas?

This is extremely annoying.

@freak4pc
Copy link
Member Author

I was actually able to get the example project itself to work. Already heading out today so will try to share tomorrow.

@mfcollins3
Copy link
Contributor

@kzaher I did a fresh clone of the repo and checked out the static lib branch. In RxExample-iOS, I removed the embedded frameworks (which also removed the linked frameworks). I added back RxSwift and RxCocoa to the linked frameworks (they don't need to be embedded since they're now linked into the executable). I also changed the Other Linker Flags to -ObjC. I couldn't figure out what the -objc_loadall flag did, and once I removed it, it didn't seem to be necessary. I think that -ObjC is necessary when using RxCocoa because of the Objective-C code used for the delegate proxies. I didn't need the -all_load flag. This worked for RxExample-OSX as well.

For RxPlaygrounds, I just added RxSwift, RxCocoa, and RxRelay to the linked libraries list on the General tab. Once I did that, they playgrounds worked. I saw that -all_load and -ObjC were already specified in the Other Linker Flags build setting.

After that, it looks like you just need to update scripts/validate-playgrounds.sh to build RxPlaygrounds before attempting to build/run the playgrounds program. I'm not the best bash person, but here's the modified script that I got to work:

. scripts/common.sh

PLAYGROUND_CONFIGURATIONS=(Release)

# make sure macOS builds
for configuration in ${PLAYGROUND_CONFIGURATIONS[@]}
do
  for scheme in "RxSwift"
  do
    rx ${scheme} ${configuration} "" build
  done

  rx RxPlaygrounds ${configuration} "" build
  PAGES_PATH=${BUILD_DIRECTORY}/Build/Products/${configuration}/all-playground-pages.swift
  cat Rx.playground/Sources/*.swift Rx.playground/Pages/**/*.swift > ${PAGES_PATH}
  swiftc -v -D NOT_IN_PLAYGROUND -target x86_64-apple-macosx10.10 -F ${BUILD_DIRECTORY}/Build/Products/${configuration} -framework RxSwift ${PAGES_PATH}   
  ./all-playground-pages
done

I was trying to get RxPlaygrounds to work in the for scheme in... loop, but it wouldn't work for me for some reason.

I put these in a PR for the staticlib branch. See #1942.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants