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

Buildable release builds #14479

Open
findepi opened this issue Feb 4, 2025 · 7 comments
Open

Buildable release builds #14479

findepi opened this issue Feb 4, 2025 · 7 comments

Comments

@findepi
Copy link
Member

findepi commented Feb 4, 2025

Problem description

From security standpoint it would be great to have reproducible (byte-for-byte) release builds, however this issue is not about this (but is a prerequisite thereof).
DataFusion downstream consumers consume DataFusion source code, via crates or forking.

Im both these situations it would be great to guarantee that a given release actually builds, not only at the moment of the release.

Observed

A fork of 43.0.0 release no longer passes tests today: https://github.com/findepi/datafusion/actions/runs/13130900468
A fork of 44.0.0 release no longer passes clippy today: https://github.com/findepi/datafusion/actions/runs/13132627770

Expected

A release compiles and works correctly (including passing test). Ideally it also passes clippy.
This is beneficial to anyone consuming a DataFusion code. People running a permanent fork likely mostly figured how to solve this already, but that would be very beneficial to anyone running a particular DataFusion version and wanting to fix a bug. Being able to check out code for the version they use and reproduce the bug there is really good first step, but this requires that the code works.

@findepi
Copy link
Member Author

findepi commented Feb 4, 2025

Proposed solution: commit Cargo.lock to the repository.
Lack of Cargo.lock in the repo gives some benefit (checking with latest versions of unpinned deps automatically). This can be provided with a CI job. I.e. make this a machine problem, not a human (consumer) problem.

@mbrobbel
Copy link
Contributor

mbrobbel commented Feb 4, 2025

Proposed solution: commit Cargo.lock to the repository. Lack of Cargo.lock in the repo gives some benefit (checking with latest versions of unpinned deps automatically). This can be provided with a CI job. I.e. make this a machine problem, not a human (consumer) problem.

#14135

@alamb
Copy link
Contributor

alamb commented Feb 4, 2025

As @mbrobbel says, I agree it is time to put Cargo.lock int he repo:

@findepi
Copy link
Member Author

findepi commented Feb 4, 2025

Great. This probably would solve the biggest issue with re-buildable releases.
Some other stuff of similar kind is using pinned vs latest stable rust version.

@alamb
Copy link
Contributor

alamb commented Feb 5, 2025

Some other stuff of similar kind is using pinned vs latest stable rust version.

I think this can be set using a rust-toolchain.toml file, for example https://github.com/influxdata/influxdb/blob/main/rust-toolchain.toml

[toolchain]
channel = "1.84.0"
components = ["rustfmt", "clippy", "rust-analyzer"]

@findepi
Copy link
Member Author

findepi commented Feb 5, 2025

Let's do this. we upgrade compiler version manually anyway, so we can update one more place i believe.

@alamb
Copy link
Contributor

alamb commented Feb 7, 2025

Let's do this. we upgrade compiler version manually anyway, so we can update one more place i believe.

It would help us in InfluxDB too as it we have a patched version of DataFusion we keep around too.

I'll try and make a PR for it over the next few days if no one beats me to it

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

3 participants