build(lib): do not run build script based on changes to kernel_v0.rs

[eightfilms]

#887 added a build script that builds kernel_v0.rs, but the script runs on changes to the file itself. I suspect this was the reason for test times doubling after that PR as pointed out in #903.

This PR simply removes this rerun check. I doubt this check is the correct behaviour either way, since if it reruns based on changes to the kernel_v0.rs, it means it reruns every time the file is generated - which means the build script is triggered between each invocation of cargo nextest as well.

I've tested this in my personal repo - the run is here: https://github.com/eightfilms/miden-base/actions/runs/12064550221/job/33641593187 and is based on this branch: https://github.com/eightfilms/miden-base/tree/refs/heads/kernel-build-once. There is no longer recompilation between the different test steps, which means we should expect to see CI times for the test step go back down to ~15 minutes from ~40 minutes (which is similar to the last commit prior to #887)

[Mirko-von-Leipzig]

Thank you, I think your diagnosis is correct. I wonder if the kernel errors file should also be removed in a similar fashion.

More generally I think the issue is that we're generating code in the source directory which isn't correct.

[PhilippGackstatter]

Thanks!

We recently had problems with the docs.rs docs not building and the build was failing because of a "read-only" filesystem. This was also because we're writing to src rather than OUT_DIR, e.g.:

We actually should not write into the src directory directly. I think build.rs files should always write into OUT_DIR (which can be written to in docs.rs environment) and then simply be included with something like include!(concat!(env!("OUT_DIR"), "/build_file.rs"));. docs.rs is setup with a read-only src directory because, I guess, it assumes that these best practices are followed. Us having problems with rustfmt and generated files in the past was basically also a symptom of us writing directly into src.
#970 (review)

So not necessarily for this PR, but it seems to me that it would be better to just move to the include approach and not write kernel_v0.rs and tx_kernel_errors.rs directly into src.

I'm curious why the CI times already went down even though the same problem should exist for the println!("cargo:rerun-if-changed={KERNEL_ERRORS_FILE}"); line, which is also built in that build.rs file. Shouldn't that file change roughly whenever the kernel_v0 file changes? (Edit: Ah @Mirko-von-Leipzig pointed this out already :D)

[Mirko-von-Leipzig]

I'm curious why the CI times already went down even though the same problem should exist for the println!("cargo:rerun-if-changed={KERNEL_ERRORS_FILE}"); line, which is also built in that build.rs file. Shouldn't that file change roughly whenever the kernel_v0 file changes? (Edit: Ah @Mirko-von-Leipzig pointed this out already :D)

The error file write is skipped most of the time:

    // Skip this build script in BUILD_KERNEL_ERRORS environment variable is not set to `1`.
    if env::var("BUILD_KERNEL_ERRORS").unwrap_or("0".to_string()) == "1" {
        // Generate kernel error constants.
        generate_kernel_error_constants(&source_dir)?;
    }

@eightfilms could you also remove this ^^ variable from the rerun-if pragmas? This would wrap this PR up nicely, and we can go bikeshed internally over the dangers of generating code in-tree.

[eightfilms]

I'm curious why the CI times already went down even though the same problem should exist for the println!("cargo:rerun-if-changed={KERNEL_ERRORS_FILE}"); line, which is also built in that build.rs file. Shouldn't that file change roughly whenever the kernel_v0 file changes? (Edit: Ah @Mirko-von-Leipzig pointed this out already :D)

Probably because generating that file is gated behind an environment variable: https://github.com/0xPolygonMiden/miden-base/blob/main/miden-lib/build.rs#L80C5-L83C6

Regardless, +1 on the sentiment that writing into src is definitely not the best practice. We should definitely have a follow up PR that fixes this.

Edit: got frontrun by @Mirko-von-Leipzig :) just saw your comment, will do once I'm back at the computer

[eightfilms]

I'm curious why the CI times already went down even though the same problem should exist for the println!("cargo:rerun-if-changed={KERNEL_ERRORS_FILE}"); line, which is also built in that build.rs file. Shouldn't that file change roughly whenever the kernel_v0 file changes? (Edit: Ah @Mirko-von-Leipzig pointed this out already :D)

The error file write is skipped most of the time:

    // Skip this build script in BUILD_KERNEL_ERRORS environment variable is not set to `1`.
    if env::var("BUILD_KERNEL_ERRORS").unwrap_or("0".to_string()) == "1" {
        // Generate kernel error constants.
        generate_kernel_error_constants(&source_dir)?;
    }

@eightfilms could you also remove this ^^ variable from the rerun-if pragmas? This would wrap this PR up nicely, and we can go bikeshed internally over the dangers of generating code in-tree.

done

[Mirko-von-Leipzig]

Thanks again!