Details

Reviewers

varun

Commits

rCOMM2c0a48ffe7a8: Introduce Cargo workspace and update Dockerfiles

Summary

Introduced cargo workspace for services, shared and rust-node-addon.
Some context in this comment.

I put inline comments to explain it better.

This is the first and most important diff in the stack.

Test Plan

Locally: cargo commands inside packages work as before
- When ran in repo root, e.g. cargo check runs wor the whole workspace
Docker build times and image sizes are comparable to non-workspace setup
Pre-commit hooks for Rust still work as before
CI: All Docker images are built successfully, including Keyserver
CI: All non-Docker jobs (e.g. Android, iOS) compile Rust successfully
Ran Services with Commtest locally
Commtest on CI runs - it requires all services' docker images + its own Docker

Diff Detail

Repository

rCOMM Comm

Branch

barthap/cargo-workspace

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.May 21 2024, 6:09 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptMay 21 2024, 6:09 AM

Harbormaster failed remote builds in B29089: Diff 40453!May 21 2024, 6:10 AM

commit cargo.toml

Harbormaster failed remote builds in B29091: Diff 40455!May 21 2024, 6:17 AM

fix dockerfiles

Harbormaster failed remote builds in B29093: Diff 40457!May 21 2024, 6:23 AM

fix dockerfiles

Harbormaster failed remote builds in B29095: Diff 40459!May 21 2024, 6:45 AM

fix dockerfiles

Harbormaster failed remote builds in B29097: Diff 40461!May 21 2024, 6:52 AM

try fix

try fix ios

Harbormaster failed remote builds in B29101: Diff 40467!May 21 2024, 7:27 AM

Harbormaster failed remote builds in B29102: Diff 40468!May 21 2024, 7:30 AM

another try

Harbormaster failed remote builds in B29118: Diff 40485!May 21 2024, 8:36 AM

another try

Harbormaster failed remote builds in B29119: Diff 40486!May 21 2024, 8:50 AM

another try

Harbormaster failed remote builds in B29124: Diff 40492!May 21 2024, 9:28 AM

Some cleanup

Harbormaster completed remote builds in B29125: Diff 40493.May 21 2024, 10:06 AM

Clean up and split diffs

bartek retitled this revision from [DRAFT] Try cargo workspace for CI to Introduce Cargo workspace and update Dockerfiles.May 21 2024, 1:28 PM

bartek edited the summary of this revision. (Show Details)

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

Harbormaster completed remote builds in B29146: Diff 40514.May 21 2024, 1:32 PM

bartek published this revision for review.May 21 2024, 10:41 PM

bartek edited the summary of this revision. (Show Details)

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

bartek added inline comments.

keyserver/Dockerfile
145–147 ↗	(On Diff #40514)	For keyserver, we don't copy any of `services` into Docker image, so `cargo build` would fail with `Couldn't find 'services/*/Cargo.toml' - no such file or directory`. This `sed` removes that line from `Cargo.toml`
services/backup/Dockerfile
7 ↗	(On Diff #40514)	Directory structure inside Docker image must follow structure from repo. Otherwise `Cargo` won't be able to find correct paths
19 ↗	(On Diff #40514)	Similarly to keyserver scenario, this sed removes `rust-node-addon` from workspace members because it's not copied into Docker image and Cargo would be unable to find it. It should work without the others now (they're now excluded), I'll check it and apply this change in the next revision
services/commtest/src/identity/mod.rs
2 ↗	(On Diff #40514)	When I included CommTest into workspace, it downloaded a littne newer version of `ed25519_dalek` which replaced `SignerMut` with `Signer`
services/identity/Dockerfile
14–23 ↗	(On Diff #40514)	This was to cache dependencies when only modifying sources. But from my experience, it wasn't saving much time. On CI, everything is recompiled each time

bartek added a child revision: D12168: Delete unused cargo.lock files.May 22 2024, 2:42 AM

nice!

This revision is now accepted and ready to land.May 24 2024, 10:13 AM

Closed by commit rCOMM2c0a48ffe7a8: Introduce Cargo workspace and update Dockerfiles (authored by bartek). · Explain WhyMay 28 2024, 8:12 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMM2c0a48ffe7a8: Introduce Cargo workspace and update Dockerfiles.

Introduce Cargo workspace and update DockerfilesClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 40461

.gitignore

Cargo.lock

Cargo.toml

keyserver/Dockerfile

keyserver/addons/rust-node-addon/Cargo.toml

native/native_rust_library/Cargo.toml

native/native_rust_library/src/backup.rs

native/native_rust_library/src/identity/login.rs

native/native_rust_library/src/identity/registration.rs

native/native_rust_library/src/identity/x3dh.rs

native/native_rust_library/src/lib.rs

services/backup/Cargo.toml

services/backup/Dockerfile

services/blob/Cargo.toml

services/blob/Dockerfile

services/blob/src/service.rs

services/commtest/Cargo.toml

services/commtest/Dockerfile

services/commtest/src/identity/mod.rs

services/commtest/tests/identity_access_tokens_tests.rs

services/commtest/tests/identity_one_time_key_tests.rs

services/feature-flags/Cargo.toml

services/feature-flags/Dockerfile

services/identity/Cargo.toml

services/identity/Dockerfile

services/identity/src/database.rs

services/identity/src/ddb_utils.rs

services/identity/src/device_list.rs

services/identity/src/grpc_services/authenticated.rs

services/identity/src/grpc_services/shared.rs

services/identity/src/regex.rs

services/identity/src/siwe.rs

services/reports/Cargo.toml

services/reports/Dockerfile

services/tunnelbroker/Cargo.toml

services/tunnelbroker/Dockerfile

shared/backup_client/Cargo.toml

shared/comm-lib/Cargo.toml

shared/comm-lib/src/auth/service.rs

shared/comm-lib/src/database.rs

shared/comm-lib/src/lib.rs

Introduce Cargo workspace and update Dockerfiles
ClosedPublic
Actions

Revision Contents
Changeset List