Tighten bignum dependency versions

[mzabaluev]

Fixes #4477

As bigdecimal API exposes implementations of traits defined in a particular version of num-bigint, it's fragile to have loosely ranged versions on these dependencies. Lock both down to the currently latest ^0.4, that's consistent with how bigdecimal 0.4.x depends on num-bigint.

[weiznich]

Thanks for opening this PR. I'm sorry but I fear we won't be able to accept this PR as this is a breaking change as types from these crates appear in our public API and this removes support for certain versions of these crates.

[mzabaluev]

types from these crates appear in our public API

Doesn't this essentially mean that your public API (its part gated with "numeric") is currently unstable and may break when an unrelated crate in the consuming namespace is updated to pull a particular version of num-bigint that also fits in the range?

[weiznich]

Doesn't this essentially mean that your public API (its part gated with "numeric") is currently unstable and may break when an unrelated crate in the consuming namespace is updated to pull a particular version of num-bigint that also fits in the range?

No that's not what this means and thats also not what semver requires here. The only requirement for a stable API is that the code continues to compile if you use the same version as before. If you somehow change versions on your side that's not a problem in diesel as you always can control which version of a certain crate is used by adjusting your local Cargo.lock file, e.g by using cargo update to get exactly the right dependency versions there.

The default behavior depends on cargo, which again is nothing diesel controls. If you believe that this should be handled differently you should raise this to the cargo team.

[mzabaluev]

If you somehow change versions on your side that's not a problem in diesel as you always can control which version of a certain crate is used by adjusting your local Cargo.lock file, e.g by using cargo update to get exactly the right dependency versions there.

In movementlabsxyz/aptos-core#132 we were only able to fix this (in the short term) by downgrading bigdecimal to ^0.3, which is not desirable.

The default behavior depends on cargo, which again is nothing diesel controls.

Cargo gives you what you allow it. Your cargo manifest claims "this crate can be used with any combination of num-bigint and bigdecimal versions in the given ranges". Since bigdecimal actually depends on a narrower version range of num-bigint, this claim is false. #4477 is a valid bug and should not have been closed.

[weiznich]

In movementlabsxyz/aptos-core#132 we were only able to fix this (in the short term) by downgrading bigdecimal to ^0.3, which is not desirable.

I highly doubt that this is the only way to fix that. As your repo seems to be releated to web3/blockchain stuff and I highly disapprove that technology I won't provide further support on fixing that, other than stating: You can fix that by correcting the version used in your Cargo.lock file.

Cargo gives you what you allow it. Your cargo manifest claims "this crate can be used with any combination of num-bigint and bigdecimal versions in the given ranges".

Given that there is no other way to express this, yes that's what we are saying there. If you want to see that fixed, please work with the cargo team to get us a better way to express what we really want to express there. There is literally nothing in diesel we can do to address this issue without support in cargo itself. Not even changing how num-bigint and bigdecimal interact would help as that again would require increasing the supported minimal version, which again would require a semver major version bump.

#4477 is a valid bug and should not have been closed.

Bugs in our issue tracker should always represent something actionable where people could start working on it. This is not actionable as there is no clear way to fix this, therefore it's closed as won't fix.

As written above: I'm happy to accept a fix that doesn't require a major semver version bump. That's not the case for your presented fix.

That's likely my last comment on this topic.