Keeping generating files in sync with the source code

Help
1

In TigerBeetle’s repo, we have a tb_client.h C header file which describes our soon-intended-to-be-stable ABI for clients. This file is curious because:

  • it is actually automatically generated from our C source code
  • at the same time, it is very much intentionally a part of the repository. Downstream projects should be able to include this header without necessary building TigerBeetle themselves, and this file is intended to be read by human. We generate readable C.

So, we have an invariant that, at all times, .h file should match the .zig file.

What’s the best way to achieve this with build.zig? My first idea is to use a test here, because tests is what you use to check invariants in code. But there’s an issue with “just a test”: although the .h file is generated, this file is also an input to some further build steps (for example, to the step which builds a c example using this header). So ideall, when I run zig build c_sample, I’d get this .h file re-generated automatically. What’s the best pattern to handle this?

2

The std.Build.Step.Run type actually has a nice caching mechanism for this type of thing. So long as you don’t set “has_side_effects” to true, it will take all the command-line arguments and hash them to determine if/when it needs to be rebuilt.

I have an example of this: zigwin32gen/build.zig at main · marlersoft/zigwin32gen · GitHub. This project parses a set of JSON files that describe the win32 API. However, it is split into 2 passes. The first pass will parse the json files and then output an extra pass1.json file (analagous to the tb_client.h file you generate):

    const pass1_out_file = blk: {
        const pass1_exe = b.addExecutable(.{
            .name = "pass1",
            .root_source_file = b.path("src/pass1.zig" ),
            .optimize = optimize,
            .target = b.host,
        });

        const run = b.addRunArtifact(pass1_exe);
        run.addDirectoryArg(win32json_dep.path(""));
        const out_file = run.addOutputFileArg("pass1.json");

        pass1_step.dependOn(&run.step);
        break :blk out_file;
    };

The key parts here are the 2 arguments we add to the run step. We add a directory arg and a file arg. For directories, the run step will only hash the name, but in this case that’s fine because win32json_dep.path("") resolves to a hashed dependency in the global cache so if it changes, then it’s path will also change. Secondly, we add an output file argument which returns a LazyPath. Further down the file we pass this LazyPath to the second executable genzig, which automatically causes genzig to depend on pass1 through this generated file.

So long as all our other build steps use the lazy path, they’ll automatically depend on the step that generates it.

2 Likes
3

Aww I remember tigerbeetle is open source :slight_smile: Here’s what I would do: build: use LazyPath for tb_client.h by marler8997 · Pull Request #2044 · tigerbeetle/tigerbeetle · GitHub

1 Like
4

Yeah, that’s definitely much better than what we’ve been using before, thanks!

Though, I think it’s still not quite at 100% — the new LazyPath approach guarantees that the file is appropriately refreshed, but I don’t think it ensures that the committed version of the file is kept fresh?

That is, if someone updates .zig, but doesn’t run anything that would update .h and commit the code, the .h file in the repo would be wrong. I think this can be brute-forced by adding another step to zig build test dependencies, which would fail if the file isn’t fresh, but that means writing more code. I wonder if there’s some more elegant solution here which solves both problems (dependency tracking & guaranteeing that the committed code is fresh) at the same time?

1 Like
5

What do you see as the pain point here? Adding a header generation step and then using b.dependsOn to link it to the associated build stage seems basically correct to me.

What about combining them into a single ‘verb’? Like b.onChange(track: *Compile, do: *Run)? That isn’t maximally fine-grained, but there should be a way for it to cooperate with caching so that the *Run step does the checking to see if the specific file it cares about has updated.

6

There are two things I need:

  1. refresh the generated file if the source changes and a build step, which depends on the generated file, is run
  2. make sure that main branch contains only fresh files all the time

Using a build steps and setting up dependencies solves 1, but not 2.

Using a test solves 2, but not 1.

Using both means I need to write some duplicate code to hook the same logic both with tests and with normal build steps

7

The part I don’t understand is why a generated file is part of the source and committed with it, seems like that should be seen as part of the build and thus not committed.

Do I misunderstand and there is some other reason / is this somehow different from an intermediate build result?

8

What’s the specific failure mode that concerns you? Is it someone updating a file but not building that file, and then committing the change to the repo? That can be handled with a pre-commit hook. Is it something else?

Or are you looking for something more eloquent that can express e.g. “all builds must check the freshness of the header” without hooking the dependencies up manually?

I don’t want to just say “adding an extra line of b.dependsOn for each type of build is fine, actually”, but this question would benefit from an illustration of what the failure mode is that you’re looking to prevent. “the user does this, and forgets to do this, and then [undesirable thing] happens” sort of analysis.

9

The generated header file is part of the source code because that’s our public API. It is intended to be viewed by other people. The fact that we generate, rather than hand-write this file is an implementation detail.

1 Like
10

This gets philosophical, but I believe very strongly in not rocket science rule of software engineering: that, at all times, code on the main branch satisfies certain correctness invariants. Not rocket science rule requires two components:

  • a suite of automated checks, which, when run, tell if there are any invariant violations.
  • a CI system which only advanced the main branch to a new commit once that commit hash is known to be good.

pre-commit just doesn’t cut here — it’s a client side check, it tells something about your local environment but can’t enforce global invariants.

So the question is how to hook this check to the standard code-invariant-checking process: zig build test.

11

That’s why I was asking about scenarios, because if we look at the full quote:

This is a scenario where the build didn’t run, so no amount of zig build will prevent it.

So that’s not the concern here, I’m still trying to figure out what is.

When you say “check”, what’s being checked? What’s the failure mode which needs to be prevented?

If we’re talking about one generated file, then you can ensure that it’s generated before a build by making all the builds depend on the generation step. It seems reasonable to take it as a given that the dependency graph in the build.zig file will function correctly, so I don’t think that’s what you’re getting at either.

If the CI runs the tests (which clearly it will in a no-rocket-science scenario), and the tests depend on the header-generation step, then what’s left? Would a git status check to make sure nothing changed, as part of the merge system, be effective?

I’m still trying to understand what you see as missing here. It still looks like making the header generation a dependency of subsequent steps would work, but you clearly don’t agree with that, so there’s something I’m not getting.

12

This alone won’t work, as this doesn’t guarantee generated file freshness.

This gets closer, but somewhat falls apart if you look at details. One solution is to add git status to the CI script, but that’s bad, because CI should be just zig build test: encoding logic into CI scripts is bad, because it makes is non-deniable and non-reproducible outside of specific CI system.

So the next idea is to hook status checks to the tests, but that obviously is a poor experience for local development: only status of the synced generated files should be checked.

You can check status of specific files, but at that point it’s easier to make generation step conditional on either it should fail or not if the file is not fresh, hook failing version into tests and non-failing version into other steps, and that is the code duplication I am trying to find a smarter way around :slight_smile:

1 Like
13

Ok, thanks, I have a much clearer picture of the goal here. It isn’t “ensure that builds properly generate headers when the dependent file is changed” but more “have a build step which fails if the header changes”, correct?

Should that actually be zig build test? I would personally want that to generate a fresh header if changes required it, I read that command as build-then-test and it tends to be the main way I build things when I’m developing. But the idea of having a build step which fails if a file changes doesn’t really depend on what that build step is called, so that isn’t really central.

So just for clarity, let’s postulate two steps here, zig build CI and zig build test, where the difference is that zig build test does all the build steps (including generating new headers when necessary), then tests, and zig build CI has checks to confirm repository-specific invariants: it will fail if the header file changes.

Because if I’m reading you correctly, the failure mode would in fact be something like: write new code which changes the header, commit, then build-and-test works, but since it doesn’t change the header, the commit which is being tested is in fact invalid.

That shouldn’t require code duplication, I don’t think. It’s an extra step between building and testing. That doesn’t really change if the goal is for every test run to fail if the header changes, maybe that’s the right workflow for your project, in that case we’re talking about zig build and zig build test.

Either way, it looks to me like the missing piece here is a build conditional like b.onChange, which triggers a *Run action only if the resource at *LazyPath changes. Which should probably return a *Step, so that it can be woven into dependency chains.

Would that be enough? It seems like a general enough mechanism to be worth adding to the build system, since it already knows about artifacts changing. It also avoids the build system having to know things about version control, which is something to avoid.

14

For posterity here’s my solutions to the problem.

