[Pkg-rust-maintainers] Bug#1040837: Bug#1040837: rust-log: breaks API without coordination

Fabian Grünbichler fabian at gruenbichler.email
Wed Jul 12 18:53:08 BST 2023


On Tue, Jul 11, 2023 at 01:47:53PM +0200, Jonas Smedegaard wrote:
> Source: rust-log
> Version: 0.4.19-2
> Severity: serious
> 
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> rust-log 0.4.19-2 apparently removed upstream feature kv_unstable
> (according to inspection of source of that packaging release -
> related changelog entry does not mention that feature).
> 
> This change to upstream API was done without coordination with
> reverse dependencies, and breaks at least librust-async-std-dev
> and its 17 reverse dependencies.
> 
> Yes, I am aware that changelog entry indicates this being a temporary
> change, pending packages lingering in NEW queue.  Please don't release
> breaking changes without prior coordination with reverse dependencies
> (e.g. the changes that cause bug#1040702), and in cases that is not
> possible (e.g. when accidentally ending at bug#1040702) then please
> notify reverse dependencies e.g. using a bugreport with tag "affects".

This was by mistake, and not on purpose.

The feature in question is probably not a good candidate for packaging
though, given the lack of stability guarantees provided by upstream[0]:

> This module is unstable and breaking changes may be made at any time.
> See the tracking issue for more details.

The referenced tracking issue can be found here[1].

While the required crates (multiple ones from sval-rs/value-bag) are
being packaged (mostly done, pending a re-upload to NEW and review
there), I wonder whether enabling the feature was not a
mistake/premature in the first place.

I did a quick test with ratt with the attached diff applied, and except
for rust-sequoia-net (which fails for other reasons which are already
fixed in experimental and just need a re-upload of the version there to
unstable), building at least worked fine. I am not too familiar with
either async-std, nor the kv feature of log to say whether this approach
would be feasible - I'd be happy to hear your thoughts though! IMHO
keeping this unstable aspect out of the archive for the time being would
save us all from periodic breakage, with log itself (without the KV
feature) being rather widely used.

In a slightly related note, there will be two upcoming transitions that
will affect (rust) packages of yours (bindgen to 0.65, and toml to 0.7)
as part of updating rust-cargo to 0.70. Would you appreciate bugs with
patches filed before the transition starts, or do you want to handle
those on your own? You can find some details in the (WIP) tracking
issue[2]. bindgen should be pretty straight-forward, for toml we will
likely opt for a period of semver-suffixing since the versions do have
breaking changes where breakage is not detected at compile time.

0: https://docs.rs/log/latest/log/kv/index.html
1: https://github.com/rust-lang/log/issues/328
2: https://salsa.debian.org/rust-team/debcargo-conf/-/issues/48
-------------- next part --------------
diff --git a/rust-async-std-1.12.0/debian/changelog b/rust-async-std-1.12.0-patched/debian/changelog
index dfa43a8..5d6258f 100644
--- a/rust-async-std-1.12.0/debian/changelog
+++ b/rust-async-std-1.12.0-patched/debian/changelog
@@ -1,3 +1,9 @@
+rust-async-std (1.12.0-12.1) unstable; urgency=medium
+
+  * Non-maintainer upload.
+
+ -- Fabian Gr?nbichler <f.gruenbichler at proxmox.com>  Wed, 12 Jul 2023 13:43:22 +0200
+
 rust-async-std (1.12.0-12) unstable; urgency=medium
 
   * tighten autopkgtests
diff --git a/rust-async-std-1.12.0/debian/control b/rust-async-std-1.12.0-patched/debian/control
index 767ca01..84a3e66 100644
--- a/rust-async-std-1.12.0/debian/control
+++ b/rust-async-std-1.12.0-patched/debian/control
@@ -12,15 +12,12 @@ Build-Depends:
  librust-async-lock-2+default-dev <!nocheck>,
  librust-async-process-1+default-dev <!nocheck>,
  librust-crossbeam-utils-0.8+default-dev <!nocheck>,
- librust-femme-2+default-dev <!nocheck>,
  librust-futures-0.3+default-dev <!nocheck>,
  librust-futures-core-0.3+alloc-dev <!nocheck>,
  librust-futures-core-0.3+std-dev <!nocheck>,
  librust-futures-io-0.3+default-dev <!nocheck>,
  librust-futures-lite-1+default-dev <!nocheck>,
- librust-kv-log-macro-1+default-dev <!nocheck>,
  librust-log-0.4+default-dev <!nocheck>,
- librust-log-0.4+kv-unstable-dev <!nocheck>,
  librust-memchr-2+default-dev <!nocheck>,
  librust-once-cell-1+default-dev <!nocheck>,
  librust-pin-project-lite-0.2+default-dev <!nocheck>,
@@ -53,9 +50,7 @@ Depends:
  librust-futures-core-0.3+alloc-dev,
  librust-futures-io-0.3+default-dev,
  librust-futures-lite-1+default-dev,
- librust-kv-log-macro-1+default-dev,
  librust-log-0.4+default-dev,
- librust-log-0.4+kv-unstable-dev,
  librust-memchr-2+default-dev,
  librust-once-cell-1+default-dev,
  librust-pin-project-lite-0.2+default-dev,
diff --git a/rust-async-std-1.12.0-patched/debian/patches/disable-unstable-log-kv.diff b/rust-async-std-1.12.0-patched/debian/patches/disable-unstable-log-kv.diff
new file mode 100644
index 0000000..33b5de9
--- /dev/null
+++ b/rust-async-std-1.12.0-patched/debian/patches/disable-unstable-log-kv.diff
@@ -0,0 +1,77 @@
+Index: rust-async-std-1.12.0/Cargo.toml
+===================================================================
+--- rust-async-std-1.12.0.orig/Cargo.toml
++++ rust-async-std-1.12.0/Cargo.toml
+@@ -25,8 +25,6 @@ default = [
+   "async-global-executor",
+   "async-io",
+   "futures-lite",
+-  "kv-log-macro",
+-  "log",
+   "pin-project-lite",
+ ]
+ docs = ["attributes", "unstable", "default"]
+@@ -60,8 +58,7 @@ async-lock = { version = "2.3.0", option
+ crossbeam-utils = { version = "0.8.0", optional = true }
+ futures-core = { version = "0.3.4", optional = true, default-features = false }
+ futures-io = { version = "0.3.4", optional = true }
+-kv-log-macro = { version = "1.0.6", optional = true }
+-log = { version = "0.4.8", features = ["kv_unstable"], optional = true }
++log = { version = "0.4.8", features = [], optional = true }
+ memchr = { version = "2.3.3", optional = true }
+ once_cell = { version = "1.3.1", optional = true }
+ pin-project-lite = { version = "0.2.0", optional = true }
+@@ -76,7 +73,6 @@ futures-lite = { version = "1.0.0", opti
+ async-process = { version = "1.0.1", optional = true }
+ 
+ [dev-dependencies]
+-femme = "2.1.1"
+ rand = "0.8.0"
+ tempfile = "3.1.0"
+ futures = "0.3.4"
+Index: rust-async-std-1.12.0/src/task/builder.rs
+===================================================================
+--- rust-async-std-1.12.0.orig/src/task/builder.rs
++++ rust-async-std-1.12.0/src/task/builder.rs
+@@ -54,6 +54,7 @@ impl Builder {
+     {
+         let wrapped = self.build(future);
+ 
++        #[cfg(feature = "kv_log_macro")]
+         kv_log_macro::trace!("spawn", {
+             task_id: wrapped.tag.id().0,
+             parent_task_id: TaskLocalsWrapper::get_current(|t| t.id().0).unwrap_or(0),
+@@ -74,6 +75,7 @@ impl Builder {
+     {
+         let wrapped = self.build(future);
+ 
++        #[cfg(feature = "kv_log_macro")]
+         kv_log_macro::trace!("spawn_local", {
+             task_id: wrapped.tag.id().0,
+             parent_task_id: TaskLocalsWrapper::get_current(|t| t.id().0).unwrap_or(0),
+@@ -147,6 +149,7 @@ impl Builder {
+         let wrapped = self.build(future);
+ 
+         // Log this `block_on` operation.
++        #[cfg(feature = "kv_log_macro")]
+         kv_log_macro::trace!("block_on", {
+             task_id: wrapped.tag.id().0,
+             parent_task_id: TaskLocalsWrapper::get_current(|t| t.id().0).unwrap_or(0),
+Index: rust-async-std-1.12.0/examples/logging.rs
+===================================================================
+--- rust-async-std-1.12.0.orig/examples/logging.rs
++++ rust-async-std-1.12.0/examples/logging.rs
+@@ -3,11 +3,11 @@
+ use async_std::task;
+ 
+ fn main() {
+-    femme::with_level(log::LevelFilter::Trace);
++//    femme::with_level(log::LevelFilter::Trace);
+ 
+     task::block_on(async {
+         let handle = task::spawn(async {
+-            log::info!("Hello world!");
++//            log::info!("Hello world!");
+         });
+ 
+         handle.await;
diff --git a/rust-async-std-1.12.0/debian/patches/series b/rust-async-std-1.12.0-patched/debian/patches/series
index 1781273..5f5a6d2 100644
--- a/rust-async-std-1.12.0/debian/patches/series
+++ b/rust-async-std-1.12.0-patched/debian/patches/series
@@ -1,3 +1,4 @@
 2001_wasm.patch
 2002_surf.patch
 2003_no_feature_tokio0N.patch
+disable-unstable-log-kv.diff


More information about the Pkg-rust-maintainers mailing list