Details

Reviewers

• jon
varun
marcin
tomek

Commits

rCOMM478edf15de16: [services] Tunnelbroker - Creating `build.rs` file for building and linking C++…

Summary

This diff is a part of the flat diffs stack to transform the current Tunnelbroker project into a Rust app by calling the current C++ codebase from Rust. The related transition task is ENG-1072.

This diff creates a Cargo build.rs file which builds and links the current C++ codebase to the Rust application.

Related Linear task and discussion: ENG-2093

CI Notice:
The CI will fail on this and further diffs until the D5461 where the changes to CI build commands are made.
This stack will be landed all in one to prevent CI from failing on diffs out of this stack.

Test Plan

1. CI Testing:
These diff changes are included as a part of the stack in the later D5461 diff, where the changes to CI build commands are made. The CI succeded on the D5461 which reflects that this changes are passing the build.

2. Manually testing:
These diff changes can be tested in the middle of the stack at D5436. To test it patch to D5436 using arc patch D5436, run nix develop. Go to tunnelbroker service directory services/tunnelbroker and run the Cargo building process cargo build or cargo run.
The Rust application will be successfully built.

Diff Detail

Repository

rCOMM Comm

Branch

add-build-rs

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.Oct 20 2022, 7:39 AM

• max held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptOct 20 2022, 7:39 AM

Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald Transcript

• max added a parent revision: D5432: [services] Tunnelbroker - Creating `main.rs` entry point file for Rust.Oct 20 2022, 7:40 AM

• max added a child revision: D5435: [services] Tunnelbroker - Updating `.gitignore` on the Tunnelbroker transition to Rust app.

Harbormaster failed remote builds in B12927: Diff 17718!Oct 20 2022, 7:44 AM

• max edited the summary of this revision. (Show Details)Oct 20 2022, 8:11 AM

• max edited the test plan for this revision. (Show Details)

• max added reviewers: varun, marcin. • max added 1 blocking reviewer(s): • jon.

• max edited the summary of this revision. (Show Details)Oct 20 2022, 8:19 AM

• max published this revision for review.Oct 20 2022, 8:26 AM

• max edited the summary of this revision. (Show Details)

Adding of Tunnelbroker[.h,.cpp] to the if-changed scope.

Harbormaster failed remote builds in B12936: Diff 17735!Oct 20 2022, 9:03 AM

• max added inline comments.Oct 20 2022, 9:09 AM

services/tunnelbroker/build.rs
16 ↗	(On Diff #17735)	We are using this function instead of passing the string path directly because the `CFG.exported_header_dirs` wants an absolute path and we are making it here from the relative.
30 ↗	(On Diff #17735)	We are using the `get_cpp_sources` function to traverse the directory and get all the `.cpp` inside to be built.

Adding of -w flag to the C++ building process to suppress flooding of warnings.

Harbormaster failed remote builds in B12965: Diff 17808!Oct 21 2022, 8:40 AM

Removing of unused PathBuff.

Harbormaster failed remote builds in B12966: Diff 17809!Oct 21 2022, 8:49 AM

Fixing include path to meet the Docker, adding of linking double-conversion and gflags for the building in Ubuntu Docker container.

Harbormaster failed remote builds in B12969: Diff 17815!Oct 21 2022, 11:00 AM

Updating on the stack reordering.

Harbormaster failed remote builds in B12984: Diff 17842!Oct 24 2022, 3:25 AM

Update on a stack changes.

• max edited the summary of this revision. (Show Details)Oct 24 2022, 9:30 AM

• max edited the test plan for this revision. (Show Details)

Harbormaster failed remote builds in B13006: Diff 17865!Oct 24 2022, 9:30 AM

• max mentioned this in D5436: [services] Tunnelbroker - Creating Cargo config file for Tunnelbroker as a Rust app.Oct 26 2022, 2:48 AM

• jon requested changes to this revision.Oct 26 2022, 2:01 PM

• jon added inline comments.

services/tunnelbroker/build.rs
8–10 ↗	(On Diff #17865)	feel like this is a bit hard to read, and we should avoid explicit unwraps
11 ↗	(On Diff #17865)	`String::from` and `.to_string()` should be redundant
17 ↗	(On Diff #17865)	`unwrap` -> `expect` please.

This revision now requires changes to proceed.Oct 26 2022, 2:01 PM

• max planned changes to this revision.Oct 27 2022, 3:56 AM

Small changes addressing comments.

Harbormaster failed remote builds in B13128: Diff 18022!Nov 2 2022, 8:40 AM

• max added inline comments.Nov 2 2022, 8:50 AM

services/tunnelbroker/build.rs
8–10 ↗	(On Diff #17865)	feel like this is a bit hard to read, and we should avoid explicit unwraps I don't mind changing it if you find it hard to read. Changed.
11 ↗	(On Diff #17865)	`String::from` and `.to_string()` should be redundant Good catch, thanks!

• jon accepted this revision.Nov 3 2022, 9:45 AM

This revision is now accepted and ready to land.Nov 3 2022, 9:45 AM

• max added a reviewer: tomek.Nov 3 2022, 10:24 AM

This revision now requires review to proceed.Nov 3 2022, 10:24 AM

tomek accepted this revision.Nov 4 2022, 7:27 AM

tomek added inline comments.

services/tunnelbroker/build.rs
7 ↗	(On Diff #18022)	I guess we could use `vec!` macro
24–28 ↗	(On Diff #18022)	Is it possible to provide a wildcard - any directory under `src`?

This revision is now accepted and ready to land.Nov 4 2022, 7:27 AM

Changing to use vec! macro.

Harbormaster failed remote builds in B13200: Diff 18107!Nov 7 2022, 5:23 AM

• max marked 2 inline comments as done.Nov 7 2022, 5:23 AM

• max added inline comments.

services/tunnelbroker/build.rs
7 ↗	(On Diff #18022)	I guess we could use `vec!` macro Yes, we can! I've changed it.
24–28 ↗	(On Diff #18022)	Is it possible to provide a wildcard - any directory under `src`? Unfortunately no, it fails if we try to use any of the regexes like `src/libcpp/src/*/`: thread 'main' panicked at 'Error getting full import path