The first solution is simple (enforce generated file synchronization by marler8997 · Pull Request #2048 · tigerbeetle/tigerbeetle · GitHub). Simply add the command git diff --exit-code at the end of every CI workflow to ensure that no files tracked by git have changed. Since zig build test is setup to regenerate the headers when they are out-of-date, this check will catch all generated files that weren’t updated in the commit being tested. This solution is great at catching errors but only catches them at the last possible moment and requires testing after you commit. It also depends on “git”. It would also be nice to be able to detect errors if you just had an archive snapshot.

With this I came up with a second solution (build: add -Denforce-no-changes option by marler8997 · Pull Request #2052 · tigerbeetle/tigerbeetle · GitHub). This one adds the build option -Denforce-no-changes which is also enabled by default if we see the GitHub action CI=true environment variable. This option will cause the build to only verify whether generated files are already up-to-date rather than re-generating them. The implementation for this was simple because of the way I had implemented generating the files. We generate them to zig-cache and then use a custom “install step” to copy them to the various locations inside the Git repository. So the change was simply to update the custom install step to verify instead of install when this build option is present.

5 Likes
15

Clever stuff!

The patch is straightforward enough. Given the screen shot, it functions correctly, as well. With the current tools available, this approach is probably optimal.

I remain curious what sort of minimal and general actions can be added to the build system to enable this kind of dependency flow. The essence of a build system is “detect changes to files and take action accordingly”, after all. So opening the file in a user function and comparing its contents is a tiny inner-system, it duplicates something the build system already knows how to do.

I’m musing here about how to a) generalize the idea and b) make it bulletproof by expressing it in the build graph.

So what are the hazards here? One is that this technique relies on a staging area, for a couple reasons: you need the canonical version of the header to check against, and (generally, not necessarily here) we want to both fail on change and not mutate the program by clobbering the old version with the new one. This version doesn’t mutate the header file before it fails, but since CI throws away all the work, it would be acceptable if it did. But this isn’t always the case.

In fact we’ve identified a general hazard with build.zig. The build system doesn’t just produce out-of-band artifacts, it’s also capable of changing the project itself. This feature is essential, but now we have a distinct class of problem: we want to be able to use the build system to make an up-to-date version of the project, and test it, but in general that means installing mutated files where they belong, and if that fails, the project is left in an inconsistent state.

Imagine that, instead of failure being defined by any change to the header, it’s defined by an artifact built with that header failing: we need that header to provide an API, and it doesn’t. Detecting that involves installing it.

Often this can be repaired with git reset --hard HEAD but a) that’s using revision control to clean up and b) no one likes to type that string, for obvious reasons! I, for one, frequently run builds without having committed the chances I’m about to test.

The documentation is aware that this is a dangerous situation, here’s the quote:

Be careful with this functionality; it should not be used during the normal build process, but as a utility run by a developer with intention to update source files, which will then be committed to version control. If it is done during the normal build process, it will cause caching and concurrency bugs.

So I’m seeing a couple avenues for improvement. Keep in mind that I’ve read the current documentation, and spent some time with Build.zig, but the build system remains under-documented, and I’m not 100% sure what it can and can’t do right now.

The first level is providing a way to create an action dependent on a tracked file changing during the build process. The build system natively uses a staging area, so that would allow us to let the build system figure out whether a new artifact is the same as an old one, and give users an opportunity to insert an intermediate step. This is the one where I’m not confident that such an affordance doesn’t already exist.

For the motivating problem, that would eliminate all (I think) of the custom code. An onChange step could be made a dependency of the install step, and if the build is CI then it fails.

This works whenever the old and new files can be statically compared to determine if an invariant is violated. But that’s not always or even usually the case.

So the second thing to consider is adding a transaction concept to the build. As in databases generally, in the absence of an explicit transaction, every change is considered to be one. In this context, that means that an install step just installs the new version in the way it does now.

But if a step defines a transaction, then the old version of everything is saved to the cache before new versions are installed, and the build script has to either commit or rollback. Given that the build system creates a dependency graph, it should be possible to make it a load error to create a dependency graph where any transaction is not either committed or rolled back: load error meaning that build.zig will fail before taking any action if the graph doesn’t have the correct shape. Close enough to a compile error, given comptime it probably can be a compile error.

That would cover all the bases. The build script can make arbitrary changes to the state of the project (as it can now) and also guarantee that if those changes don’t pass muster, they won’t be visible in the final state of the project at exit. The only failure mode left is that file systems aren’t actually a database, so the rollback can itself fail under rare circumstance, but that can be loudly and prominently displayed to the user if and when it happens. I’m not sure how far it’s appropriate to take this, in terms of chasing the long tail of reliability, but I want to note that for rollback failure, the cache would contain everything it needs to try the rollback again. I’d hazard that when we’re talking about actual filesystem / syscall failures like this, it’s ok to clean up out-of-band, as long as the problem is cleanly reported.

Something to consider! This would generalize quite nicely: consider downloading nightlies of some dependency, and conditionally installing them in order to test the project against the new version, such that only passing versions actually get installed.

16

In terms of what’s available now, there’s a CheckFile Step, which could substitute for some of the custom code in the current solution.