[Android] Add script to install protoc for CI
ClosedPublic
Actions

Authored by • jon on Dec 13 2022, 7:16 AM.

Details

Reviewers

atul
• max
varun

Commits

rCOMM1f2687430c4a: [Android] Add script to install protoc for CI

Summary

Add protobuf install script to make installation of the protoc
command easy for CI.

The intent is to use a more recent version of protobuf which doesn't fail
immediately when "optional" is present in a protobuf file. Ubuntu still uses
3.12 which deprecated optional, but 3.15 added usage of optional back.

https://linear.app/comm/issue/ENG-2404

Test Plan

$ docker run -it --rm -v $PWD:/comm reactnativecommunity/react-native-android:latest /bin/bash

# apt update
# apt install -y autoconf libtool build-essential cmake git libgtest-dev libssl-dev zlib1g-dev
# ./comm/native/android/scripts/install_protobuf.sh
# protoc --version
libprotoc 3.15.8

Example run (just asserts that protobuf was installed correctly, not that it was used):
https://buildkite.com/comm/android-ci-plus-protoc-install/builds/1#01850bfd-2572-4257-b2fe-afbd13f48d04

Diff Detail

Repository

rCOMM Comm

Branch

jonringer/android-protoc

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jon created this revision.Dec 13 2022, 7:16 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptDec 13 2022, 7:16 AM

Harbormaster completed remote builds in B14120: Diff 19320.Dec 13 2022, 7:28 AM

• jon requested review of this revision.Dec 13 2022, 7:28 AM

• max mentioned this in D5807: [native/rust_library] Add Tunnelbroker protobuf file compilation to the native client.Dec 14 2022, 3:18 AM

• max added a child revision: D5807: [native/rust_library] Add Tunnelbroker protobuf file compilation to the native client.Dec 14 2022, 3:19 AM

I've rebased D5807 on top of this diff changes and it seems it doesn't solve the issue.

This revision now requires changes to proceed.Dec 14 2022, 4:05 AM

Well @max I think you would still need to call this script somewhere. @jon where should this script be called?

In D5861#175531, @ashoat wrote:

Well @max I think you would still need to call this script somewhere.

🤦‍♂️ right! Missed that we don't have any diffs in the stack that call this script.

@jon where should this script be called?

I assume we should call it on the CI machine.

Well @max I think you would still need to call this script somewhere. @jon where should this script be called?

Correct, it would be be part of CI, as in the example: https://buildkite.com/comm/android-ci-plus-protoc-install/builds/1#01850bfd-2572-4257-b2fe-afbd13f48d04

I've rebased D5807 on top of this diff changes and it seems it doesn't solve the issue.

You would need to remove the apt install -y protobuf-compiler protobuf-compiler-grpc line, 3.12 is still getting installed, and for whatever reason, that's the protoc which is getting picked up first

root@d17bf368eefd:/comm/native# cd native_rust_library/
root@d17bf368eefd:/comm/native/native_rust_library# cargo build
bash: cargo: command not found
root@d17bf368eefd:/comm/native/native_rust_library# curl https://sh.rustup.rs -sSf | sh -s -- -y
...
root@d17bf368eefd:/comm/native/native_rust_library# source /root/.cargo/env
root@d17bf368eefd:/comm/native/native_rust_library# cargo build
...
   Compiling native_rust_library v0.1.0 (/comm/native/native_rust_library)
   Compiling h2 v0.3.14
   Compiling tower v0.4.13
   Compiling tower-http v0.3.4
   Compiling hyper v0.14.20
   Compiling axum v0.5.16
   Compiling hyper-timeout v0.4.1
   Compiling tonic v0.8.1
    Finished dev [unoptimized + debuginfo] target(s) in 2m 35s
root@d17bf368eefd:/comm/native/native_rust_library#

🤦‍♂️ right! Missed that we don't have any diffs in the stack that call this script.

Current buildkite CI doesn't use the yaml. Will be changed soon

• jon added a child revision: D5872: [CI] Update android.yml to use protobuf 3.15.Dec 14 2022, 2:26 PM

🤦‍♂️ right! Missed that we don't have any diffs in the stack that call this script.

Now addressed in D5872, now that it's up-to-date. Still won't be "ratified" for a while since it will require people to rebase to not have broken CI gates.

• jon requested review of this revision.Dec 14 2022, 2:28 PM

Thanks, @jon!

This revision is now accepted and ready to land.Dec 19 2022, 4:07 AM

Closed by commit rCOMM1f2687430c4a: [Android] Add script to install protoc for CI (authored by • jon). · Explain WhyDec 19 2022, 6:34 AM

This revision was automatically updated to reflect the committed changes.

• jon added a commit: rCOMM1f2687430c4a: [Android] Add script to install protoc for CI.

• max removed a child revision: D5807: [native/rust_library] Add Tunnelbroker protobuf file compilation to the native client.Dec 19 2022, 9:01 AM