miden-vm icon indicating copy to clipboard operation
miden-vm copied to clipboard

Reproducible (ish) builds

Open hackaugusto opened this issue 11 months ago • 6 comments

Issues:

  • CI uses unpinned dependencies nightly versions
  • CI uses unpinned library versions
  • There isn't an easy way to reproduce the CI steps locally

The points above means that it is cumbersome to reproduce the CI build

  • One has to look at the CI logs to find the rustc/cargo/rustup versions to re-use
  • In theory, one would need to copy each individual library version by hand to reproduce the build
  • One has to read the ci.yaml file and copy/past the commands

Usually the steps above would be done as a last resort. But when it happens it takes a lot of time to reproduce CI issues and fix them.

This issue is on how to make the above steps more reproducible, so that it takes less effort to maintain the CI.

Discord conversation: https://discord.com/channels/893151281848406016/1148602660132487279/1211740187177001080

hackaugusto avatar Feb 26 '24 18:02 hackaugusto

@bitwalker suggests:

  • Add a rust-toolchain.toml file to pin the uses versions of the compiler
  • Commit the Cargo.lock file to pin the versions of the dependencies used by CI
  • Using cargo make to expose the CI scripts. We would use the make rules in the CI and locally

hackaugusto avatar Feb 26 '24 18:02 hackaugusto

See here for details on what you can do with cargo make. We use it in the compiler, so you can also reference our Makefile.toml and GitHub Actions jobs to see how we use it.

We use rust-toolchain.toml to set our default toolchain to the nightly toolchain (and we currently are only building nightly, but this will be changing soon to return to stable). In other projects I've used cargo make to choose a toolchain configuration based on the build profile, see this as an example.

But in general, I think everything @hackaugusto laid out is good practice and will remove a lot of friction related to CI vs local build discrepancies.

bitwalker avatar Feb 26 '24 18:02 bitwalker

We should probably do something similar for other repos too (crypto, base, node, client).

bobbinth avatar Feb 26 '24 19:02 bobbinth

@bobbinth totally agree. I opened this issue to get feedback and document the discord conversation. Collecting any objections and/or tool preferences. If there aren't any, I can replicate the issue on all repos.

hackaugusto avatar Feb 26 '24 20:02 hackaugusto

Yep - makes sense. One question: in some repos we already have Makefiles - would these be replaced with cargo make?

bobbinth avatar Feb 27 '24 07:02 bobbinth

Yep - makes sense. One question: in some repos we already have Makefiles - would these be replaced with cargo make?

That would be my recommendation, there's no need for both, but you could have the Makefile have a single target that installs the cargo-make extension and then dispatches the requested task to cargo make and also prints a warning so the user knows they shouldn't be using it. Not sure if that's worth it though, a note in the dev docs seems sufficient to me.

bitwalker avatar Feb 27 '24 09:02 bitwalker

@phklive - I think this is mostly done, right? Maybe one thing that is missing is that we should commit the Cargo.lock file. Could you take a look and see what's needed to close this issue?

bobbinth avatar Jul 24 '24 08:07 bobbinth

@phklive - I think this is mostly done, right? Maybe one thing that is missing is that we should commit the Cargo.lock file. Could you take a look and see what's needed to close this issue?

Indeed I believe that this was one of the last things standing.

Added the needed changes to close this issue in this PR: #1415

Once merged I believe that this issue can be closed.

phklive avatar Jul 26 '24 12:07 phklive

Closed by #1415 and others.

bobbinth avatar Jul 30 '24 16:07 bobbinth