ConfigHeader silently swallows invalid values

[the-argus]

Zig Version

0.11.0

Steps to Reproduce and Observed Output

Make a folder with the following three files:

build.zig:

const std = @import("std");

pub fn build(b: *std.Build) void {
    const dummy_lib = b.addStaticLibrary(.{
        .target = b.standardTargetOptions(.{}),
        .optimize = b.standardOptimizeOption(.{}),
        .name = "dummy",
    });

    dummy_lib.addCSourceFiles(&.{"main.c"}, &.{});

    const exported_h = b.addConfigHeader(
        .{ .style = .{ .cmake = .{ .path = "config.h" } }, .include_path = "export.h" },
        .{
            .EXPORT = void{},
            .PLATFORM_NAME = "PLATFORM_WHATEVER",
        },
    );

    dummy_lib.addConfigHeader(exported_h);
    b.installArtifact(dummy_lib);
}

main.c:

#include <export.h>

#ifndef USING_PLATFORM_WHATEVER
#error "no platform"
#endif

int EXPORT nothing() {
  static int i = 0;
  return ++i;
}

config.h:

#pragma once
// clang-format off
#define USING_@PLATFORM_NAME@
// clang-format on

then run zig build.

Expected Output

Either an error message saying a value was not present for replacement in the config header, or that additional values would be appended onto the end of the header using the same generator as .blank.

[MrDmitry]

Looking at CMake behavior, they do not identify that specific case themselves, as they expose all of CMake variables (as well as env vars and caches, I believe) to the variable expansion.

I don't think zig needs to be restricted by CMake's behavior though. I could add a .strict option for cmake style that would enforce additional checking, as well as a backwards check - when variable is not defined in zig but used by the header.

[MrDmitry]

I have a prototype implementation under https://github.com/ziglang/zig/compare/master...MrDmitry:zig:feat/cmake_config_validator

I'm not a fan of it it for 3 reasons:

1. it's utilizing dynamic dispatch, I was not able to figure out how to turn it into a static dispatch
2. it's a breaking change, but maybe the validator should be a general configuration (and be added where makes sense), at least it would not break existing code
3. step reporting is not handled by the validator itself, although I could move it over

I'm also not entirely sure about subverting the error processing. It's somewhat limited by the error types being explicit errors from this file, but it just feels wrong to suppress errors for the default validator.

Here's an example of an undefined variable error reporting (the use case from this issue):

$ zig build test --summary all
test
└─ configure cmake header wrapper.h.in to wrapper.h failure
error: /home/dmitry/zig/git/test/standalone/cmakedefine/wrapper.h.in:29: unable to substitute variable: error: UndefinedVariable
error: /home/dmitry/zig/git/test/standalone/cmakedefine/wrapper.h.in:30: unable to substitute variable: error: UndefinedVariable
error: /home/dmitry/zig/git/test/standalone/cmakedefine/wrapper.h.in:35: unable to substitute variable: error: UndefinedVariable
error: HeaderConfigFailed
Build Summary: 4/6 steps succeeded; 1 failed
test transitive failure
├─ configure cmake header config.h.in to config.h cached
├─ configure cmake header pwd.sh.in to pwd.sh cached
├─ configure cmake header sigil.h.in to sigil.h cached
├─ configure cmake header stack.h.in to stack.h cached
└─ configure cmake header wrapper.h.in to wrapper.h failure

I tried making the error more verbose (e.g. listing the key that wasn't found), but couldn't find a nice way of communicating that up the stack together with an error. Maybe I should play more with storing extra data in the validator itself, but whatever I tried didn't feel right.

Suggestions/feedback are welcome, as I don't think it's ready to become a PR.

[MrDmitry]

I've been doing some digging around and stumbled upon #17995

I will prototype a Diagnostics-like approach