From 374496c053b654d9d70e6d749c056875c6e13ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 20 Feb 2025 12:19:50 +0100 Subject: [PATCH 1/4] Report patchable fetch progress --- Cargo.lock | 198 ++++++++++++++++++++++++++++++++++++ Cargo.toml | 1 + rust/patchable/Cargo.toml | 1 + rust/patchable/src/main.rs | 2 + rust/patchable/src/repo.rs | 34 ++++++- rust/patchable/src/utils.rs | 59 +++++++++++ 6 files changed, 293 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 71183abcd..96f7f82bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -61,12 +61,24 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "arrayvec" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" + [[package]] name = "bitflags" version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f68f53c83ab957f72c32642f3868eec03eb974d1fb82e453128456482613d36" +[[package]] +name = "bumpalo" +version = "3.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1628fb46dfa0b37568d12e5edd512553eccf6a22a78e8bde00bb4aed84d5bdbf" + [[package]] name = "cc" version = "1.2.11" @@ -130,6 +142,19 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b63caa9aa9397e2d9480a9b13673856c78d8ac123288526c37d7839f2a86990" +[[package]] +name = "console" +version = "0.15.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ea3c6ecd8059b57859df5c69830340ed3c41d30e3da0c1cbed90a96ac853041b" +dependencies = [ + "encode_unicode", + "libc", + "once_cell", + "unicode-width 0.2.0", + "windows-sys", +] + [[package]] name = "deranged" version = "0.3.11" @@ -150,6 +175,12 @@ dependencies = [ "syn", ] +[[package]] +name = "encode_unicode" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" + [[package]] name = "equivalent" version = "1.0.1" @@ -369,12 +400,32 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "indicatif" +version = "0.17.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "183b3088984b400f4cfac3620d5e076c84da5364016b4f49473de574b2586235" +dependencies = [ + "console", + "number_prefix", + "portable-atomic", + "unicode-width 0.2.0", + "vt100", + "web-time", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7943c866cc5cd64cbc25b2e01621d07fa8eb2a1a23160ee81ce38704e97b8ecf" +[[package]] +name = "itoa" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d75a2a4b1b190afb6f5425f10f6a8f959d2ea0b9c2b1d79553551850539e4674" + [[package]] name = "jobserver" version = "0.1.32" @@ -384,6 +435,16 @@ dependencies = [ "libc", ] +[[package]] +name = "js-sys" +version = "0.3.77" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1cfaf33c695fc6e08064efbc1f72ec937429614f25eef83af942d0e227c3a28f" +dependencies = [ + "once_cell", + "wasm-bindgen", +] + [[package]] name = "lazy_static" version = "1.5.0" @@ -485,6 +546,12 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "51d515d32fb182ee37cda2ccdcb92950d6a3c2893aa280e540671c2cd0f3b1d9" +[[package]] +name = "number_prefix" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "830b246a0e5f20af87141b25c173cd1b609bd7779a4617d6ec582abaf90870f3" + [[package]] name = "once_cell" version = "1.20.2" @@ -527,6 +594,7 @@ dependencies = [ "time", "toml", "tracing", + "tracing-indicatif", "tracing-subscriber", ] @@ -548,6 +616,12 @@ version = "0.3.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "953ec861398dccce10c670dfeaf3ec4911ca479e9c02154b3a215178c5f566f2" +[[package]] +name = "portable-atomic" +version = "1.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "280dc24453071f1b63954171985a0b0d30058d287960968b9b2aca264c8d4ee6" + [[package]] name = "powerfmt" version = "0.2.0" @@ -864,6 +938,18 @@ dependencies = [ "valuable", ] +[[package]] +name = "tracing-indicatif" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8201ca430e0cd893ef978226fd3516c06d9c494181c8bf4e5b32e30ed4b40aa1" +dependencies = [ + "indicatif", + "tracing", + "tracing-core", + "tracing-subscriber", +] + [[package]] name = "tracing-log" version = "0.2.0" @@ -899,6 +985,18 @@ version = "1.0.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a210d160f08b701c8721ba1c726c11662f877ea6b7094007e1ca9a1041945034" +[[package]] +name = "unicode-width" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" + +[[package]] +name = "unicode-width" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fc81956842c57dac11422a97c3b8195a1ff727f06e85c84ed2e8aa277c9a0fd" + [[package]] name = "url" version = "2.5.4" @@ -940,6 +1038,39 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" +[[package]] +name = "vt100" +version = "0.15.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "84cd863bf0db7e392ba3bd04994be3473491b31e66340672af5d11943c6274de" +dependencies = [ + "itoa", + "log", + "unicode-width 0.1.14", + "vte", +] + +[[package]] +name = "vte" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f5022b5fbf9407086c180e9557be968742d839e68346af7792b8592489732197" +dependencies = [ + "arrayvec", + "utf8parse", + "vte_generate_state_changes", +] + +[[package]] +name = "vte_generate_state_changes" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e369bee1b05d510a7b4ed645f5faa90619e05437111783ea5848f28d97d3c2e" +dependencies = [ + "proc-macro2", + "quote", +] + [[package]] name = "wasi" version = "0.13.3+wasi-0.2.2" @@ -949,6 +1080,73 @@ dependencies = [ "wit-bindgen-rt", ] +[[package]] +name = "wasm-bindgen" +version = "0.2.100" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1edc8929d7499fc4e8f0be2262a241556cfc54a0bea223790e71446f2aab1ef5" +dependencies = [ + "cfg-if", + "once_cell", + "wasm-bindgen-macro", +] + +[[package]] +name = "wasm-bindgen-backend" +version = "0.2.100" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f0a0651a5c2bc21487bde11ee802ccaf4c51935d0d3d42a6101f98161700bc6" +dependencies = [ + "bumpalo", + "log", + "proc-macro2", + "quote", + "syn", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-macro" +version = "0.2.100" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7fe63fc6d09ed3792bd0897b314f53de8e16568c2b3f7982f468c0bf9bd0b407" +dependencies = [ + "quote", + "wasm-bindgen-macro-support", +] + +[[package]] +name = "wasm-bindgen-macro-support" +version = "0.2.100" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ae87ea40c9f689fc23f209965b6fb8a99ad69aeeb0231408be24920604395de" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "wasm-bindgen-backend", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-shared" +version = "0.2.100" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a05d73b933a847d6cccdda8f838a22ff101ad9bf93e33684f39c1f5f0eece3d" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "web-time" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a6580f308b1fad9207618087a65c04e7a10bc77e02c8e84e9b00dd4b12fa0bb" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + [[package]] name = "winapi" version = "0.3.9" diff --git a/Cargo.toml b/Cargo.toml index a019301e4..673aa5881 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,4 +11,5 @@ tempfile = "3.16.0" time = { version = "0.3.37", features = ["parsing"] } toml = "0.8.19" tracing = "0.1.41" +tracing-indicatif = "0.3.9" tracing-subscriber = { version = "0.3.19", features = ["env-filter"] } diff --git a/rust/patchable/Cargo.toml b/rust/patchable/Cargo.toml index 69895307c..cb72ed092 100644 --- a/rust/patchable/Cargo.toml +++ b/rust/patchable/Cargo.toml @@ -12,4 +12,5 @@ tempfile.workspace = true time.workspace = true toml.workspace = true tracing.workspace = true +tracing-indicatif.workspace = true tracing-subscriber.workspace = true diff --git a/rust/patchable/src/main.rs b/rust/patchable/src/main.rs index 45c180842..6635208a6 100644 --- a/rust/patchable/src/main.rs +++ b/rust/patchable/src/main.rs @@ -14,6 +14,7 @@ use std::{ use git2::{Oid, Repository}; use serde::{Deserialize, Serialize}; use snafu::{OptionExt, ResultExt as _, Snafu}; +use tracing_indicatif::IndicatifLayer; use tracing_subscriber::{layer::SubscriberExt as _, util::SubscriberInitExt as _}; #[derive(clap::Parser)] @@ -221,6 +222,7 @@ type Result = std::result::Result; fn main() -> Result<()> { tracing_subscriber::registry() .with(tracing_subscriber::fmt::layer().with_writer(std::io::stderr)) + .with(IndicatifLayer::new()) .with( tracing_subscriber::EnvFilter::builder() .with_default_directive(tracing_subscriber::filter::LevelFilter::INFO.into()) diff --git a/rust/patchable/src/repo.rs b/rust/patchable/src/repo.rs index 07f926ae2..f7efd2459 100644 --- a/rust/patchable/src/repo.rs +++ b/rust/patchable/src/repo.rs @@ -1,9 +1,16 @@ use std::path::{Path, PathBuf}; -use git2::{FetchOptions, ObjectType, Oid, Repository, RepositoryInitOptions, WorktreeAddOptions}; +use git2::{ + FetchOptions, ObjectType, Oid, RemoteCallbacks, Repository, RepositoryInitOptions, + WorktreeAddOptions, +}; use snafu::{ResultExt, Snafu}; +use tracing_indicatif::span_ext::IndicatifSpanExt; -use crate::error::{self, CommitRef}; +use crate::{ + error::{self, CommitRef}, + utils::{progress_bar_style, Quantizer}, +}; #[derive(Debug, Snafu)] pub enum Error { @@ -129,6 +136,28 @@ pub fn resolve_and_fetch_commitish( error = &err as &dyn std::error::Error, "base commit not found locally, fetching from upstream" ); + let span_recv = tracing::info_span!("receiving"); + let span_index = tracing::info_span!("indexing"); + span_recv.pb_set_style(&progress_bar_style()); + span_index.pb_set_style(&progress_bar_style()); + let _ = span_recv.enter(); + let _ = span_index.enter(); + let mut callbacks = RemoteCallbacks::new(); + let mut quant_recv = Quantizer::percent(); + let mut quant_index = Quantizer::percent(); + callbacks.transfer_progress(move |progress| { + quant_recv.update_span_progress( + progress.received_objects(), + progress.total_objects(), + &span_recv, + ); + quant_index.update_span_progress( + progress.indexed_objects(), + progress.total_objects(), + &span_index, + ); + true + }); repo.remote_anonymous(upstream_url) .context(CreateRemoteSnafu { repo, @@ -139,6 +168,7 @@ pub fn resolve_and_fetch_commitish( Some( FetchOptions::new() .update_fetchhead(true) + .remote_callbacks(callbacks) // TODO: could be 1, CLI option maybe? .depth(0), ), diff --git a/rust/patchable/src/utils.rs b/rust/patchable/src/utils.rs index f71557a28..26b94e928 100644 --- a/rust/patchable/src/utils.rs +++ b/rust/patchable/src/utils.rs @@ -1,6 +1,65 @@ use std::path::Path; use git2::Repository; +use tracing::Span; +use tracing_indicatif::{span_ext::IndicatifSpanExt, style::ProgressStyle}; + +pub fn progress_bar_style() -> ProgressStyle { + ProgressStyle::with_template( + "{span_child_prefix}{spinner} {span_name}{{{span_fields}}} {wide_msg} {bar:40} {percent:>3}%", + ) + .expect("hard-coded template should be valid") +} + +/// Runs a function whenever a `value` changes "enough". +/// +/// See [`Self::update`], and especially [`Self::update_span_progress`]. +pub struct Quantizer { + value: usize, + total: usize, + divisor: usize, +} + +impl Quantizer { + fn new(divisor: usize) -> Self { + Self { + divisor, + total: 0, + value: 0, + } + } + + pub fn percent() -> Self { + Self::new(100) + } + + fn fraction(&self, value: usize) -> usize { + value.checked_div(self.total / self.divisor).unwrap_or(0) + } + + /// Runs `f` if `total` has changed, or `value` has changed significantly compared to `total`. + pub fn update(&mut self, value: usize, total: usize, f: impl FnOnce()) { + if self.total != total + || self.fraction(self.value) != self.fraction(value) + || (value == total) != (self.value == total) + { + f(); + self.value = value; + self.total = total; + } + } + + /// Updates the progress of `span`, if there is a significant change. + /// + /// Use this when there are many fine-grained steps, since otherwise the cost of [`Span::pb_set_position`] + /// will dominate the cost of the actual task. + pub fn update_span_progress(&mut self, value: usize, total: usize, span: &Span) { + self.update(value, total, || { + span.pb_set_length(total as u64); + span.pb_set_position(value as u64); + }); + } +} /// Runs a raw git command in the environment of a Git repository. /// From c6ca2cc76d7a3548bb2b670d9073c8fab28de99a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 20 Feb 2025 12:27:14 +0100 Subject: [PATCH 2/4] Avoid clobbering child processes' output --- rust/patchable/src/patch.rs | 27 +++++++++-------- rust/patchable/src/patch_mail.rs | 50 ++++++++++++++++++-------------- rust/patchable/src/utils.rs | 5 ++++ 3 files changed, 48 insertions(+), 34 deletions(-) diff --git a/rust/patchable/src/patch.rs b/rust/patchable/src/patch.rs index b29504c17..62637f6f8 100644 --- a/rust/patchable/src/patch.rs +++ b/rust/patchable/src/patch.rs @@ -5,6 +5,7 @@ use std::{ use git2::{Oid, Repository}; use snafu::{OptionExt, ResultExt as _, Snafu}; +use tracing_indicatif::suspend_tracing_indicatif; use crate::{ error::{self, CommitId}, @@ -342,18 +343,20 @@ pub fn format_patches( } tracing::info!("exporting commits since base"); - let status = raw_git_cmd(repo) - .arg("format-patch") - .arg(format!("{base_commit}..{leaf_commit}")) - .arg("-o") - .arg(patch_dir) - .args([ - "--keep-subject", - // By default, git includes its own version number as a suffix, which makes patches unstable across git versions - "--no-signature", - ]) - .status() - .context(RunFormatMailSnafu)?; + let status = suspend_tracing_indicatif(|| { + raw_git_cmd(repo) + .arg("format-patch") + .arg(format!("{base_commit}..{leaf_commit}")) + .arg("-o") + .arg(patch_dir) + .args([ + "--keep-subject", + // By default, git includes its own version number as a suffix, which makes patches unstable across git versions + "--no-signature", + ]) + .status() + }) + .context(RunFormatMailSnafu)?; if !status.success() { return FormatMailFailedSnafu { status }.fail(); } diff --git a/rust/patchable/src/patch_mail.rs b/rust/patchable/src/patch_mail.rs index 6ff96d8a4..1472addc3 100644 --- a/rust/patchable/src/patch_mail.rs +++ b/rust/patchable/src/patch_mail.rs @@ -11,6 +11,7 @@ use git2::{Diff, Repository, Signature}; use snafu::{OptionExt as _, ResultExt, Snafu}; use tempfile::{tempdir, NamedTempFile}; use time::{format_description::well_known::Rfc2822, OffsetDateTime}; +use tracing_indicatif::suspend_tracing_indicatif; use crate::utils::raw_git_cmd; @@ -76,19 +77,21 @@ type Result = std::result::Result; /// Splits a series of git patch emails into individual patch emails. pub fn mailsplit(repo: &Repository, patch_file: &Path) -> Result> { let base_dir = tempdir().context(CreateTempDirSnafu)?; - let mailsplit = raw_git_cmd(repo) - .arg("mailsplit") - // mailsplit doesn't accept split arguments ("-o dir") - .arg({ - let mut output_arg = OsString::from("-o"); - output_arg.push(base_dir.path()); - output_arg - }) - .arg("--") - .arg(patch_file) - .stderr(Stdio::inherit()) - .output() - .context(RunMailsplitSnafu)?; + let mailsplit = suspend_tracing_indicatif(|| { + raw_git_cmd(repo) + .arg("mailsplit") + // mailsplit doesn't accept split arguments ("-o dir") + .arg({ + let mut output_arg = OsString::from("-o"); + output_arg.push(base_dir.path()); + output_arg + }) + .arg("--") + .arg(patch_file) + .stderr(Stdio::inherit()) + .output() + }) + .context(RunMailsplitSnafu)?; if !mailsplit.status.success() { return MailsplitFailedSnafu { status: mailsplit.status, @@ -170,15 +173,18 @@ pub fn mailinfo(repo: &Repository, patch_email_file: &Path) -> Result let patch_file = NamedTempFile::new() .context(CreateTempFileSnafu)? .into_temp_path(); - let mailinfo = raw_git_cmd(repo) - .arg("mailinfo") - .args([&msg_file, &patch_file]) - .stdin(File::open(patch_email_file).context(OpenMailFileSnafu { - path: patch_email_file, - })?) - .stderr(Stdio::inherit()) - .output() - .context(RunMailinfoSnafu)?; + let patch_email_file = File::open(patch_email_file).context(OpenMailFileSnafu { + path: patch_email_file, + })?; + let mailinfo = suspend_tracing_indicatif(|| { + raw_git_cmd(repo) + .arg("mailinfo") + .args([&msg_file, &patch_file]) + .stdin(patch_email_file) + .stderr(Stdio::inherit()) + .output() + }) + .context(RunMailinfoSnafu)?; if !mailinfo.status.success() { return MailinfoFailedSnafu { status: mailinfo.status, diff --git a/rust/patchable/src/utils.rs b/rust/patchable/src/utils.rs index 26b94e928..9d08666d1 100644 --- a/rust/patchable/src/utils.rs +++ b/rust/patchable/src/utils.rs @@ -64,6 +64,11 @@ impl Quantizer { /// Runs a raw git command in the environment of a Git repository. /// /// Used for functionality that is not currently implemented by libgit2/gix. +/// +/// NOTE: To avoid clobbering the terminal output, processes executed by this +/// that inherit stdout and/or stderr *must* wrap the execution of the child +/// (such as [`std::process::Command::output`] or [`std::process::Command::status`]) +/// in [`tracing_indicatif::suspend_tracing_indicatif`]. pub fn raw_git_cmd(repo: &Repository) -> std::process::Command { let mut cmd = std::process::Command::new("git"); cmd.env("GIT_DIR", repo.path()); From dc128c301a9578b80592692fa21b0463c58912e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 20 Feb 2025 12:31:05 +0100 Subject: [PATCH 3/4] Fix rustdoc warnings --- rust/patchable/src/main.rs | 2 ++ rust/patchable/src/patch.rs | 3 +++ 2 files changed, 5 insertions(+) diff --git a/rust/patchable/src/main.rs b/rust/patchable/src/main.rs index 6635208a6..e01dba052 100644 --- a/rust/patchable/src/main.rs +++ b/rust/patchable/src/main.rs @@ -110,6 +110,8 @@ struct Opts { } #[derive(clap::Parser)] +// CLI parameters are documented for the CLI's `--help`, not for rustdoc +#[allow(rustdoc::bare_urls, rustdoc::invalid_html_tags)] enum Cmd { /// Check out a patched source tree to docker-images//patchable-work/worktree/ /// diff --git a/rust/patchable/src/patch.rs b/rust/patchable/src/patch.rs index 62637f6f8..00cbd1ae9 100644 --- a/rust/patchable/src/patch.rs +++ b/rust/patchable/src/patch.rs @@ -13,6 +13,9 @@ use crate::{ utils::raw_git_cmd, }; +#[cfg(doc)] +use crate::repo::ensure_worktree_is_at; + #[derive(Debug, Snafu)] pub enum Error { #[snafu(display("failed to open stgit series file {path:?}"))] From 3ad86d0dd764a19b76b4ec58e10893e26aaaa6f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 20 Feb 2025 12:34:56 +0100 Subject: [PATCH 4/4] Changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be778a0dd..3b3b39973 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ All notable changes to this project will be documented in this file. - trino-cli: Add version 470 ([#999]). - trino-storage-connector: Add version 470 ([#999]). - superset: Add version `4.1.1` ([#991]). -- Add Patchable patch management tool ([#1003]). +- Add Patchable patch management tool ([#1003], [#1007]). - nifi: Add 1.28.1, 2.2.0 ([#1006]). ### Changed @@ -77,6 +77,7 @@ All notable changes to this project will be documented in this file. [#1000]: https://github.com/stackabletech/docker-images/pull/1000 [#1003]: https://github.com/stackabletech/docker-images/pull/1003 [#1006]: https://github.com/stackabletech/docker-images/pull/1006 +[#1007]: https://github.com/stackabletech/docker-images/pull/1007 ## [24.11.1] - 2025-01-14