Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gstring: Add fmt::Debug bounds #1185

Closed
wants to merge 1 commit into from
Closed

gstring: Add fmt::Debug bounds #1185

wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Contributor

This fixes a compliation error for me that looks correct; I have no idea how CI is working. I'm using Rust 1.72 here.

@cgwalters
gstring: Add fmt::Debug bounds
78d1ddd 
This fixes a compliation error for me that looks correct; I have
no idea how CI is working.  I'm using Rust 1.72 here.
@cgwalters
Copy link
Contributor Author

Ah the answer may be that CI is using nightly, AFAICS:

   nightly-x86_64-unknown-linux-gnu installed - rustc 1.74.0-nightly (030e4d382 2023-09-10)

info: checking for self-update
warning: tool `rust-analyzer` is already installed, remove it from `/home/runner/.cargo/bin`, then run `rustup update` to have rustup manage this tool.
warning: tool `rustfmt` is already installed, remove it from `/home/runner/.cargo/bin`, then run `rustup update` to have rustup manage this tool.
warning: tool `cargo-fmt` is already installed, remove it from `/home/runner/.cargo/bin`, then run `rustup update` to have rustup manage this tool.
/home/runner/.cargo/bin/rustup override set nightly
info: using existing install for 'nightly-x86_64-unknown-linux-gnu'
info: override toolchain for '/home/runner/work/gtk-rs-core/gtk-rs-core' set to 'nightly-x86_64-unknown-linux-gnu'

@cgwalters cgwalters mentioned this pull request Sep 15, 2023
Merged
@sdroege
Copy link
Member

sdroege commented Sep 15, 2023

Why would this be needed? The Debug bound is only needed for the own Debug and Error impls in theory, not for everything else. It's usually a good idea to not put bounds on struct definitions as they otherwise contaminate everything :)

@cgwalters
Copy link
Contributor Author

Why would this be needed?

Did you try compiling with a stable rustc?

@sdroege
Copy link
Member

sdroege commented Sep 15, 2023

Sure:

$ rustc --version
rustc 1.72.0 (5680fa18f 2023-08-23)
$ cargo check --features=v2_76 --all-targets -p glib
   Compiling proc-macro2 v1.0.67
   Compiling unicode-ident v1.0.12
[...]
    Checking glib v0.19.0 (/home/slomo/Projects/rust/gtk-rs/gtk-rs-core/glib)
    Checking basic-toml v0.1.4
    Checking ciborium v0.2.1
    Checking tinytemplate v1.2.1
    Checking criterion v0.5.1
    Finished dev [unoptimized + debuginfo] target(s) in 8.31s

Also with nightly

$ rustc +nightly --version
rustc 1.74.0-nightly (ca2b74f1a 2023-09-14)
$ cargo +nightly check --features=v2_76 --all-targets -p glib
   Compiling proc-macro2 v1.0.67
   Compiling unicode-ident v1.0.12
[...]
    Checking glib v0.19.0 (/home/slomo/Projects/rust/gtk-rs/gtk-rs-core/glib)
    Checking ciborium v0.2.1
    Checking basic-toml v0.1.4
    Checking tinytemplate v1.2.1
    Checking criterion v0.5.1
    Finished dev [unoptimized + debuginfo] target(s) in 8.45s

@sdroege
Copy link
Member

sdroege commented Sep 15, 2023

What are the errors you're getting?

@cgwalters
Copy link
Contributor Author

cgwalters commented Sep 15, 2023 •
edited
Loading

Over here #1181 (comment) but just to repeat for convenience:

$ rustc --version
rustc 1.72.0 (5680fa18f 2023-08-23) (Fedora 1.72.0-1.fc38)
$ cargo check
    Checking glib v0.19.0 (/var/srv/walters/src/github/gtk-rs/gtk-rs-core/glib)
error[E0277]: `T` doesn't implement `Debug`
    --> glib/src/gstring.rs:1302:12
     |
1302 | pub struct GStringNoTrailingNulError<T>(T);
     |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `T` cannot be formatted using `{:?}` because it doesn't implement `Debug`
     |
note: required for `GStringNoTrailingNulError<T>` to implement `Debug`
    --> glib/src/gstring.rs:1300:50
     |
1300 | #[derive(thiserror::Error, Clone, PartialEq, Eq, Debug)]
     |                                                  ^^^^^ unsatisfied trait bound introduced in this `derive` macro
note: required by a bound in `std::error::Error`
    --> /builddir/build/BUILD/rustc-1.72.0-src/library/core/src/error.rs:32:1
     = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider restricting type parameter `T`
     |
1302 | pub struct GStringNoTrailingNulError<T: std::fmt::Debug>(T);
     |                                       +++++++++++++++++

(I've done this after a cargo clean too, suspecting some sort of incremental compliation bug)

@sdroege
Copy link
Member

sdroege commented Sep 15, 2023 •
edited
Loading

Can you try if this also happens with the official rustc 1.72 build? Would seem strange if Fedora's build behaves different in that regard but that's the best I can guess right now.

The error itself doesn't make much sense. The Debug derive macro always expands to something that adds Debug bounds to every single generic parameter (i.e. impl<T: Debug> Debug for GStringNoTrailingNulError<T> etc).

@cgwalters
Copy link
Contributor Author

cgwalters commented Sep 16, 2023 •
edited
Loading

OK, I tried a few things. First, so far I'd been building in my toolbox, which isn't really a hermetic environment at all. My first step was to go to our FCOS "buildroot" container, and run it (mostly) hermetically, just bind mounting in the git working directory:

$ podman run --pull=newer --security-opt=label=disable -v /var/srv/:/var/srv --rm -ti quay.io/coreos-assembler/fcos-buildroot:testing-devel
$ cd /var/srv/src/gtk-rs-core
$ cargo clean
$ cargo build -p lib

Reproduces the error here (and I'm curious if it does for you/anyone else)!

Now, my next step was to dnf -y remove cargo rust in that container, then install rustup and build from that using the upstream binaries. Same error!

I noticed this project also uses a custom container as a buildroot; but what I tried next was to reproduce this in a trivial stock Github Action that just runs cargo check using the Rust toolchain that comes with the default ubuntu runner...and that works fine.

Now here's something interesting...this also reproduces for me with:

$ podman run quay.io/centos/centos:stream9
$ dnf -y install cargo glib2-devel
$ rpm -q cargo 
cargo-1.71.1-1.el9.x86_64
$ cargo check -p glib

So...that's really weird IMO; I'm going to ping our toolchain maintainer...

@sdroege
Copy link
Member

sdroege commented Sep 16, 2023

I'm just running on F38 with rustc from rustup btw, no Rust toolchain installed via packages.

@cgwalters
Copy link
Contributor Author

Ugh of course...the problem was I had a really old Cargo.lock. Duh. Sorry for the noise...

@cgwalters cgwalters closed this Sep 19, 2023
@cgwalters cgwalters deleted the fix-impl-debug branch September 25, 2023 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cgwalters @sdroege