[services] Tunnelbroker - Creating `main.rs` entry point file for Rust
ClosedPublic
Actions

Authored by • max on Oct 20 2022, 7:38 AM.

Details

Reviewers

• jon
varun
marcin
tomek

Commits

rCOMM7c253be3d099: [services] Tunnelbroker - Creating `main.rs` entry point file for Rust

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 Rust application entry point file main.rs and calls the initialization function (D5429) from the current C++ codebase.

Related Linear task: ENG-2092

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-main-rs

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

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

• max held this revision as a draft.

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

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

• max added a parent revision: D5431: [services] Tunnelbroker - Removing of deprecated C++ files.Oct 20 2022, 7:38 AM

• max added a child revision: D5434: [services] Tunnelbroker - Creating `build.rs` file for building and linking C++ dependencies.

Harbormaster failed remote builds in B12925: Diff 17716!Oct 20 2022, 7:43 AM

• max mentioned this in D5429: [services] Tunnelbroker - Initialization function exposed to Rust.Oct 20 2022, 8:00 AM

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

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

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

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

• jon requested changes to this revision.Oct 21 2022, 9:26 AM

• jon added inline comments.

services/tunnelbroker/src/main.rs
4 ↗	(On Diff #17716)	I guess this is highly unrecommended: https://doc.rust-lang.org/std/macro.include.html Leads to potential footguns around what is "defined" through it

This revision now requires changes to proceed.Oct 21 2022, 9:26 AM

• max requested review of this revision.Oct 21 2022, 10:19 AM

• max marked an inline comment as done.

• max added inline comments.

services/tunnelbroker/src/main.rs
4 ↗	(On Diff #17716)	I guess this is highly unrecommended: https://doc.rust-lang.org/std/macro.include.html Leads to potential footguns around what is "defined" through it We should include the C++ codebase exposed functions to the Rust app. In a regular Rust app we shouldn't do this, but in our scenario, seems that this is the only way to include headers according to the cxx documentation.

• max edited parent revisions, added: D5430: [services] Tunnelbroker - Moving CMakeLists file to use for gTest only; removed: D5431: [services] Tunnelbroker - Removing of deprecated C++ files.Oct 24 2022, 3:07 AM

Update on a stack reordering.

Harbormaster failed remote builds in B12983: Diff 17841!Oct 24 2022, 3:22 AM

Update on the stack changes.

Harbormaster failed remote builds in B13005: Diff 17864!Oct 24 2022, 9:28 AM

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

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

• max added 1 blocking reviewer(s): • jon.

don't feel like blocking this work, since we have the goal of transitioning away from C++ eventually. So the issue of include! hygiene might not be relevant in the future.

This revision is now accepted and ready to land.Oct 26 2022, 1:51 PM

• max added a reviewer: tomek.Oct 27 2022, 2:32 AM

This revision now requires review to proceed.Oct 27 2022, 2:32 AM

tomek accepted this revision.Oct 27 2022, 4:37 AM

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

tomek added inline comments.Oct 27 2022, 4:38 AM

services/tunnelbroker/src/main.rs
10 ↗	(On Diff #17864)	I would rather mane a function a a verb `initialize`

Rebasing on parent changes.

Harbormaster failed remote builds in B13198: Diff 18105!Nov 7 2022, 4:54 AM

• max marked an inline comment as done.Nov 7 2022, 4:54 AM

• max added inline comments.

services/tunnelbroker/src/main.rs
10 ↗	(On Diff #17864)	I would rather mane a function a a verb `initialize` Looks ok to me to change it. Done in https://phab.comm.dev/D5429#164086.

• max mentioned this in rCOMMe8d2e66a98bc: [services] Tunnelbroker - Initialization function exposed to Rust.Nov 7 2022, 5:52 AM

Rebasing.

This revision was landed with ongoing or failed builds.Nov 7 2022, 6:02 AM

Closed by commit rCOMM7c253be3d099: [services] Tunnelbroker - Creating `main.rs` entry point file for Rust (authored by • max). · Explain Why

This revision was automatically updated to reflect the committed changes.

• max added a commit: rCOMM7c253be3d099: [services] Tunnelbroker - Creating `main.rs` entry point file for Rust.

Harbormaster failed remote builds in B13205: Diff 18116!Nov 7 2022, 6:02 AM

Revision Contents
Changeset List

Path

Size

services/

tunnelbroker/

src/

main.rs

11 lines

Diff 18105

View Options

services/tunnelbroker/src/main.rs

# Motivation # Motivation

We use Nix to package all of the dependencies for our dev environment. It does several things for us: We use Nix to package all of the dependencies for our dev environment. It does several things for us:

- Enables new devs on macOS or Linux to set up our dev environment with just one command - Enables new devs on macOS or Linux to set up our dev environment with just one command

- Makes sure that everybody on the team is using the same versions of all the tools - Makes sure that everybody on the team is using the same versions of all the tools

- Allows us to isolate development dependencies from affecting the host system - Allows us to isolate development dependencies from affecting the host system

For how Nix package management works, please refer to the official [how Nix works guide](https://nixos.org/guides/how-nix-works.html). For how Nix package management works, please refer to the official [how Nix works guide](https://nixos.org/guides/how-nix-works.html).

# Supported workflows # Supported workflows

| ------------------------------- | --------------- | | ------------------------------- | --------------- |

| `keyserver` (Node.js) | ✅ | | `keyserver` (Node.js) | ✅ **†** |

| `web` (Webpack) | ✅ | | `web` (Webpack) | ✅ **†** |

| `native` iOS (React Native) | ❌ **†** | | `native` iOS (React Native) | ✅ **†** |

| `native` Android (React Native) | ❌ **†** | | `native` Android (React Native) | ✅ **†** |

aboshUnsubmitted

Done

At this point, nearly every workflow requires additional steps. Maybe we can think about a way to make this seem better in the future -- users should know before reading the footnote that Nix isn't going to do everything for them. But, it will do most of the work for them, and anything else will be smaller.

While the user might still know this, the footnote isn't really helping right now. It makes it seem like we told them Nix will do a lot of things, and then later slid in that they will have to do more things.

Currently, the footnote for these sections has a lot of examples for "additional steps." Plus, "additional steps" is broad and could mean any amount of steps, especially to a user who has no familiarity with the dev environment.

So we should think about that, since most of the time footnotes should be used sparingly. If they end up affecting a large portion of a section, we should just make it information the user knows outside of just the footnote. But this can be a separate issue.

abosh: At this point, nearly every workflow requires additional steps. Maybe we can think about a way…

jonAuthorUnsubmitted

Done

I originally wrote this before the prerequisite section existed. I can just reference that section now.

Probably be a better user experience anyway.

Would you like me to just reference the section? ... I went ahead and just referenced it anyway.

jon: I originally wrote this before the prerequisite section existed. I can just reference that…

aboshUnsubmitted

Done

Perfect! This is much better and explicitly links a section that users can find easily.

abosh: Perfect! This is much better and explicitly links a section that users can find easily.

| C++ services | ❌ **\*** | | C++ services | ❌ **\*** |

| Rust services | ✅ | | Rust services | ✅ |

**†** Workflow requires additional undocumented steps after Nix installation (e.g. installing XCode, XCode profile, Android Studio, or Android NDK). **†** Workflow requires some additional steps after Nix installation (e.g. installing XCode, XCode profile, optional browser extensions, flipper, `idb`, `reactotron`, Android Studio, or Android NDK).

aboshUnsubmitted

Done

| Rust services | ✅ |

- **†** Workflow requires some additional steps after Nix installation (e.g. installing XCode, XCode profile, optional browser extensions, flipper, `idb`, `reactotron`, Android Studio, or Android NDK).

+ **†** Workflow requires some additional steps after Nix installation (e.g. installing XCode, XCode profile, browser extensions, Flipper, `idb`, `reactotron`, Android Studio, or Android NDK).

**\*** Workflow requires documentation; and it requires the RabbitMQ and AWS/Localstack services to be available.

I don't think we need the "optional," that can be made clearer in the actual documentation for React/Redux Dev Tools. Here, in a footnote, it's not as necessary.

abosh: I don't think we need the "optional," that can be made clearer in the actual documentation for…

jonAuthorUnsubmitted

Done

Right, but it was never clear in the original documentation what is optional or not. Same with Flipper and idb.

I can also just remove the statement and replace it with a link to prerequisite section

jon: Right, but it was never clear in the original documentation what is optional or not. Same with…

jonAuthorUnsubmitted

Done

Removed this comment in favor of just referencing the prerequisite section.

jon: Removed this comment in favor of just referencing the prerequisite section.

aboshUnsubmitted

Done

| Rust services | ✅ |

+ **†** Workflow requires some additional steps after Nix installation (e.g. installing XCode, XCode profile, optional browser extensions, flipper, `idb`, `reactotron`, Android Studio, or the Android NDK).

**\*** Workflow requires documentation; and it requires the RabbitMQ and AWS/Localstack services to be available.

Should this be "the" Android NDK?

abosh: Should this be "the" Android NDK?

jonAuthorUnsubmitted

Done

If I switch Studio and NDK, I think it would sounds odd to me either way reactotron, the Android NDK, or Android Studio.

I can add it.

jon: If I switch Studio and NDK, I think it would sounds odd to me either way `reactotron, the…

**\*** Workflow requires RabbitMQ and AWS/Localstack to be available. **\*** Workflow requires documentation; and it requires the RabbitMQ and AWS/Localstack services to be available.

aboshUnsubmitted

Done

What exactly does this mean?

abosh: What exactly does this mean?

jonAuthorUnsubmitted

Done

It means that it's still in active development for nix support. It's largely documented here: https://github.com/CommE2E/comm/blob/master/docs/dev_services.md.

https://www.notion.so/commapp/Nix-Dev-Env-Status-170d0600d2a447fa840eb720fb11e951 will probably give you a better overview of the status; @ashoat didn't want this level of detail in user facing documentation.

jon: It means that it's still in active development for nix support. It's largely documented here…

aboshUnsubmitted

Done

Yeah I agree with @ashoat here. It's more confusing than helpful to the reader.

abosh: Yeah I agree with @ashoat here. It's more confusing than helpful to the reader.

# Requirements # Requirements

To set up a dev environment using Nix, you will need a macOS or Linux machine. To set up a dev environment using Nix, you will need a macOS or Linux machine.

# Prerequisites # Prerequisites

## Nix package manager ## Nix package manager

▲ Show 20 Lines • Show All 55 Lines • Show Last 20 Lines

[services] Tunnelbroker - Creating `main.rs` entry point file for RustClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 18105

services/tunnelbroker/src/main.rs

[services] Tunnelbroker - Creating `main.rs` entry point file for Rust
ClosedPublic
Actions

Revision Contents
Changeset List