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.

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

Switch from dialoguer to inquire #511

Closed
simbleau opened this issue Mar 9, 2024 · 5 comments · Fixed by #512
Closed

Switch from dialoguer to inquire #511

simbleau opened this issue Mar 9, 2024 · 5 comments · Fixed by #512
Labels
good first issue Good for newcomers O-MacOS Applicable specifically to the MacOS target

Comments

@simbleau
Copy link
Member

simbleau commented Mar 9, 2024

I can't compile vello anymore on 1.76 stable for very strange reasons related to lazy_static, see console-rs/dialoguer#306

dialoguer is less maintained, less stars, etc. than inquire, and dialoguer never made the change to once_cell and I think in recent versions of rust (1.75+?) a dep with lifetimes in lazy_static are no longer producing macro-expansion that is valid.

The issue is simple... replace dialoguer with inquire. Should take 5 minutes, but I wanted to explain the purpose here.

We only use dialoguer on native to assist with downloads, which we could easily replace with the "Confirm" equivalent on inquire: https://github.com/mikaelmello/inquire?tab=readme-ov-file#Confirm

@simbleau simbleau added good first issue Good for newcomers O-MacOS Applicable specifically to the MacOS target labels Mar 9, 2024
@xStrom
Copy link
Member

xStrom commented Mar 9, 2024

What cargo command do you use that fails? I tried both cargo build -p vello and cargo build -p scenes on my macOS 10.15 with Rust 1.76 and it succeeded. The build also succeeds in the GitHub CI with both macOS 12 and macOS 14 on M1.

Have you tried cargo clean ?

@simbleau
Copy link
Member Author

simbleau commented Mar 9, 2024

Same specs, M1, 1.76 stable. I did a cargo clean. It's hard for me to figure out.

Vello runs, it's just the with_winit example that didn't. Not sure if that's CI tested. It's also new behavior... not sure when exactly or what caused it.

Either way, I don't think there's a good reason not to switch. :)

@xStrom
Copy link
Member

xStrom commented Mar 9, 2024

Vello runs, it's just the with_winit example that didn't.

Does with_winit still not run? What is the cargo command that fails?

@simbleau
Copy link
Member Author

simbleau commented Mar 9, 2024

It happens when compiling console, which is a dependant of dialoguer. The error seems unhelpful...

16:27 simbleau on main ~ cargo run -p with_winit
    Updating crates.io index
  Downloaded skrifa v0.16.0
  Downloaded read-fonts v0.16.0
  Downloaded 2 crates (385.7 KB) in 1.01s
    Blocking waiting for file lock on package cache
   Compiling bitflags v2.4.2
   Compiling libc v0.2.153
   Compiling font-types v0.4.3
   Compiling core-graphics-types v0.1.3
   
   ... lots of crates
   
   Compiling rand_chacha v0.3.1
   Compiling fsevent-sys v4.1.0
   Compiling console v0.15.8            <--- This right here officer
   Compiling filetime v0.2.23
error[E0195]: lifetime parameters or bounds on type `Target` do not match the trait declaration
  --> /Users/simbleau/.cargo/registry/src/index.crates.io-6f17d22bba15001f/console-0.15.8/src/utils.rs:25:1
   |
25 | / lazy_static! {
26 | |     static ref STDOUT_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stdout()));
27 | |     static ref STDERR_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stderr()));
28 | | }
   | |_^ lifetimes do not match type in trait
   |
   = note: this error originates in the macro `__lazy_static_internal` which comes from the expansion of the macro `lazy_static` (in Nightly builds, run with -Z macro-backtrace for more info)

   Compiling rand v0.8.5
error[E0599]: no method named `load` found for struct `STDOUT_COLORS` in the current scope
  --> /Users/simbleau/.cargo/registry/src/index.crates.io-6f17d22bba15001f/console-0.15.8/src/utils.rs:39:19
   |
25 | / lazy_static! {
26 | |     static ref STDOUT_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stdout()));
27 | |     static ref STDERR_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stderr()));
28 | | }
   | |_- method `load` not found for this struct
...
39 |       STDOUT_COLORS.load(Ordering::Relaxed)
   |                     ^^^^ method not found in `STDOUT_COLORS`

error[E0599]: no method named `store` found for struct `STDOUT_COLORS` in the current scope
  --> /Users/simbleau/.cargo/registry/src/index.crates.io-6f17d22bba15001f/console-0.15.8/src/utils.rs:48:19
   |
25 | / lazy_static! {
26 | |     static ref STDOUT_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stdout()));
27 | |     static ref STDERR_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stderr()));
28 | | }
   | |_- method `store` not found for this struct
...
48 |       STDOUT_COLORS.store(val, Ordering::Relaxed)
   |                     ^^^^^ method not found in `STDOUT_COLORS`

error[E0599]: no method named `load` found for struct `STDERR_COLORS` in the current scope
  --> /Users/simbleau/.cargo/registry/src/index.crates.io-6f17d22bba15001f/console-0.15.8/src/utils.rs:60:19
   |
25 | / lazy_static! {
26 | |     static ref STDOUT_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stdout()));
27 | |     static ref STDERR_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stderr()));
28 | | }
   | |_- method `load` not found for this struct
...
60 |       STDERR_COLORS.load(Ordering::Relaxed)
   |                     ^^^^ method not found in `STDERR_COLORS`

error[E0599]: no method named `store` found for struct `STDERR_COLORS` in the current scope
  --> /Users/simbleau/.cargo/registry/src/index.crates.io-6f17d22bba15001f/console-0.15.8/src/utils.rs:69:19
   |
25 | / lazy_static! {
26 | |     static ref STDOUT_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stdout()));
27 | |     static ref STDERR_COLORS: AtomicBool = AtomicBool::new(default_colors_enabled(&Term::stderr()));
28 | | }
   | |_- method `store` not found for this struct
...
69 |       STDERR_COLORS.store(val, Ordering::Relaxed)
   |                     ^^^^^ method not found in `STDERR_COLORS`

Some errors have detailed explanations: E0195, E0599.
For more information about an error, try `rustc --explain E0195`.
error: could not compile `console` (lib) due to 6 previous errors
warning: build failed, waiting for other jobs to finish...

@xStrom
Copy link
Member

xStrom commented Mar 10, 2024

Very curious indeed.

I have reasonable confidence that this is not a general problem with the source code, as evidenced by it compiling fine on a bunch of macOS systems.

Smells more like a Rust toolchain bug, maybe somehow related to the new parallelism work, in which case the manifestation may depend on a certain number of threads being used.

Sure we can "fix" it by merging #512 but we can easily end up depending on console again via some dependency in the future. Even worse if this is as more generic toolchain bug that can start causing issues elsewhere. This time it resulted in a compilation error, but other times it might just result in buggy machine instructions that can silently corrupt data at runtime. Not great, but also not easy to track down. Luckily there are a bunch of other people seeing the same issue, so hopefully someone will put in the effort to figure it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers O-MacOS Applicable specifically to the MacOS target
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants