greshake / i3status-rust

Very resourcefriendly and feature-rich replacement for i3status, written in pure Rust
GNU General Public License v3.0
2.88k stars 475 forks source link

pacman block displaying 18446744073709551615 updates ... #190

Closed yawpitch closed 6 years ago

yawpitch commented 6 years ago

Still learning rust, but cannot see an obvious explanation for this.

After updating pacman itself to v5.1.0, the pacman block now displays 2^64-1 required updates when pacman -Su -p shows no updates required, and 0 updates required when I've downgraded a package and pacman -Su -p suggests an upgrade.

I've tried wiping out the tmp db, and running the command manually, to no avail.

greshake commented 6 years ago

Can you build i3status rust with cargo build --debug and try again? If it is an issue with the shell command used internally, it will still show the incorrect number. If it is an integer overflow on this part:

{...}.lines()
.count() - 1

it should panic and crash.

(Rust only checks for integer over/underflow in debug compilation mode, it wraps silently in release mode)

yawpitch commented 6 years ago

With both stable and nightly of 1.26.1 I don't see a --debug option to cargo build, and it's also not an option on the executable ... but cargo run does indicate an underflow.

$ cargo run -- ~/.config/i3status-rs/config.toml 
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target/debug/i3status-rs /home/michael/.config/i3status-rs/config.toml`
{"version": 1, "click_events": true}
thread 'main' panicked at 'attempt to subtract with overflow', src/blocks/pacman.rs:121:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Which makes sense ... it's only when there's no output from the wrapped command that the underflow occurs. Also today I realized it's under-reporting by one ... if two updates are shows with pacman -Su -p then the block displays 1.

atheriel commented 6 years ago

I think this should be connected to the discussion in #179 on moving to checkupdates for the pacman block. That would be a good opportunity to try and prevent this kind of bug.

yawpitch commented 6 years ago

I've mentioned in #179 that checkupdates is not available by default on Arch, as the pacman-contrib package is apparently not a dependency of any of the base packages.

Under the hood the only difference in what checkupdates does from the current block is use pacman -Qu for the final call ... and therein lies the bug.

In pacman < 5.1.0 the pacman -Su -p call would print a ":: Starting full system upgrade..." line before listing out all the packages to upgrade, hence why the -1 was introduced. In 5.1.0 that extra print has been removed from the output, and thus the underflow.

The pacman -Qu command did not print that extra line on either side of the pacman version shift, so simply changing to that and counting the lines that actually appear should work for all.