Skip to content

Use CocoaPods #3987

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Apr 3, 2020
Merged

Use CocoaPods #3987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
954fbe5
deps: Lock rn-fetch-blob@0.10.15; breaking change at 0.10.16.
Mar 28, 2020
feaf42f
ios build: Use angle-bracket form for third-party imports.
Apr 2, 2020
f475bc4
ios build: Remove system headers from HEADER_SEARCH_PATHS.
Apr 2, 2020
33f4b41
ios build: Start using CocoaPods again.
Mar 19, 2020
f253be0
ios deps: react-native-simple-toast ^0.1.1 -> ^1.0.0
Mar 28, 2020
8b4b784
ios build: Turn on build parallelization.
Mar 27, 2020
17f66e9
MainApplication.java [nfc]: Apply auto-"optimize" imports.
Mar 30, 2020
83ec41d
docs/build-run [nfc]: Add workaround for Android build failure with J…
Apr 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Suppress noisy generated files in diffs.
yarn.lock -diff
src/webview/js/generatedEs3.js -diff
ios/Podfile.lock -diff

# The flow-typed files that aren't pure generated noise have names with at
# least a major version number -- like `redux_v4.x.x.js`.
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ npm-debug.log
# Test files
/pirlo-cli

# CocoaPods (not currently used)
# CocoaPods
/ios/Pods

# Transifex, for a maintainer syncing translations up and down
Expand Down
6 changes: 3 additions & 3 deletions android/app/src/main/java/com/zulipmobile/MainApplication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
import android.app.Application;
import com.RNFetchBlob.RNFetchBlobPackage;
import com.facebook.react.ReactApplication;
import com.reactnativecommunity.asyncstorage.AsyncStoragePackage;
import com.reactnativecommunity.netinfo.NetInfoPackage;
import io.github.elyx0.reactnativedocumentpicker.DocumentPickerPackage;
import com.facebook.react.ReactNativeHost;
import com.facebook.react.ReactPackage;
import com.facebook.react.shell.MainReactPackage;
Expand All @@ -15,8 +12,11 @@
import com.learnium.RNDeviceInfo.RNDeviceInfo;
import com.nikolaiwarner.RNTextInputReset.RNTextInputResetPackage;
import com.reactnative.photoview.PhotoViewPackage;
import com.reactnativecommunity.asyncstorage.AsyncStoragePackage;
import com.reactnativecommunity.netinfo.NetInfoPackage;
import com.reactnativecommunity.webview.RNCWebViewPackage;
import com.zmxv.RNSound.RNSoundPackage;
import io.github.elyx0.reactnativedocumentpicker.DocumentPickerPackage;
import io.sentry.RNSentryPackage;
import java.util.Arrays;
import java.util.List;
Expand Down
90 changes: 87 additions & 3 deletions docs/howto/build-run.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ Native upstream on
You want the tab "React Native CLI Quickstart"; the "Expo CLI Quickstart" does
not apply.

If you're starting with iOS development, be sure to install CocoaPods,
as the guide instructs. After you've done that, run
`cd ios && pod install && cd ..`.
Comment on lines +77 to +79
Copy link
Member

@gnprice gnprice Apr 1, 2020

Choose a reason for hiding this comment

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

People working with iOS will also need to run this after each time they run yarn install (or plain yarn). There's no great place to document that -- really because we should automate it 🙂 as a near-term followup task -- but probably adding it to this paragraph is the best place.

I ran rg yarn docs/ to scan for other relevant spots. Probably docs/howto/ios-tips.md is a good spot to repeat this point.

And then the release instructions is the other spot I noticed... but actually, better than that is tools/ios. That already has a line that runs yarn; right after that is a good spot to add:

    ( cd ios && pod install )

(The parens mean these commands are run in a "subshell", a new shell that's a child process of the one running the rest of the script. That keeps the cd from mutating the main shell, which is slightly cleaner.)

Or oh hey, actually, better:

    pod install --project-directory=ios

Found that from pod install --help. The version with two cd commands is likely still the right one for these instructions, for their short lifetime before we automate it away, because I think it successfully optimizes for being easy to casually understand.

Copy link
Contributor Author

@chrisbobbe chrisbobbe Apr 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Mmm — it also sounds right, even if we do automate it, to simulate not having successfully run pod install, and add a troubleshooting entry for that; I can do that tomorrow.

Copy link
Contributor Author

@chrisbobbe chrisbobbe Apr 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Mmm — it also sounds right, even if we do automate it, to simulate not having successfully run pod install, and add a troubleshooting entry for that; I can do that tomorrow.

Mm, to save you the trouble of looking, I didn't get around to doing this today. But I expect there will be other changes I'll need to make from your review, so I can lump this one in with those. 🙂


Continue those instructions until you can run the Zulip Mobile app
with either `react-native run-android` or `react-native run-ios`.
You'll want to be able to use both an emulator and a physical device; but
Expand Down Expand Up @@ -199,7 +203,7 @@ packages in `node_modules/` to match the current `package.json`. You
might need to restart Metro / `react-native start` after doing so.


### Build failure: java.lang.UnsupportedClassVersionError
### Build failure: java.lang.UnsupportedClassVersionError, "Unsupported major.minor version 52.0"

When trying to build the Android app, you may see this error:

Expand All @@ -222,6 +226,76 @@ version number starts with "1.8", and with JDK 11, it starts with
[jdk-lts-roadmap]: https://adoptopenjdk.net/support.html#roadmap


### Build failure: java.lang.UnsupportedClassVersionError, "Unsupported class file major version 57"
Copy link
Member

@gnprice gnprice Apr 1, 2020

Choose a reason for hiding this comment

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

commit message nit:

Chris was somehow on Java 13, probably from an automatic update when
he upgraded to macOS 10.15 Catalina.

Generally I use the first person in commit messages, because they come with the author's name attached.

In docs and comments in the tree, I use the third person just like this, because they don't come with my name on them and so the first person would be confusing.

Copy link
Contributor Author

@chrisbobbe chrisbobbe Apr 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, OK — I had been avoiding first person singular at all costs, frequently favoring the academic "we" (I think we've discussed doing that?), but I agree that first person singular is helpful for something anecdotal like this.

Copy link
Member

@gnprice gnprice Apr 3, 2020

Choose a reason for hiding this comment

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

Yeah — I think "anecdotal" pretty well captures the difference between when I generally say "I" in a commit message vs. the academic "we".


When trying to build the Android app, you may see this error:

```
* Where:
Build file '/Users/jappleseed/dev/zulip-mobile/node_modules/@unimodules/react-native-adapter/android/build.gradle'

* What went wrong:
Could not compile build file '/Users/chrisbobbe/dev/zulip-mobile/node_modules/@unimodules/react-native-adapter/android/build.gradle'.
> startup failed:
General error during semantic analysis: Unsupported class file major version 57

java.lang.IllegalArgumentException: Unsupported class file major version 57
at groovyjarjarasm.asm.ClassReader.<init>(ClassReader.java:184)
at groovyjarjarasm.asm.ClassReader.<init>(ClassReader.java:166)
at groovyjarjarasm.asm.ClassReader.<init>(ClassReader.java:152)
at groovyjarjarasm.asm.ClassReader.<init>(ClassReader.java:273)
[...]
```

This can sometimes happen if you're using JDK 13 to invoke the build
command (e.g., when calling `react-native run-android`, or
`tools/test android`, or
`android/gradlew -p android :app:assembleDebug`). You can check the
version by running `java -version`. It seems that upgrading to
macOS 10.15 Catalina automatically upgrades Java to 13.

Somehow, JDK 8 is getting involved, and trying to use classfiles that
it doesn't understand because they were created with JDK 13. We're not
sure how JDK 8 is getting involved; it still seems to be involved
after locating and killing some Gradle-related processes with
`ps auxwww | grep gradle` that were using a JDK 8 installation that's
bundled with Android Studio (yours, if you have one, might be at
"/Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/jre/bin/java").

A solution, if you can't find why JDK 8 is getting involved and
prevent that, is to ensure those classfiles are also created with
JDK 8. (JDK 11 might also work; we haven't tested this.)

1. First, find a JDK installation. When you check a Java
version, note that the version number will start with "1.8" for
JDK 8.
- As noted above, you may have a JDK installation included in
Android Studio at a path like the one above. Check its version
by running, e.g.,
`"/Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/jre/bin/java" -version`.
- Otherwise, on Ubuntu or Debian, you can install it with
`sudo apt install openjdk-8-jdk`.
- In general, you can [download it from Oracle][jdk-8-oracle-dl].
2. Find a path to that installation to set `JAVA_HOME` to. (This is
used by `gradlew` to find the `java` command to run Gradle under.)
- If you used the installation included in Android Studio, take
that path minus the "/jre/bin/java" at the end, e.g.,
"/Applications/Android Studio.app/Contents/jre/jdk/Contents/Home".
- If you installed the Debian/Ubuntu package, then
`ls -l /usr/lib/jvm/` should list `java-8-openjdk-amd64` or
something similar to that.
3. Using the path from step 2, prefix your build command with
`JAVA_HOME=[that path] ` (for example,
`JAVA_HOME=... android/gradlew ...`), and see if that works.
4. If it does, you can run `export JAVA_HOME=[that path]` by itself,
so future build commands in the same terminal window will be able
to use it. You can also add that line to your `~/.bashrc` or
equivalent so you don't have to repeat it in new terminal windows.

You can read the debugging conversation around [here](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Android.20build.3A.20unimodules/near/844331).

[jdk-8-oracle-dl]: http://www.oracle.com/technetwork/java/javase/downloads/jdk8-downloads-2133151.html

### Build failure: Task 'installDebug' not found in project ':app'

When trying to build the Android app, you may see this error:
Expand All @@ -239,7 +313,7 @@ android` there, may give clearer error messages for debugging.
[rn-installation]: https://facebook.github.io/react-native/docs/getting-started.html


### Build failure in `react-native-screens` or at `new RNNotificationsPackage()`
### Build failure after running `react-native link`

When trying to build the Android app, if you get an error like this:

Expand Down Expand Up @@ -268,6 +342,15 @@ or like this:
[...]
```

or, when trying to build the iOS app, if you get several errors like
this:

```
Multiple commands produce '/Users/jappleseed/Library/Developer/Xcode/DerivedData/ZulipMobile-gdifomwwvfiwepckshlwoludvdct/Build/Products/Debug-iphonesimulator/ZulipMobile.app/Fontisto.ttf':
1) Target 'ZulipMobile' (project 'ZulipMobile') has copy command from '/Users/jappleseed/dev/zulip-mobile/node_modules/react-native-vector-icons/Fonts/Fontisto.ttf' to '/Users/jappleseed/Library/Developer/Xcode/DerivedData/ZulipMobile-gdifomwwvfiwepckshlwoludvdct/Build/Products/Debug-iphonesimulator/ZulipMobile.app/Fontisto.ttf'
2) That command depends on command in Target 'ZulipMobile' (project 'ZulipMobile'): script phase “[CP] Copy Pods Resources”
```

then check that `git status` shows you're running unmodified code from
our repo. These errors can be caused by the modifications made if you
run the command `react-native link`.
Expand All @@ -280,7 +363,8 @@ the project's version-control repo -- and not a build step. We don't
use it as a build step.

If you've run `react-native link`, you can discard the edits it made
(along with any other edits you've made) by running `git reset --hard`.
(along with any other edits you've made!) by running
`git reset --hard`.


### Build failure in `:app:buildDebugStaticWebviewAssets`
Expand Down
28 changes: 16 additions & 12 deletions docs/howto/ios-tips.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ this tab.

## Other commands

* `cd ios && pod install && cd ..` - if you've already run `yarn`,
installs native iOS dependencies to match the Podfile.lock

* `yarn ios-min` - runs in an iOS simulator in the minimally supported device
(currently iPhone 5S)

Expand Down Expand Up @@ -42,15 +45,16 @@ and choose it as your `Signing > Team` for both ZulipMobile and ZulipMobileTests
### Tips when running on your iOS device
When you change the BundleIdentifier and Team (required in order to run on a device),
it **will** modify your `.pbxproj` file, which you do **not** want unless you intend
to. For instance, if you linking a new dependency, your `.pbxproj` will be modified to
reflect the new changes.

If you are simply testing it on the iOS device, simply do not stage the said file to
be committed. On the other hand, if you are also adding a dependency, it is recommended
that you first `git commit` the dependency link modification itself, and then start
developing. This way, when you stage your intended changes, you can do a `git reset
path/to/.pbxproj` to discard any changes relating to the modification of the BundleIdentifier
and Team, and then continue to commit the rest of the files. When you prepare to push your
changes, you can just squash the initial commit with your later commits to retain a clean
commit history. This way, you won't have to deal with any merge conflicts or manual
deletion of the lines in your `.pbxproj` when you submit your code for a review.
to.

If you are simply testing it on the iOS device, simply do not stage
the said file to be committed.

If other changes to the `.pbxproj` file are needed (and they shouldn't
usually be, especially after we started managing our iOS dependencies
with CocoaPods), it's recommended that you put them in their own
commit, first, and leave the BundleIdentifier and Team changes
unstaged. Later, you can always [squash that commit with other
commits][fixing-commits], if appropriate.
Comment on lines 45 to +58
Copy link
Member

@gnprice gnprice Apr 3, 2020

Choose a reason for hiding this comment

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

Ah, good to fix this too, thanks!


[fixing-commits]: https://zulip.readthedocs.io/en/latest/git/fixing-commits.html
55 changes: 55 additions & 0 deletions ios/Podfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# This should match the iOS Deployment Target
# (project > ZulipMobile > Info in Xcode)
platform :ios, '10.3'

# This list will need to change for RN >= v0.60. If you're changing it
# for any other reason, first look at the example Podfile for v0.59 at
# https://github.com/facebook/react-native-website/blob/ded79d2cf/docs/integration-with-existing-apps.md#configuring-cocoapods-dependencies.
target 'ZulipMobile' do
pod 'React', :path => '../node_modules/react-native', :subspecs => [
'Core',
'CxxBridge',
'DevSupport',
'RCTText',
'RCTNetwork',
'RCTWebSocket',
'RCTAnimation',
'RCTImage',
'RCTPushNotification',
'ART',
'RCTAnimation',
'RCTCameraRoll',
'RCTActionSheet',
'RCTGeolocation',
'RCTLinkingIOS', # RCTLinking?
'RCTSettings',
'RCTVibration',
]
pod 'yoga', :path => '../node_modules/react-native/ReactCommon/yoga'
pod 'DoubleConversion', :podspec => '../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec'
pod 'glog', :podspec => '../node_modules/react-native/third-party-podspecs/glog.podspec'
pod 'Folly', :podspec => '../node_modules/react-native/third-party-podspecs/Folly.podspec'

# Pods we need that depend on React Native
pod 'RNDeviceInfo', :path => '../node_modules/react-native-device-info'
pod 'react-native-notifications', :path => '../node_modules/react-native-notifications'
pod 'RNSound', :path => '../node_modules/react-native-sound'
pod 'rn-fetch-blob', :path => '../node_modules/rn-fetch-blob'
pod 'react-native-photo-view', :path => '../node_modules/react-native-photo-view'
pod 'react-native-safe-area', path: '../node_modules/react-native-safe-area'
pod 'react-native-orientation', :path => '../node_modules/react-native-orientation'
pod 'react-native-image-picker', :path => '../node_modules/react-native-image-picker'
pod 'react-native-webview', :path => '../node_modules/react-native-webview'
pod 'RNVectorIcons', :path => '../node_modules/react-native-vector-icons'
pod 'RNSentry', :path => '../node_modules/@sentry/react-native'
pod 'react-native-safari-view', :path => '../node_modules/react-native-safari-view'
pod 'react-native-netinfo', :path => '../node_modules/@react-native-community/netinfo'
pod 'RNCAsyncStorage', :path => '../node_modules/@react-native-community/async-storage'
pod 'react-native-simple-toast', :path => '../node_modules/react-native-simple-toast'

target 'ZulipMobileTests' do
inherit! :search_paths
# Pods for testing
end

end
Loading