Page MenuHomePhabricator
Differential D5807

[native/rust_library] Add Tunnelbroker protobuf file compilation to the native client
ClosedPublic
Actions

Authored by • max on Dec 5 2022, 2:31 AM.
Tags
None
Referenced Files
F3491045: D5807.id20174.diff
Wed, Dec 18, 6:16 PM
F3491044: D5807.id19933.diff
Wed, Dec 18, 6:16 PM
F3491043: D5807.id19755.diff
Wed, Dec 18, 6:16 PM
F3491041: D5807.id19648.diff
Wed, Dec 18, 6:16 PM
F3491038: D5807.id19342.diff
Wed, Dec 18, 6:16 PM
F3491036: D5807.id19542.diff
Wed, Dec 18, 6:16 PM
F3491035: D5807.id19131.diff
Wed, Dec 18, 6:16 PM
F3491034: D5807.id20022.diff
Wed, Dec 18, 6:16 PM
View All Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
varun
atul
• jon
Commits
rCOMM5b7c49cb858a: [native/rust_library] Add Tunnelbroker protobuf file compilation to the native…
Summary

This diff introduces adding the Tunnelbroker's protobuf file compilation for the native client. The compilation step was added to the build step of the native/native_rust_library Rust library linked to the native by the CXX bridge.

Linear task: ENG-1333

Test Plan

Rust native library is successfully built.

Diff Detail

Repository
rCOMM Comm
Branch
tb-native-client-add-proto
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• max created this revision.Dec 5 2022, 2:31 AM
• max held this revision as a draft.
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptDec 5 2022, 2:31 AM
• max published this revision for review.Dec 5 2022, 2:35 AM
• max edited the summary of this revision. (Show Details)
• max edited the test plan for this revision. (Show Details)
• max added reviewers: varun, atul, • jon.
• max added inline comments.
native/native_rust_library/build.rs
14 ↗(On Diff #19130)

@varun Do we have an src/hello.c in the native Rust lib or this is something from the test code?

Harbormaster failed remote builds in B13961: Diff 19130!Dec 5 2022, 2:42 AM
• max planned changes to this revision.Dec 5 2022, 2:45 AM
• max updated this revision to Diff 19131.Dec 5 2022, 3:08 AM

Rebasing on the master.

• max planned changes to this revision.Dec 5 2022, 3:15 AM
Harbormaster failed remote builds in B13962: Diff 19131!Dec 5 2022, 3:20 AM
ashoat added a comment.Dec 5 2022, 6:04 AM

Is the CI failing because of the Protobuf version issue?

• max added a comment.Dec 5 2022, 6:23 AM
In D5807#173355, @ashoat wrote:

Is the CI failing because of the Protobuf version issue?

Yes, it's related to the ENG-2404

• max updated this revision to Diff 19342.Dec 14 2022, 3:18 AM

Rebasing on top of the D5861

• max planned changes to this revision.Dec 14 2022, 3:18 AM
• max added a parent revision: D5861: [Android] Add script to install protoc for CI.
Harbormaster failed remote builds in B14142: Diff 19342!Dec 14 2022, 3:29 AM
• max mentioned this in D5861: [Android] Add script to install protoc for CI.Dec 14 2022, 4:05 AM
• max updated this revision to Diff 19542.Dec 19 2022, 3:44 AM

Rebasing on the stack changes.

• max planned changes to this revision.Dec 19 2022, 3:44 AM
Harbormaster failed remote builds in B14303: Diff 19542!Dec 19 2022, 3:55 AM
• max removed a parent revision: D5861: [Android] Add script to install protoc for CI.Dec 19 2022, 9:01 AM
• max updated this revision to Diff 19648.Dec 19 2022, 9:03 AM

Rebasing on master changes.

Harbormaster failed remote builds in B14399: Diff 19648!Dec 19 2022, 9:26 AM
• max planned changes to this revision.Dec 19 2022, 9:31 AM
• max updated this revision to Diff 19755.Dec 20 2022, 5:52 AM

Rebasing on the master changes.

• max planned changes to this revision.Dec 20 2022, 5:52 AM
Harbormaster failed remote builds in B14498: Diff 19755!Dec 20 2022, 6:03 AM
• max updated this revision to Diff 19933.Dec 21 2022, 3:49 AM

Rebasing on master changes.

• max planned changes to this revision.Dec 21 2022, 3:49 AM
Harbormaster completed remote builds in B14613: Diff 19933.Dec 21 2022, 4:03 AM
• max updated this revision to Diff 20022.Dec 22 2022, 6:36 AM

Rebasing on master.

• max planned changes to this revision.Dec 22 2022, 6:36 AM
• max added a child revision: D5997: [native/rust_library] Adding of the Tunnelbroker Tonic client initialization.Dec 22 2022, 6:47 AM
Harbormaster completed remote builds in B14682: Diff 20022.Dec 22 2022, 6:50 AM
• max requested review of this revision.Dec 22 2022, 6:51 AM
• max retitled this revision from [native] Add Tunnelbroker protobuf file compilation to the native client to [native/rust_library] Add Tunnelbroker protobuf file compilation to the native client.Dec 22 2022, 8:20 AM
varun added inline comments.Dec 22 2022, 10:38 PM
native/native_rust_library/build.rs
14 ↗(On Diff #19130)

ah, this was from my local testing... you can remove it, i don't think we need a rerun-if-changed condition

varun accepted this revision.Dec 22 2022, 10:39 PM
varun added inline comments.
native/native_rust_library/build.rs
3 ↗(On Diff #20022)

good call

9 ↗(On Diff #20022)

why do we need to include this folder?

This revision is now accepted and ready to land.Dec 22 2022, 10:39 PM
• max added a child revision: D6014: [native/rust_library] Removing of testing `hello.c` and increase C++ standard to 17 in `build.rs`.Dec 23 2022, 9:31 AM
• max marked 3 inline comments as done.Dec 23 2022, 9:56 AM
• max added inline comments.
native/native_rust_library/build.rs
9 ↗(On Diff #20022)

why do we need to include this folder?

This is how the compile(...) works. We should provide a directory where the protos reside.

14 ↗(On Diff #19130)

ah, this was from my local testing... you can remove it, i don't think we need a rerun-if-changed condition

Got it, I've removed it in the D6014.

Closed by commit rCOMM5b7c49cb858a: [native/rust_library] Add Tunnelbroker protobuf file compilation to the native… (authored by • max). · Explain WhyDec 27 2022, 6:17 AM
This revision was automatically updated to reflect the committed changes.
• max marked 2 inline comments as done.
• max added a commit: rCOMM5b7c49cb858a: [native/rust_library] Add Tunnelbroker protobuf file compilation to the native….

Revision Contents
Changeset List

PathSize
native/
native_rust_library/
10 lines

Diff 19648

View Options

native/native_rust_library/build.rs

Loading...