Concordium / concordium-node

The main concordium node implementation.
GNU Affero General Public License v3.0
45 stars 22 forks source link

Builds are not reproducible #109

Closed bisgardo closed 3 years ago

bisgardo commented 3 years ago

The following .gitignore rules currently exclude a number of dependency management lock files from the repo:

The files record the resolved versions of all libraries used in the build. Without these files, builds are not reproducible.

It's not clear to me which exact lock files should be included, but it seems that at least concordium-node/Cargo.lock and concordium-base/rust-src/Cargo.lock (excluded in concordium-base) should:

For example, the most recently tagged commit (1.0.1-0) no longer builds with Rust 1.45.2. The reason is the dependency chain

concordium_node v1.0.1
-> crypto_common v0.1.0
-> ed25519-dalek v1.0.1
-> curve25519-dalek v3.1.0
-> zeroize v1.4.0

The newly released v1.4.0 of zeroize requires Rust 1.51+. As curve25519-dalek depends on the library as just version "1", it automatically bumps the minor version which raises the minimum supported Rust version (they explicitly allow themselves to do that).

Had Cargo.lock been checked in, this would not have been an issue.

bisgardo commented 3 years ago

See https://github.com/bisgardo/concordium-docker/commit/5756ed77f3cd7de8289c7a6141459bdd664a5cad for a workaround of the particular example issue.

abizjak commented 3 years ago

Cargo.lock files should only be included if the crate produces a final artifact. Otherwise they are ignored (for good reasons). A final artifact for us would be a binary or a shared library or a .a archive. For example, we include it in wasm-chain-integration but not in wasm-transform, as a result.

Since the entire rust-src is a single workspace, and that is how it is built when it is linked with Haskell, a single Cargo.lock file there will suffice.

The other shared libraries we produce for linking, wasm-chain-integration, already have a lock file.

Also note that the .dockerignore file lists Cargo.lock.

stack.yaml.lock, while they probably would not hurt, should in practice not be necessary though, since we use LTS resolvers which (in practice) do not change the versions of bundled libraries.