From 81e8d88f9ab917cd85ec47e269c0f6254463c640 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 25 Nov 2024 09:59:37 +0100 Subject: [PATCH 1/4] Update contributing.md (#8205) ### Related * Follow-up to https://github.com/rerun-io/rerun/pull/8192 ### What * move some stuff in we had previously on pr template * shorten & update --- CONTRIBUTING.md | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c7a2bf7413cb..ea608d9b4657 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,7 +18,11 @@ This is written for anyone who wants to contribute to the Rerun repository. You can also look at our [`good first issue` tag](https://github.com/rerun-io/rerun/labels/good%20first%20issue). ## Pull requests -We use [Trunk Based Development](https://trunkbaseddevelopment.com/), which means we encourage small, short-lived branches. Open draft PR:s to get some early feedback on your work. +We use [Trunk Based Development](https://trunkbaseddevelopment.com/), which means we encourage small, short-lived branches. + +Open draft PR:s to get some early feedback on your work until you feel it is ready for a proper review. +Do not make PR:s from your own `main` branch, as that makes it difficult for reviewers to add their own fixes. +Add any improvements to the branch as new commits instead of rebasing to make it easier for reviewers to follow the progress (add images if possible!). All PR:s are merged with [`Squash and Merge`](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#squash-and-merge-your-commits), meaning they all get squashed to just one commit on the `main` branch. This means you don't need to keep a clean commit history on your feature branches. In fact, it is preferable to add new commits to a branch rather than rebasing or squashing. For one, it makes it easier to track progress on a branch, but rebasing and force-pushing also discourages collaboration on a branch. @@ -34,12 +38,7 @@ Members of the `rerun-io` organization and collaborators in the `rerun-io/rerun` ## Contributing to CI -Every CI job would in its ideal state consist of only two steps: - -1. Install tools and libraries[^1] -2. Run a script - -In which the script is written and tested locally before being wrapped in a CI workflow file. This does not mean that scripts are merely _reproducible_ locally (though that is also true), it means that they must be written with a _local-first mindset_, as if they are not supposed to run on CI at all. +Every CI job would in its ideal state consist of only a single `pixi` (or similar) script invocation that works locally as-is. This approach has a number of benefits: - Instead of Bash embedded in YAML, scripts may be written in an Actual Programming Language™ @@ -48,8 +47,6 @@ This approach has a number of benefits: Additionally, always output any artifacts produced by CI to GCS instead of the GHA artifact storage. This can be a serious lifesaver when something breaks, as it allows anyone to download the output of a script and continue from where it failed, instead of being forced to start over from scratch. -[^1]: For some larger jobs, we prefer to use a [docker image](https://hub.docker.com/r/rerunio/ci_docker) to make managing dependencies simpler, and to keep everything locked to a specific version as much as possible. In this case, it's still good practice to install dependencies, because it ensures the job continues to work even if the docker image is out of date. - Here are some guidelines to follow when writing such scripts: Local-first means easy for contributors to run. @@ -114,7 +111,6 @@ cargo run -p rerun -- --help ## Tools We use the [`pixi`](https://prefix.dev/) for managing dev-tool versioning, download and task running. To see available tasks, use `pixi task list`. -TODO(andreas): This doesn't list tasks from all Pixi environments. There's no way to this so far, see also [here](https://discord.com/channels/1082332781146800168/1227563080934756475/1227563080934756475). We use [cargo deny](https://github.com/EmbarkStudios/cargo-deny) to check our dependency tree for copy-left licenses, duplicate dependencies and [rustsec advisories](https://rustsec.org/advisories). You can configure it in `deny.toml`. Usage: `cargo deny check` Configure your editor to run `cargo fmt` on save. Also configure it to strip trailing whitespace, and to end each file with a newline. Settings for VSCode can be found in the `.vscode` folder and should be applied automatically. If you are using another editor, consider adding good setting to this repository! @@ -123,7 +119,7 @@ Depending on the changes you made run `cargo test --all-targets --all-features`, ### Linting Prior to pushing changes to a PR, at a minimum, you should always run `pixi run fast-lint`. This is designed to run -in a few seconds and should catch the more trivial issues to avoid wasting CI time. +in a few seconds for repeated runs and should catch the more trivial issues to avoid wasting CI time. ### Hooks We recommend adding the Rerun pre-push hook to your local checkout, which among other-things will run @@ -145,4 +141,5 @@ You can use [bacon](https://github.com/Canop/bacon) to automatically check your You can set up [`sccache`](https://github.com/mozilla/sccache) to speed up re-compilation (e.g. when switching branches). You can control the size of the cache with `export SCCACHE_CACHE_SIZE="256G"`. ### Other -You can view higher log levels with `export RUST_LOG=debug` or `export RUST_LOG=trace`. +You can view higher log levels with `export RUST_LOG=trace`. +Debug logging is automatically enabled for the viewer as long as you're running inside the `rerun` checkout. From 7b3fc2b708669192ef519d5423fa14f884a782af Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Mon, 25 Nov 2024 10:00:52 +0100 Subject: [PATCH 2/4] Do not re-hash interned hashes in non-IntMap contexts (#8200) Title. --- crates/utils/re_string_interner/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/utils/re_string_interner/src/lib.rs b/crates/utils/re_string_interner/src/lib.rs index cc3ce7cb6d60..26aa4344e8a7 100644 --- a/crates/utils/re_string_interner/src/lib.rs +++ b/crates/utils/re_string_interner/src/lib.rs @@ -78,7 +78,7 @@ impl std::cmp::PartialEq for InternedString { impl std::hash::Hash for InternedString { #[inline] fn hash(&self, state: &mut H) { - self.hash.hash(state); + state.write_u64(self.hash); } } From de225e002cb766dd27bcb31957ce1390c7937f0c Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Mon, 25 Nov 2024 10:19:07 +0100 Subject: [PATCH 3/4] Remove dead code: `Hash128` (#8203) `Hash128` isn't actually used anywhere, I just randomly stumbled upon it while refactoring some hashing related stuff. Still a good idea to fix it (or remove it entirely?) before someone's get bitten. --- crates/store/re_log_types/src/hash.rs | 73 --------------------------- 1 file changed, 73 deletions(-) diff --git a/crates/store/re_log_types/src/hash.rs b/crates/store/re_log_types/src/hash.rs index 63be3694adb1..93826b76edee 100644 --- a/crates/store/re_log_types/src/hash.rs +++ b/crates/store/re_log_types/src/hash.rs @@ -1,7 +1,3 @@ -// ---------------------------------------------------------------------------- - -use std::hash::BuildHasher; - /// 64-bit hash. /// /// 10^-12 collision risk with 6k values. @@ -61,77 +57,8 @@ impl std::fmt::Debug for Hash64 { // ---------------------------------------------------------------------------- -/// 128-bit hash. Negligible risk for collision. -#[derive(Copy, Clone, Eq)] -pub struct Hash128([u64; 2]); - -impl Hash128 { - pub const ZERO: Self = Self([0; 2]); - - pub fn hash(value: impl std::hash::Hash + Copy) -> Self { - Self(double_hash(value)) - } - - #[inline] - pub fn hash64(&self) -> u64 { - self.0[0] - } - - #[inline] - pub fn first64(&self) -> u64 { - self.0[0] - } - - #[inline] - pub fn second64(&self) -> u64 { - self.0[1] - } -} - -impl std::hash::Hash for Hash128 { - #[inline] - fn hash(&self, state: &mut H) { - state.write_u64(self.0[0]); - } -} - -impl std::cmp::PartialEq for Hash128 { - #[inline] - fn eq(&self, other: &Self) -> bool { - self.0 == other.0 - } -} - -impl nohash_hasher::IsEnabled for Hash128 {} - -impl std::fmt::Debug for Hash128 { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(&format!("Hash128({:016X}{:016X})", self.0[0], self.0[1])) - } -} - -// ---------------------------------------------------------------------------- - pub const HASH_RANDOM_STATE: ahash::RandomState = ahash::RandomState::with_seeds(0, 1, 2, 3); -#[inline] -fn double_hash(value: impl std::hash::Hash + Copy) -> [u64; 2] { - [hash_with_seed(value, 123), hash_with_seed(value, 456)] -} - -/// Hash the given value. -#[inline] -fn hash_with_seed(value: impl std::hash::Hash, seed: u128) -> u64 { - use std::hash::Hash as _; - use std::hash::Hasher as _; - - // Don't use ahash::AHasher::default() since it uses a random number for seeding the hasher on every application start. - let mut hasher = HASH_RANDOM_STATE.build_hasher(); - seed.hash(&mut hasher); - value.hash(&mut hasher); - hasher.finish() -} - /// Hash the given value. #[inline] fn hash(value: impl std::hash::Hash) -> u64 { From 1c9b942a4f149741538a4f107cf52771a8d584a8 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 25 Nov 2024 10:41:40 +0100 Subject: [PATCH 4/4] Update rustls v0.23.16 -> v0.23.18 (#8216) ### What Fixes https://rustsec.org/advisories/RUSTSEC-2024-0399 --- Cargo.lock | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4409be78b369..e4995d91c394 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3092,7 +3092,7 @@ dependencies = [ "http 1.1.0", "hyper 1.5.0", "hyper-util", - "rustls 0.23.16", + "rustls 0.23.18", "rustls-native-certs", "rustls-pki-types", "tokio", @@ -3547,7 +3547,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4979f22fdb869068da03c9f7528f8297c6fd2606bc3a4affe42e6a823fdb8da4" dependencies = [ "cfg-if", - "windows-targets 0.52.6", + "windows-targets 0.48.5", ] [[package]] @@ -5045,7 +5045,7 @@ dependencies = [ "quinn-proto", "quinn-udp", "rustc-hash 2.0.0", - "rustls 0.23.16", + "rustls 0.23.18", "socket2", "thiserror", "tokio", @@ -5062,7 +5062,7 @@ dependencies = [ "rand", "ring", "rustc-hash 2.0.0", - "rustls 0.23.16", + "rustls 0.23.18", "slab", "thiserror", "tinyvec", @@ -6770,7 +6770,7 @@ dependencies = [ "percent-encoding", "pin-project-lite", "quinn", - "rustls 0.23.16", + "rustls 0.23.18", "rustls-native-certs", "rustls-pemfile 2.2.0", "rustls-pki-types", @@ -7318,9 +7318,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.16" +version = "0.23.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eee87ff5d9b36712a58574e12e9f0ea80f915a5b0ac518d322b24a465617925e" +checksum = "9c9cc1d47e243d655ace55ed38201c19ae02c148ae56412ab8750e8f0166ab7f" dependencies = [ "log", "once_cell", @@ -8331,7 +8331,7 @@ version = "0.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c7bc40d0e5a97695bb96e27995cd3a08538541b0a846f65bba7a359f36700d4" dependencies = [ - "rustls 0.23.16", + "rustls 0.23.18", "rustls-pki-types", "tokio", ] @@ -8567,7 +8567,7 @@ dependencies = [ "httparse", "log", "rand", - "rustls 0.23.16", + "rustls 0.23.18", "rustls-pki-types", "sha1", "thiserror", @@ -8684,7 +8684,7 @@ dependencies = [ "flate2", "log", "once_cell", - "rustls 0.23.16", + "rustls 0.23.18", "rustls-pki-types", "serde", "serde_json",