Skip to content

Discuss: not update Cargo.toml minor/patch version? #14962

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

Open
xxchan opened this issue Mar 2, 2025 · 3 comments
Open

Discuss: not update Cargo.toml minor/patch version? #14962

xxchan opened this issue Mar 2, 2025 · 3 comments

Comments

@xxchan
Copy link
Member

xxchan commented Mar 2, 2025

Hi, I see we've checked in Cargo.lock recently #14135, and I think it's good!

But I'm not sure whether this was discussed: what about not updating Cargo.toml (for minor/patch versions), but only Cargo.lock?

From the discussion, I can see the main motivation is to have reproducible build (agains near latest dependencies) in CI. To achieve this, Cargo.lock (updated by bot) is enough.

Whether or not updating Cargo.toml means whether or not force downstream users to use only the latest dependency versions. Personally I prefer a more tolerable version range, so that downstream can update deps 1 by 1 and audit each dep's changes.

FYI in iceberg-rust, we have similar discussions on this topic, and we prefer to have a wider range of versions support, to allow users to choose their dep version (by not updating Cargo.toml too often) https://lists.apache.org/thread/pv3onm41229lovs1odqg94fdc60wcp73

@alamb
Copy link
Contributor

alamb commented Mar 3, 2025

From the discussion, I can see the main motivation is to have reproducible build (agains near latest dependencies) in CI. To achieve this, Cargo.lock (updated by bot) is enough.

Another reason we always increase Cargo.toml versions of submodules is that we don't test DataFusion building with other versions.

For example, if one of the dependencies like arrow releases a new minor version that has a new function, and then we update DataFusion to use that new function, then DataFusion actually really does require the newer version

We don't spend much time

Whether or not updating Cargo.toml means whether or not force downstream users to use only the latest dependency versions. Personally I prefer a more tolerable version range, so that downstream can update deps 1 by 1 and audit each dep's changes.

I think having a wider version range would be valuable and make DataFusion easier to use by downstream projects as well. The key thing is to figure out how to test that the ranges encoded in Cargo.toml are actually accurate

@xxchan
Copy link
Member Author

xxchan commented Mar 3, 2025

There's a flag direct-minimal-versions https://doc.rust-lang.org/cargo/reference/unstable.html#direct-minimal-versions which fits perfectly for minimal version testing,but maybe has some minor problems though rust-lang/cargo#5657 (comment)

In iceberg-rust, we currently use it to do MSRV test together with min version deps test (some deps cannot be built on MSRV in newer toolchain versions )

@alamb
Copy link
Contributor

alamb commented Mar 25, 2025

@logan-keede filed a related ticket / discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants