diff --git a/autoconf/BUILD.bazel b/autoconf/BUILD.bazel index db52abdeb..71e1407e5 100644 --- a/autoconf/BUILD.bazel +++ b/autoconf/BUILD.bazel @@ -58,6 +58,7 @@ bzl_library( srcs = ["package_info.bzl"], visibility = ["//visibility:public"], deps = [ + ":cc_autoconf_info_bzl", "//autoconf/private:bzl_lib", ], ) diff --git a/autoconf/autoconf.bzl b/autoconf/autoconf.bzl index 4ed99abd7..29171e858 100644 --- a/autoconf/autoconf.bzl +++ b/autoconf/autoconf.bzl @@ -54,6 +54,7 @@ def _autoconf_impl(ctx): cache_results = {} define_results = {} subst_results = {} + unquoted_defines = [] actions = {} @@ -89,10 +90,11 @@ def _autoconf_impl(ctx): )) define_checks[define_name] = check - - # Use cache file directly - no symlink needed define_results[define_name] = output + if check.get("unquote", False): + unquoted_defines.append(define_name) + # Check for truthy value if subst: check["subst"] = subst_name @@ -320,6 +322,7 @@ def _autoconf_impl(ctx): cache_results = cache_results, define_results = define_results, subst_results = subst_results, + unquoted_defines = unquoted_defines, ), OutputGroupInfo( autoconf_checks = depset([action.input for action in actions.values()]), diff --git a/autoconf/autoconf_hdr.bzl b/autoconf/autoconf_hdr.bzl index 24f3cdf73..25dde162b 100644 --- a/autoconf/autoconf_hdr.bzl +++ b/autoconf/autoconf_hdr.bzl @@ -39,7 +39,38 @@ def _autoconf_hdr_impl(ctx): all_define_checks = defaults.define | dep_results["define"] all_subst_checks = defaults.subst | dep_results["subst"] - inputs = depset([ctx.file.template] + all_subst_checks.values() + all_define_checks.values()) + # Merge unquoted_defines from defaults and transitive deps + all_unquoted = {} + for uq in getattr(defaults, "unquoted_defines", []): + all_unquoted[uq] = True + for uq in dep_results.get("unquoted_defines", []): + all_unquoted[uq] = True + + # Build the manifest: maps define/subst names to result file paths + metadata + manifest_data = { + "defines": {}, + "substs": {}, + } + for define_name, result_file in all_define_checks.items(): + manifest_data["defines"][define_name] = { + "path": result_file.path, + "unquote": define_name in all_unquoted, + } + for subst_name, result_file in all_subst_checks.items(): + manifest_data["substs"][subst_name] = { + "path": result_file.path, + "unquote": subst_name in all_unquoted, + } + + manifest = ctx.actions.declare_file("{}.manifest.json".format(ctx.label.name)) + ctx.actions.write( + output = manifest, + content = json.encode_indent(manifest_data, indent = " " * 4) + "\n", + ) + + inputs = depset( + [ctx.file.template, manifest] + all_subst_checks.values() + all_define_checks.values(), + ) # Process inlines: collect files and create mappings inline_files = [] @@ -55,21 +86,11 @@ def _autoconf_hdr_impl(ctx): inputs = depset(inline_files, transitive = [inputs]) - # Pass all individual results files directly to resolver (it merges internally) - # Include both defaults and transitive checks so resolver can merge them - # Use separate flags for each bucket args = ctx.actions.args() args.use_param_file("@%s", use_always = True) args.set_param_file_format("multiline") - # Add define results - for results_file_path in all_define_checks.values(): - args.add("--define-result", results_file_path) - - # Add subst results - for results_file_path in all_subst_checks.values(): - args.add("--subst-result", results_file_path) - + args.add("--manifest", manifest) args.add("--output", ctx.outputs.out) args.add("--template", ctx.file.template) args.add("--mode", ctx.attr.mode) diff --git a/autoconf/autoconf_toolchain.bzl b/autoconf/autoconf_toolchain.bzl index 070fc3f3f..24d000d08 100644 --- a/autoconf/autoconf_toolchain.bzl +++ b/autoconf/autoconf_toolchain.bzl @@ -34,6 +34,7 @@ def _autoconf_toolchain_impl(ctx): cache = direct_results["cache"], define = direct_results["define"], subst = direct_results["subst"], + unquoted_defines = direct_results["unquoted_defines"], ) return [ @@ -43,6 +44,7 @@ def _autoconf_toolchain_impl(ctx): cache = dep_results["cache"], define = dep_results["define"], subst = dep_results["subst"], + unquoted_defines = dep_results["unquoted_defines"], defaults_by_label = defaults_by_label, ), ), diff --git a/autoconf/cc_autoconf_info.bzl b/autoconf/cc_autoconf_info.bzl index df4f1f3af..fe8674b89 100644 --- a/autoconf/cc_autoconf_info.bzl +++ b/autoconf/cc_autoconf_info.bzl @@ -1,8 +1,29 @@ -"""# CcAutoconfInfo""" +"""# CcAutoconfInfo +Public API for external rules that produce autoconf-compatible check results. + +The ``CcAutoconfInfo`` provider constructor accepts defaults for every field +except ``owner``, so callers only need to supply the fields they care about: + +```python +load("@rules_cc_autoconf//autoconf:cc_autoconf_info.bzl", + "CcAutoconfInfo", "encode_result") + +def _my_rule_impl(ctx): + result = ctx.actions.declare_file("{}/MY_DEFINE.result.json".format(ctx.label.name)) + ctx.actions.write(output = result, content = encode_result("hello")) + return [CcAutoconfInfo(owner = ctx.label, define_results = {"MY_DEFINE": result})] +``` +""" + +load( + "//autoconf/private:autoconf_config.bzl", + _encode_result = "encode_result", +) load( "//autoconf/private:providers.bzl", _CcAutoconfInfo = "CcAutoconfInfo", ) CcAutoconfInfo = _CcAutoconfInfo +encode_result = _encode_result diff --git a/autoconf/macros/AC_C_RESTRICT/restrict.bzl b/autoconf/macros/AC_C_RESTRICT/restrict.bzl index 28e950975..f8edc654c 100644 --- a/autoconf/macros/AC_C_RESTRICT/restrict.bzl +++ b/autoconf/macros/AC_C_RESTRICT/restrict.bzl @@ -15,6 +15,7 @@ and a dedicated resolver tool to combine the results. """ load("@rules_cc//cc:find_cc_toolchain.bzl", "use_cc_toolchain") +load("//autoconf:cc_autoconf_info.bzl", "CcAutoconfInfo") load( "//autoconf/private:autoconf_config.bzl", "create_config_dict", @@ -22,7 +23,6 @@ load( "get_environment_variables", "write_config_json", ) -load("//autoconf/private:providers.bzl", "CcAutoconfInfo") # Test code templates for each keyword variant. # Each is a minimal C program that uses the keyword in a function signature. @@ -127,10 +127,9 @@ def _ac_c_restrict_impl(ctx): return [ CcAutoconfInfo( owner = ctx.label, - deps = depset(), cache_results = {"restrict": restrict_result}, define_results = {"restrict": restrict_result}, - subst_results = {}, + unquoted_defines = ["restrict"], ), DefaultInfo(files = depset([restrict_result])), OutputGroupInfo( diff --git a/autoconf/macros/AC_C_RESTRICT/restrict_resolver.cc b/autoconf/macros/AC_C_RESTRICT/restrict_resolver.cc index 74e0307d9..7658c9b74 100644 --- a/autoconf/macros/AC_C_RESTRICT/restrict_resolver.cc +++ b/autoconf/macros/AC_C_RESTRICT/restrict_resolver.cc @@ -215,17 +215,12 @@ std::optional read_check_success(const std::string& path) { return std::nullopt; } - // The result file has one top-level key (the cache variable name) - // with an object containing "success": true/false. - auto it = j.begin(); - const nlohmann::json& result = it.value(); - - if (!result.contains("success") || !result["success"].is_boolean()) { + if (!j.contains("success") || !j["success"].is_boolean()) { std::cerr << "Error: missing 'success' field in: " << path << std::endl; return std::nullopt; } - return result["success"].get(); + return j["success"].get(); } /** @@ -243,31 +238,20 @@ std::optional read_check_success(const std::string& path) { */ int write_result(const std::string& path, const std::optional& value, bool success) { - nlohmann::json result_obj; + nlohmann::json j; if (value.has_value()) { if (value->empty()) { - // Empty string: renders as #define restrict /**/ - result_obj["value"] = ""; + j["value"] = ""; } else { - // Keyword alias: renders as #define restrict - result_obj["value"] = *value; + j["value"] = *value; } } else { - // No define needed: keyword works natively. - // Write null value so the define is not emitted. - result_obj["value"] = nullptr; + j["value"] = nullptr; } - result_obj["success"] = success; - result_obj["is_define"] = true; - result_obj["is_subst"] = false; - result_obj["type"] = "compile"; - result_obj["define"] = "restrict"; - result_obj["unquote"] = true; - - nlohmann::json j; - j["restrict"] = result_obj; + j["success"] = success; + j["type"] = "compile"; std::ofstream out(path); if (!out.is_open()) { diff --git a/autoconf/package_info.bzl b/autoconf/package_info.bzl index 66d9a7976..db778af0d 100644 --- a/autoconf/package_info.bzl +++ b/autoconf/package_info.bzl @@ -1,6 +1,10 @@ """# package_info""" -load("//autoconf/private:providers.bzl", "CcAutoconfInfo") +load( + ":cc_autoconf_info.bzl", + "CcAutoconfInfo", + "encode_result", +) _RUNNER_SOURCES = [ "PACKAGE_NAME", @@ -84,60 +88,31 @@ def _package_info_impl(ctx): if source not in _RUNNER_SOURCES: ctx.actions.write( output = extra_results[key], - content = json.encode_indent({ - key: { - "success": True, - "value": json.encode(starlark_alias_values[source]), - }, - }, indent = " " * 4) + "\n", + content = encode_result(starlark_alias_values[source]), ) else: - # Write JSON files in check result format for PACKAGE_NAME - # Use regular string (not json.encode) so it goes through .dump() in check.cc ctx.actions.write( output = results["PACKAGE_NAME"], - content = json.encode_indent({ - "PACKAGE_NAME": { - "success": True, - "value": json.encode(ctx.attr.package_name), - }, - }, indent = " " * 4) + "\n", + content = encode_result(ctx.attr.package_name), ) - # Write JSON files in check result format for PACKAGE_VERSION ctx.actions.write( output = results["PACKAGE_VERSION"], - content = json.encode_indent({ - "PACKAGE_VERSION": { - "success": True, - "value": json.encode(ctx.attr.package_version), - }, - }, indent = " " * 4) + "\n", + content = encode_result(ctx.attr.package_version), ) - # Write PACKAGE_STRING as "package_name package_version" package_string = ctx.attr.package_name + " " + ctx.attr.package_version ctx.actions.write( output = results["PACKAGE_STRING"], - content = json.encode_indent({ - "PACKAGE_STRING": { - "success": True, - "value": json.encode(package_string), - }, - }, indent = " " * 4) + "\n", + content = encode_result(package_string), ) ctx.actions.write( output = results["PACKAGE_TARNAME"], - content = json.encode_indent({ - "PACKAGE_TARNAME": { - "success": True, - "value": json.encode( - ctx.attr.package_tarname if ctx.attr.package_tarname else ctx.attr.package_name, - ), - }, - }, indent = " " * 4) + "\n", + content = encode_result( + ctx.attr.package_tarname if ctx.attr.package_tarname else ctx.attr.package_name, + ), ) all_values = { @@ -151,43 +126,23 @@ def _package_info_impl(ctx): for key, source in ctx.attr.aliases.items(): ctx.actions.write( output = extra_results[key], - content = json.encode_indent({ - key: { - "success": True, - "value": json.encode(all_values[source]), - }, - }, indent = " " * 4) + "\n", + content = encode_result(all_values[source]), ) ctx.actions.write( output = results["PACKAGE_BUGREPORT"], - content = json.encode_indent({ - "PACKAGE_BUGREPORT": { - "success": True, - "value": json.encode(ctx.attr.package_bugreport), - }, - }, indent = " " * 4) + "\n", + content = encode_result(ctx.attr.package_bugreport), ) ctx.actions.write( output = results["PACKAGE_URL"], - content = json.encode_indent({ - "PACKAGE_URL": { - "success": True, - "value": json.encode(ctx.attr.package_url), - }, - }, indent = " " * 4) + "\n", + content = encode_result(ctx.attr.package_url), ) - # Package info creates defines (for config.h), so put them in define_results - # They're not cache variables or subst values return [ CcAutoconfInfo( owner = ctx.label, - deps = depset(), - cache_results = {}, define_results = results | extra_results, - subst_results = {}, ), ] diff --git a/autoconf/private/autoconf_config.bzl b/autoconf/private/autoconf_config.bzl index 4266f9aba..a3d617c68 100644 --- a/autoconf/private/autoconf_config.bzl +++ b/autoconf/private/autoconf_config.bzl @@ -10,6 +10,30 @@ load("//autoconf/private:providers.bzl", "CcAutoconfInfo") _TOOLCHAIN_TYPE = "//autoconf:toolchain_type" +def encode_result(value, success = True): + """Encode a value as a flat result JSON string. + + Returns the content for a result file that can be referenced from + ``CcAutoconfInfo``. Using this function insulates callers from the + internal result-file schema. + + Args: + value: The result value. Strings are JSON-encoded (quoted in the + output); numbers and booleans are stored as their JSON + representation. Use ``None`` for a valueless define + (``#define FOO`` with no value). + success: Whether the check succeeded. Defaults to ``True``. + A result with ``success=False`` is rendered as + ``/* #undef FOO */`` in the generated header. + + Returns: + A JSON string suitable for ``ctx.actions.write``. + """ + return json.encode_indent({ + "success": success, + "value": json.encode(value) if value != None else None, + }, indent = " " * 4) + "\n" + def get_autoconf_toolchain_defaults(ctx): """Get default checks from the autoconf toolchain if available. @@ -35,6 +59,7 @@ def get_autoconf_toolchain_defaults(ctx): cache = getattr(autoconf_defaults, "cache", {}), define = getattr(autoconf_defaults, "define", {}), subst = getattr(autoconf_defaults, "subst", {}), + unquoted_defines = getattr(autoconf_defaults, "unquoted_defines", []), ) def get_autoconf_toolchain_defaults_by_label(ctx): @@ -79,6 +104,7 @@ def filter_defaults(defaults_by_label, include_labels, exclude_labels): result_cache = {} result_define = {} result_subst = {} + result_unquoted = {} if include_labels: # Only include specified labels @@ -92,6 +118,8 @@ def filter_defaults(defaults_by_label, include_labels, exclude_labels): result_cache = result_cache | getattr(label_defaults, "cache", {}) result_define = result_define | getattr(label_defaults, "define", {}) result_subst = result_subst | getattr(label_defaults, "subst", {}) + for uq in getattr(label_defaults, "unquoted_defines", []): + result_unquoted[uq] = True elif exclude_labels: # Include all except specified labels exclude_set = {label: True for label in exclude_labels} @@ -100,17 +128,22 @@ def filter_defaults(defaults_by_label, include_labels, exclude_labels): result_cache = result_cache | getattr(label_defaults, "cache", {}) result_define = result_define | getattr(label_defaults, "define", {}) result_subst = result_subst | getattr(label_defaults, "subst", {}) + for uq in getattr(label_defaults, "unquoted_defines", []): + result_unquoted[uq] = True else: # Include all for label_defaults in defaults_by_label.values(): result_cache = result_cache | getattr(label_defaults, "cache", {}) result_define = result_define | getattr(label_defaults, "define", {}) result_subst = result_subst | getattr(label_defaults, "subst", {}) + for uq in getattr(label_defaults, "unquoted_defines", []): + result_unquoted[uq] = True return struct( cache = result_cache, define = result_define, subst = result_subst, + unquoted_defines = sorted(result_unquoted.keys()), ) def merge_with_defaults(defaults, results): @@ -137,24 +170,18 @@ def collect_transitive_results(dep_infos): dep_infos (list): A list of `CcAutoconfInfo`. Returns: - dict: A mapping of cache variable name to check result files. + dict: A mapping with keys "cache", "define", "subst" (each dict[str, File]) + and "unquoted_defines" (list[str]). """ cache_results = {} define_results = {} subst_results = {} + unquoted_defines_set = {} for dep_info in dep_infos: - # Check for conflicts before merging - same cache variable from different targets - # should point to the same file (they're the same check result) - # Compare file paths, not File objects, since the same file from different - # dependencies might be different File objects for cache_name, cache_file in dep_info.cache_results.items(): if cache_name in cache_results: existing_file = cache_results[cache_name] - - # Compare file paths to see if they're the same file if existing_file.path != cache_file.path: - # Different files for the same cache variable - this is a conflict - # This should not happen if Starlark properly prevents duplicates fail("Cache variable '{}' is defined in multiple dependencies with different result files:\n First: {}\n Second: {}\nThis indicates duplicate checks across different autoconf targets.".format( cache_name, existing_file.path, @@ -184,10 +211,14 @@ def collect_transitive_results(dep_infos): )) subst_results[subst_name] = subst_file + for uq in dep_info.unquoted_defines: + unquoted_defines_set[uq] = True + return { "cache": cache_results, "define": define_results, "subst": subst_results, + "unquoted_defines": sorted(unquoted_defines_set.keys()), } def collect_deps(deps): diff --git a/autoconf/private/checker/check_result.cc b/autoconf/private/checker/check_result.cc index 1d19965a3..70621a3c7 100644 --- a/autoconf/private/checker/check_result.cc +++ b/autoconf/private/checker/check_result.cc @@ -74,37 +74,19 @@ std::optional CheckResult::from_json(const std::string& name, value = std::nullopt; } - // Check for is_define, define_flag, or define for backward compatibility + // Flat result format: only success, value, type are in the JSON. + // Consumer metadata (is_define, is_subst, define, subst, unquote) is + // tracked in Starlark providers and supplied via the manifest at render + // time. Defaults here are safe for the checker's own dep-loading path. bool is_define = false; - if (json_value->contains("is_define") && - (*json_value)["is_define"].is_boolean()) { - is_define = (*json_value)["is_define"].get(); - } else if (json_value->contains("define_flag") && - (*json_value)["define_flag"].is_boolean()) { - is_define = (*json_value)["define_flag"].get(); - } else if (json_value->contains("define") && - (*json_value)["define"].is_boolean()) { - is_define = (*json_value)["define"].get(); - } - - // Check for is_subst, subst_flag, or subst for backward compatibility bool is_subst = false; - if (json_value->contains("is_subst") && - (*json_value)["is_subst"].is_boolean()) { - is_subst = (*json_value)["is_subst"].get(); - } else if (json_value->contains("subst_flag") && - (*json_value)["subst_flag"].is_boolean()) { - is_subst = (*json_value)["subst_flag"].get(); - } else if (json_value->contains("subst") && - (*json_value)["subst"].is_boolean()) { - is_subst = (*json_value)["subst"].get(); - } + std::optional define_name; + std::optional subst_name; + bool unquote = false; - // Check for type (defaults to kDefine for backward compatibility) CheckType type = CheckType::kDefine; if (json_value->contains("type") && (*json_value)["type"].is_string()) { std::string type_str = (*json_value)["type"].get(); - // Parse type string to CheckType enum if (type_str == "function") { type = CheckType::kFunction; } else if (type_str == "lib") { @@ -118,7 +100,6 @@ std::optional CheckResult::from_json(const std::string& name, } else if (type_str == "define") { type = CheckType::kDefine; } else if (type_str == "subst") { - // Backward compatibility: subst -> kM4Variable type = CheckType::kM4Variable; } else if (type_str == "m4_variable") { type = CheckType::kM4Variable; @@ -137,34 +118,6 @@ std::optional CheckResult::from_json(const std::string& name, } } - // Parse optional define and subst string fields - std::optional define_name; - if (json_value->contains("define") && (*json_value)["define"].is_string()) { - define_name = (*json_value)["define"].get(); - // If define is a string, infer is_define should be true (unless - // explicitly set to false) - if (!json_value->contains("is_define")) { - is_define = true; - } - } - - std::optional subst_name; - if (json_value->contains("subst") && (*json_value)["subst"].is_string()) { - subst_name = (*json_value)["subst"].get(); - // If subst is a string, infer is_subst should be true (unless - // explicitly set to false) - if (!json_value->contains("is_subst")) { - is_subst = true; - } - } - - // Parse unquote field (for AC_DEFINE_UNQUOTED) - bool unquote = false; - if (json_value->contains("unquote") && - (*json_value)["unquote"].is_boolean()) { - unquote = (*json_value)["unquote"].get(); - } - CheckResult result(name, value, success, is_define, is_subst, type, define_name, subst_name, unquote); return result; diff --git a/autoconf/private/checker/checker.cc b/autoconf/private/checker/checker.cc index 50dfac293..9874e2f51 100644 --- a/autoconf/private/checker/checker.cc +++ b/autoconf/private/checker/checker.cc @@ -36,11 +36,9 @@ class ResultLookup { */ void add_mapping(const std::string& lookup_name, const std::filesystem::path& file_path) { - // Check for duplicate name (strict - any duplicate is an error) std::unordered_map::iterator it = name_to_index_.find(lookup_name); if (it != name_to_index_.end()) { - // Duplicate name detected size_t existing_idx = it->second; const std::string& existing_file = file_to_path_[existing_idx]; if (existing_file != file_path.string()) { @@ -60,14 +58,10 @@ class ResultLookup { " This indicates a bug in Starlark code - it should " "deduplicate before calling C++."); } - // Same name, same file - idempotent, skip return; } - // Load result from file (or get existing index if file already loaded) - size_t idx = load_or_get_index(file_path); - - // Index by lookup name + size_t idx = load_or_get_index(lookup_name, file_path); name_to_index_[lookup_name] = idx; } @@ -108,20 +102,21 @@ class ResultLookup { /** * @brief Load result from file or return existing index if already loaded. + * @param lookup_name The name from the --dep argument, used as the result + * identity (flat format files have no embedded name). * @param file_path Path to JSON file containing the result. * @return Index of the result in results_ vector. */ - size_t load_or_get_index(const std::filesystem::path& file_path) { + size_t load_or_get_index(const std::string& lookup_name, + const std::filesystem::path& file_path) { std::string file_key = file_path.string(); - // Check if file already loaded std::unordered_map::iterator file_it = file_to_index_.find(file_key); if (file_it != file_to_index_.end()) { - return file_it->second; // Already loaded + return file_it->second; } - // Load from file if (!file_exists(file_path)) { throw std::runtime_error("Dep results file does not exist: " + file_key); @@ -137,25 +132,18 @@ class ResultLookup { results_file >> results_json; results_file.close(); - // Parse result from JSON (expect single result per file) if (results_json.empty() || !results_json.is_object()) { throw std::runtime_error("Dep results file is empty or invalid: " + file_key); } - // Get the first (and should be only) result from the JSON - nlohmann::json::iterator it = results_json.begin(); - const std::string& key = it.key(); - const nlohmann::json& json_value = it.value(); - std::optional result = - CheckResult::from_json(key, &json_value); + CheckResult::from_json(lookup_name, &results_json); if (!result.has_value()) { throw std::runtime_error("Failed to parse CheckResult from file: " + file_key); } - // Add to results vector size_t idx = results_.size(); results_.push_back(*result); file_to_index_[file_key] = idx; @@ -410,42 +398,28 @@ int Checker::run_check_from_file(const std::filesystem::path& check_path, } } - nlohmann::json j = nlohmann::json::object(); - // Parse the value as JSON to preserve type information when writing - // result.value is a JSON-encoded string, so we parse it to get the - // actual JSON value + // Flat result format: {success, value, type} only. + // Consumer metadata is tracked in Starlark providers and written + // to a manifest at rendering time. nlohmann::json value_json; if (result.value.has_value()) { if (result.value->empty()) { - // Explicitly empty value: write as empty string in JSON value_json = ""; } else { try { value_json = nlohmann::json::parse(*result.value); } catch (const nlohmann::json::parse_error&) { - // If parsing fails, treat as plain string value_json = *result.value; } } } else { - // No value: write as null in JSON value_json = nullptr; } - nlohmann::json result_json = { - {"value", value_json}, + nlohmann::json j = { {"success", result.success}, - {"is_define", result.is_define}, - {"is_subst", result.is_subst}, {"type", check_type_to_string(result.type)}, - {"unquote", result.unquote}, + {"value", value_json}, }; - if (result.define.has_value()) { - result_json["define"] = *result.define; - } - if (result.subst.has_value()) { - result_json["subst"] = *result.subst; - } - j[result.name] = result_json; std::ofstream results_file = open_ofstream(results_path); if (!results_file.is_open()) { diff --git a/autoconf/private/module_parser/main.cc b/autoconf/private/module_parser/main.cc index e38c6dcd9..69537e538 100644 --- a/autoconf/private/module_parser/main.cc +++ b/autoconf/private/module_parser/main.cc @@ -318,27 +318,23 @@ std::optional parse_args(int argc, char* argv[]) { } /** - * @brief Write a check result JSON file for a package define. + * @brief Write a flat result JSON file for a package define. * - * Writes JSON in the format: { "DEFINE_NAME": { "value": "\"...\""", "success": - * true } } + * Writes JSON in the format: { "success": true, "value": "\"...\"" } * * @param path Path to the output file. - * @param define_name The define name (e.g., "PACKAGE_NAME"). * @param value The string value (will be quoted in JSON). * @return true on success, false on failure (error printed to stderr). */ -bool write_package_json(const std::string& path, const std::string& define_name, - const std::string& value) { +bool write_package_json(const std::string& path, const std::string& value) { std::ofstream out = rules_cc_autoconf::open_ofstream(path); if (!out.is_open()) { std::cerr << "Error: Could not open output file: " << path << std::endl; return false; } - nlohmann::json j = nlohmann::json::object(); - j[define_name] = { - {"value", "\"" + value + "\""}, + nlohmann::json j = { {"success", true}, + {"value", "\"" + value + "\""}, }; out << j.dump(4) << std::endl; out.close(); @@ -415,19 +411,16 @@ int main(int argc, char* argv[]) { std::string tarname = args.forced_tarname.empty() ? name : args.forced_tarname; - if (!write_package_json(args.out_name, "PACKAGE_NAME", name)) return 1; - if (!write_package_json(args.out_version, "PACKAGE_VERSION", version)) - return 1; + if (!write_package_json(args.out_name, name)) return 1; + if (!write_package_json(args.out_version, version)) return 1; if (!args.out_string.empty()) { - if (!write_package_json(args.out_string, "PACKAGE_STRING", - name + " " + version)) + if (!write_package_json(args.out_string, name + " " + version)) return 1; } if (!args.out_tarname.empty()) { - if (!write_package_json(args.out_tarname, "PACKAGE_TARNAME", tarname)) - return 1; + if (!write_package_json(args.out_tarname, tarname)) return 1; } if (!args.aliases.empty()) { @@ -445,8 +438,7 @@ int main(int argc, char* argv[]) { << "'" << std::endl; return 1; } - if (!write_package_json(alias.output_path, alias.name, it->second)) - return 1; + if (!write_package_json(alias.output_path, it->second)) return 1; } } diff --git a/autoconf/private/providers.bzl b/autoconf/private/providers.bzl index 7ccf14204..4469318af 100644 --- a/autoconf/private/providers.bzl +++ b/autoconf/private/providers.bzl @@ -1,12 +1,31 @@ """# providers""" -CcAutoconfInfo = provider( +def _cc_autoconf_info_init( + owner, + deps = None, + cache_results = {}, + define_results = {}, + subst_results = {}, + unquoted_defines = []): + """Preprocess CcAutoconfInfo fields, filling in safe defaults.""" + return { + "cache_results": cache_results, + "define_results": define_results, + "deps": deps if deps != None else depset(), + "owner": owner, + "subst_results": subst_results, + "unquoted_defines": unquoted_defines, + } + +CcAutoconfInfo, _new_cc_autoconf_info = provider( doc = "A provider containing autoconf configuration and results.", fields = { - "cache_results": "dict[str, File]: A map of cache names to JSON files containing the defines produced by `CcAutoconfCheck` actions. Each define name must be unique.", - "define_results": "dict[str, File]: A map of define names to JSON files containing the defines produced by `CcAutoconfCheck` actions. Each define name must be unique.", + "cache_results": "dict[str, File]: A map of cache names to flat result JSON files produced by `CcAutoconfCheck` actions.", + "define_results": "dict[str, File]: A map of define names to flat result JSON files produced by `CcAutoconfCheck` actions.", "deps": "depset[CcAutoconfInfo]: Dependencies of the current info target.", "owner": "Label: The label of the owner of the results.", - "subst_results": "dict[str, File]: A map of subst names to JSON files containing the defines produced by `CcAutoconfCheck` actions. Each define name must be unique.", + "subst_results": "dict[str, File]: A map of subst names to flat result JSON files produced by `CcAutoconfCheck` actions.", + "unquoted_defines": "list[str]: Define names that should be rendered unquoted (AC_DEFINE_UNQUOTED).", }, + init = _cc_autoconf_info_init, ) diff --git a/autoconf/private/resolver/main.cc b/autoconf/private/resolver/main.cc index c398f01dc..821c90e3e 100644 --- a/autoconf/private/resolver/main.cc +++ b/autoconf/private/resolver/main.cc @@ -1,5 +1,5 @@ /** - * @brief Merges check results and generates config.h from a template. + * @brief Loads a manifest and generates config.h from a template. */ #include @@ -22,17 +22,8 @@ namespace { * @brief Parsed command-line arguments for resolver binary. */ struct ResolverArgs { - /** Paths to JSON result files for cache variables (can be specified - * multiple times) */ - std::vector cache_results_paths{}; - - /** Paths to JSON result files for defines (can be specified multiple times) - */ - std::vector define_results_paths{}; - - /** Paths to JSON result files for subst values (can be specified multiple - * times) */ - std::vector subst_results_paths{}; + /** Path to manifest JSON mapping define/subst names to result file paths */ + std::filesystem::path manifest_path{}; /** Path to template file (config.h.in) (required) */ std::filesystem::path template_path{}; @@ -56,19 +47,10 @@ struct ResolverArgs { void print_usage(const char* program_name) { std::cout << "Usage: " << program_name << " [options]\n"; std::cout << "Options:\n"; - std::cout << " --cache-result Path to JSON results file for cache " - "variables (can be " - "specified multiple times)\n"; - std::cout << " --define-result Path to JSON results file for " - "defines (can be " - "specified multiple times)\n"; - std::cout << " --subst-result Path to JSON results file for subst " - "values (can be " - "specified multiple times)\n"; + std::cout << " --manifest Path to manifest JSON mapping " + "define/subst names to result files (required)\n"; std::cout << " --template Template file (config.h.in) (required)\n"; - std::cout << " --config Optional config file (for running " - "additional checks from template)\n"; std::cout << " --output Path to output config.h file " "(required)\n"; std::cout << " --inline JSON object mapping search strings " @@ -82,7 +64,6 @@ void print_usage(const char* program_name) { } std::optional parse_args(int argc, char* argv[]) { - // Expand @file response file if present std::vector expanded_args; std::vector expanded_argv; int expanded_argc; @@ -101,30 +82,11 @@ std::optional parse_args(int argc, char* argv[]) { if (arg == "--help" || arg == "-h") { args.show_help = true; return args; - } else if (arg == "--cache-result") { - if (i + 1 < expanded_argc) { - args.cache_results_paths.push_back( - std::string(expanded_argv_ptr[++i])); - } else { - std::cerr << "Error: --cache-result requires a file path" - << std::endl; - return std::nullopt; - } - } else if (arg == "--define-result") { - if (i + 1 < expanded_argc) { - args.define_results_paths.push_back( - std::string(expanded_argv_ptr[++i])); - } else { - std::cerr << "Error: --define-result requires a file path" - << std::endl; - return std::nullopt; - } - } else if (arg == "--subst-result") { + } else if (arg == "--manifest") { if (i + 1 < expanded_argc) { - args.subst_results_paths.push_back( - std::string(expanded_argv_ptr[++i])); + args.manifest_path = std::string(expanded_argv_ptr[++i]); } else { - std::cerr << "Error: --subst-result requires a file path" + std::cerr << "Error: --manifest requires a file path" << std::endl; return std::nullopt; } @@ -194,10 +156,6 @@ std::optional parse_args(int argc, char* argv[]) { } } - // Validate required arguments - // Note: result paths can be empty - the resolver will just process the - // template without any check results - if (args.output_path.empty()) { std::cerr << "Error: --output is required" << std::endl; return std::nullopt; @@ -208,6 +166,11 @@ std::optional parse_args(int argc, char* argv[]) { return std::nullopt; } + if (args.manifest_path.empty()) { + std::cerr << "Error: --manifest is required" << std::endl; + return std::nullopt; + } + return args; } } // namespace @@ -227,7 +190,6 @@ int main(int argc, char* argv[]) { } return Resolver::resolve_and_generate( - args.cache_results_paths, args.define_results_paths, - args.subst_results_paths, args.template_path, args.output_path, - args.inlines, args.substitutions, args.mode); + args.manifest_path, args.template_path, args.output_path, args.inlines, + args.substitutions, args.mode); } diff --git a/autoconf/private/resolver/resolver.cc b/autoconf/private/resolver/resolver.cc index 14e013b4d..b12b0f8d8 100644 --- a/autoconf/private/resolver/resolver.cc +++ b/autoconf/private/resolver/resolver.cc @@ -2,16 +2,12 @@ #include #include -#include #include -#include #include #include -#include #include "autoconf/private/checker/check.h" #include "autoconf/private/checker/check_result.h" -#include "autoconf/private/checker/checker.h" #include "autoconf/private/checker/debug_logger.h" #include "autoconf/private/common/file_util.h" #include "autoconf/private/resolver/source_generator.h" @@ -21,167 +17,150 @@ namespace rules_cc_autoconf { namespace { -// Helper function to load results from JSON file -std::vector load_results_from_file( - const std::filesystem::path& path) { - std::vector loaded_results; - std::ifstream results_file = open_ifstream(path); - if (!results_file.is_open()) { - throw std::runtime_error("Failed to open results file: " + +/** + * @brief Load a flat result file and construct a CheckResult. + * + * Flat result files contain only {success, value, type}. Consumer metadata + * (name, define, subst, unquote, is_define, is_subst) is provided externally + * from the manifest. + * + * @param name The define/subst name from the manifest key. + * @param path Path to the flat result JSON file. + * @param is_define Whether this result is a define. + * @param is_subst Whether this result is a subst variable. + * @param unquote Whether this is AC_DEFINE_UNQUOTED. + * @return CheckResult with all fields populated. + */ +CheckResult load_flat_result(const std::string& name, + const std::filesystem::path& path, bool is_define, + bool is_subst, bool unquote) { + if (!file_exists(path)) { + throw std::runtime_error("Result file does not exist: " + + path.string()); + } + + std::ifstream file = open_ifstream(path); + if (!file.is_open()) { + throw std::runtime_error("Failed to open result file: " + path.string()); } nlohmann::json j; - results_file >> j; - results_file.close(); + file >> j; + file.close(); - // Handle null or empty JSON - treat as empty results if (j.is_null() || !j.is_object()) { - j = nlohmann::json::object(); + throw std::runtime_error("Invalid result JSON in: " + path.string()); } - // Use CheckResult::from_json() to parse each result - for (nlohmann::json::iterator it = j.begin(); it != j.end(); ++it) { - const std::string name = it.key(); - const nlohmann::json& json_value = it.value(); + std::optional result = CheckResult::from_json(name, &j); + if (!result.has_value()) { + throw std::runtime_error("Failed to parse result from: " + + path.string()); + } - std::optional result = - CheckResult::from_json(name, &json_value); - if (!result.has_value()) { - throw std::runtime_error("Failed to parse CheckResult from file: " + - path.string()); + result->is_define = is_define; + result->is_subst = is_subst; + result->unquote = unquote; + if (is_define) { + result->define = name; + } + if (is_subst) { + result->subst = name; + } + + return *result; +} + +/** + * @brief Load results from a manifest section (defines or substs). + * + * Each entry maps a name to {"path": "...", "unquote": bool}. + * + * @param section The manifest section JSON object. + * @param is_define Whether entries are defines. + * @param is_subst Whether entries are subst variables. + * @return Vector of CheckResult objects. + */ +std::vector load_manifest_section(const nlohmann::json& section, + bool is_define, bool is_subst) { + std::vector results; + + for (auto it = section.begin(); it != section.end(); ++it) { + const std::string& name = it.key(); + const nlohmann::json& entry = it.value(); + + if (!entry.is_object() || !entry.contains("path")) { + throw std::runtime_error("Manifest entry '" + name + + "' is missing required 'path' field"); } - loaded_results.push_back(*result); + std::string path = entry["path"].get(); + bool unquote = entry.value("unquote", false); + + results.push_back( + load_flat_result(name, path, is_define, is_subst, unquote)); } - return loaded_results; + return results; } } // namespace int Resolver::resolve_and_generate( - const std::vector& cache_results_paths, - const std::vector& define_results_paths, - const std::vector& subst_results_paths, + const std::filesystem::path& manifest_path, const std::filesystem::path& template_path, const std::filesystem::path& output_path, const std::map& inlines, const std::map& substitutions, Mode mode) { try { - // Helper function to load and merge results from a list of paths - std::function( - const std::vector&)> - load_and_merge_results = [](const std::vector< - std::filesystem::path>& paths) { - std::unordered_map results_map{}; - std::vector - order{}; // Preserve order of first occurrence - for (const std::filesystem::path& results_path : paths) { - if (!file_exists(results_path)) { - throw std::runtime_error( - "Results file does not exist: " + - results_path.string()); - } - std::vector loaded = - load_results_from_file(results_path); - // Only add results that haven't been seen yet, or verify - // they match - for (const CheckResult& result : loaded) { - std::unordered_map::iterator - existing_it = results_map.find(result.name); - if (existing_it != results_map.end()) { - // Duplicate cache variable found - check if values - // match - const CheckResult& existing = existing_it->second; - if (existing.success != result.success || - existing.value != result.value) { - std::cerr << "Error: Duplicate result '" - << result.name - << "' with conflicting values:\n"; - std::cerr - << " First: success=" - << (existing.success ? "true" : "false") - << ", value=\"" - << existing.value.value_or("") << "\"\n"; - std::cerr << " Second: success=" - << (result.success ? "true" : "false") - << ", value=\"" - << result.value.value_or("") << "\"" - << std::endl; - throw std::runtime_error( - "Conflicting result values"); - } - // Values match, silently ignore duplicate - } else { - results_map.emplace(result.name, result); - order.push_back(result.name); - } - } - } - // Convert map to vector preserving order - std::vector results; - results.reserve(order.size()); - for (const std::string& name : order) { - std::unordered_map::iterator it = - results_map.find(name); - if (it != results_map.end()) { - results.push_back(it->second); - } - } - return results; - }; - - // Load results from all three buckets - // Note: Results are loaded from files, but we need to route them to the - // correct bucket based on their is_define and is_subst flags. However, - // since we're passing separate file lists for each bucket, we assume - // files in cache_results_paths contain cache results, files in - // define_results_paths contain define results, and files in - // subst_results_paths contain subst results. This matches how - // autoconf.bzl writes the files. - std::vector cache_results = - load_and_merge_results(cache_results_paths); + if (!file_exists(manifest_path)) { + throw std::runtime_error("Manifest file does not exist: " + + manifest_path.string()); + } + + std::ifstream manifest_file = open_ifstream(manifest_path); + if (!manifest_file.is_open()) { + throw std::runtime_error("Failed to open manifest file: " + + manifest_path.string()); + } + + nlohmann::json manifest; + manifest_file >> manifest; + manifest_file.close(); + + if (!manifest.is_object()) { + throw std::runtime_error("Manifest is not a JSON object: " + + manifest_path.string()); + } + + nlohmann::json defines_section = + manifest.value("defines", nlohmann::json::object()); + nlohmann::json substs_section = + manifest.value("substs", nlohmann::json::object()); + std::vector define_results = - load_and_merge_results(define_results_paths); + load_manifest_section(defines_section, true, false); std::vector subst_results = - load_and_merge_results(subst_results_paths); - - // Merge all results for logging and backward compatibility - std::vector all_results; - all_results.reserve(cache_results.size() + define_results.size() + - subst_results.size()); - all_results.insert(all_results.end(), cache_results.begin(), - cache_results.end()); - all_results.insert(all_results.end(), define_results.begin(), - define_results.end()); - all_results.insert(all_results.end(), subst_results.begin(), - subst_results.end()); - - // Log only define values (config.h defines), not cache or subst results + load_manifest_section(substs_section, false, true); + + std::vector cache_results; + if (DebugLogger::is_debug_enabled()) { std::vector sorted_defines = define_results; std::sort(sorted_defines.begin(), sorted_defines.end(), [](const CheckResult& a, const CheckResult& b) { - std::string a_define = - a.define.has_value() ? *a.define : a.name; - std::string b_define = - b.define.has_value() ? *b.define : b.name; - return a_define < b_define; + return a.name < b.name; }); for (const CheckResult& result : sorted_defines) { std::string status = result.success ? "yes" : "no"; - std::string define_name = - result.define.has_value() ? *result.define : result.name; - DebugLogger::log("checking " + define_name + "... " + status); + DebugLogger::log("checking " + result.name + "... " + status); } } - // Generate header with three separate buckets SourceGenerator generator(cache_results, define_results, subst_results, mode); - std::string template_content{}; std::ifstream template_file = open_ifstream(template_path); if (!template_file.is_open()) { std::cerr << "Error: Failed to open template file: " @@ -190,7 +169,7 @@ int Resolver::resolve_and_generate( } std::stringstream buffer{}; buffer << template_file.rdbuf(); - template_content = buffer.str(); + std::string template_content = buffer.str(); template_file.close(); generator.generate_config_header(output_path, template_content, inlines, diff --git a/autoconf/private/resolver/resolver.h b/autoconf/private/resolver/resolver.h index b28b2ef91..a7f06d6eb 100644 --- a/autoconf/private/resolver/resolver.h +++ b/autoconf/private/resolver/resolver.h @@ -13,21 +13,20 @@ namespace rules_cc_autoconf { /** * @brief Library for resolving autoconf results and generating headers. * - * Merges check results from multiple sources, optionally runs additional checks - * from templates, logs all check results, and generates config.h headers. + * Loads check results via a manifest JSON (which maps define/subst names to + * flat result file paths and rendering metadata), then generates config.h + * headers from a template. */ class Resolver { public: /** - * @brief Resolve results and generate a header file. + * @brief Resolve results from a manifest and generate a header file. * - * Loads and merges check results from multiple JSON files, logs all check - * results in order, and generates a config.h header file from a template. + * The manifest JSON maps consumer names to flat result file paths and + * rendering metadata (e.g. unquote). Each flat result file contains only + * {success, value, type}. * - * @param cache_results_paths Paths to JSON result files for cache - * variables. - * @param define_results_paths Paths to JSON result files for defines. - * @param subst_results_paths Paths to JSON result files for subst values. + * @param manifest_path Path to the manifest JSON file. * @param template_path Path to template file (config.h.in) (required). * @param output_path Path where config.h will be written. * @param inlines Map from search strings to file paths for inline @@ -38,9 +37,7 @@ class Resolver { * @return 0 on success, 1 on error. */ static int resolve_and_generate( - const std::vector& cache_results_paths, - const std::vector& define_results_paths, - const std::vector& subst_results_paths, + const std::filesystem::path& manifest_path, const std::filesystem::path& template_path, const std::filesystem::path& output_path, const std::map& inlines = {}, diff --git a/autoconf/private/src_gen/main.cc b/autoconf/private/src_gen/main.cc index 5ee345b99..beabc0e55 100644 --- a/autoconf/private/src_gen/main.cc +++ b/autoconf/private/src_gen/main.cc @@ -161,26 +161,20 @@ ResultEntry load_single_result_from_file(const std::string& path) { file.close(); if (j.is_null() || !j.is_object() || j.empty()) { - // Treat empty/invalid as unsuccessful. return ResultEntry{}; } - // Each results file produced by these rules is expected to contain exactly - // one entry. We take the first entry regardless of its key. - nlohmann::json::iterator it = j.begin(); - const nlohmann::json& val = it.value(); - ResultEntry entry; - nlohmann::json::const_iterator value_it = val.find("value"); - if (value_it != val.end() && !value_it->is_null()) { + nlohmann::json::const_iterator value_it = j.find("value"); + if (value_it != j.end() && !value_it->is_null()) { if (value_it->is_string()) { entry.value = value_it->get(); } else { entry.value = value_it->dump(); } } - nlohmann::json::const_iterator success_it = val.find("success"); - entry.success = (success_it != val.end()) ? success_it->get() : false; + nlohmann::json::const_iterator success_it = j.find("success"); + entry.success = (success_it != j.end()) ? success_it->get() : false; return entry; }