0xPolygonMiden / miden-vm

STARK-based virtual machine
MIT License
615 stars 152 forks source link

Reproducible (ish) builds #1259

Closed hackaugusto closed 3 weeks ago

hackaugusto commented 5 months ago

Issues:

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

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 commented 5 months ago

@bitwalker suggests:

bitwalker commented 5 months ago

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.

bobbinth commented 5 months ago

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

hackaugusto commented 5 months ago

@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.

bobbinth commented 5 months ago

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

bitwalker commented 5 months ago

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.

bobbinth commented 4 weeks ago

@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?

phklive commented 3 weeks ago

@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.

bobbinth commented 3 weeks ago

Closed by #1415 and others.