topgrade-rs / topgrade

Upgrade all the things
GNU General Public License v3.0
2.08k stars 141 forks source link

Topgrade panics when trying to update nix packages on a pre-release version of nix #956

Open lapkowski opened 1 month ago

lapkowski commented 1 month ago

Erroneous Behavior

Topgrade panics when trying to update nix packages on a pre-release version of nix (in this case 2.25.0pre20240920_ca3fc169) with the following message:

Message:  invalid version: Error("unexpected character 'p' after patch version number")
Location: src/steps/os/unix.rs:455

Expected Behavior

Topgrade should parse the version string with the pre(..) suffix and update the packages.

Steps to reproduce

  1. Install a pre-release version of nix.
  2. Run topgrade to update nix packages.

Possible Cause (Optional)

The nix --version parser passes a version string with the pre(...) suffix to Version::parse, but it needs a x.x.x-pre(...) syntax to parse properly (according to Version::parse documentation)

Problem persists without calling from topgrade

Did you run topgrade through Remote Execution

Additional Details

DEBUG Step "nix"
DEBUG Detected "/home/USER/.nix-profile/bin/nix" as "nix"
DEBUG Detected "/home/USER/.nix-profile/bin/nix-channel" as "nix-channel"
DEBUG Detected "/home/USER/.nix-profile/bin/nix-env" as "nix-env"
DEBUG nix profile: "/home/USER/.nix-profile"

── 09:46:25 - Nix ──────────────────────────────────────────────────────────────
DEBUG Executing command `/home/USER/nix-profile/bin/nix-channel --update`
unpacking 2 channels...
DEBUG Executing command `/home/USER/.nix-profile/bin/nix --version`
The application panicked (crashed).
Message:  invalid version: Error("unexpected character 'p' after patch version number")
Location: src/steps/os/unix.rs:455
SteveLauC commented 1 month ago

Thanks for the bug report and digging possible root cause!

I am thinking maybe we can just trim the prexxx part and pass the remaining part to Version::parse(), then we treat pre-releases just like normal nix🤔

lapkowski commented 1 month ago

I think semver just treats -pre* as a boolean, so it wouldn't make any diffrence, and unix.rs just uses the version string to make sure it is above 2.21 so it should be fine.

SteveLauC commented 1 month ago

I think semver just treats -pre* as a boolean, so it wouldn't make any difference

Get it.