From 0160c3b5410ff82cc2a242ee401d8d70d1c0c88c Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Fri, 15 May 2026 20:44:24 +0800 Subject: [PATCH 01/16] feat(csharp): implement config patching to align with Go and C++ loaders Mirror the patching mechanism already implemented in Go (load.go) and C++ (load.pc.cc / util.pc.cc) so all three generated loaders share the same runtime semantics: * scalar fields are overwritten by populated values from the patch * list fields are appended (or cleared first when PATCH_REPLACE is set) * map fields are merged by key with recursive PatchMessage on message values (or cleared first when PATCH_REPLACE is set) * nested message fields are patched recursively C# changes * Util.pc.cs: add PatchMessage / GetSheetPatch using protobuf reflection (FieldDescriptor.Accessor + IList / IDictionary) and read sheet- and field-level patch type from (tableau.worksheet) / (tableau.field).prop extensions. * Load.pc.cs: add LoadMode (All / OnlyMain / OnlyPatch), PatchDirs, PatchPaths, Mode options; LoadMessagerInDir now dispatches to a new LoadMessagerWithPatch when a sheet-level patch is declared, with PatchPaths taking precedence over PatchDirs and existence filtering on PatchDirs entries. * Sync the changes from the embedded source-of-truth files (cmd/protoc-gen-csharp-tableau-loader/embed/*) into the generated test loader (test/csharp-tableau-loader/tableau/*). * Program.cs: add TestPatch covering PatchDirs, OnlyMain, OnlyPatch and multi-PatchPaths flows. Tests * test/go-tableau-loader/main_test.go: add Test_Patch, replicating the scenarios from cpp main.cpp::TestPatch and csharp Program.cs::TestPatch (patchconf / patchconf2 / different format / multiple patch files / ModeOnlyMain / ModeOnlyPatch). The "patchconf" sub-test asserts proto.Equal against testdata/patchresult/RecursivePatchConf.json, confirming Go's xproto.PatchMessage produces the same output as the C++ and C# reflection-based implementations. Chore * .gitignore: ignore *.lscache (VS Code C# Dev Kit cache). * .github/workflows/*: read Go version from go.mod via go-version-file instead of hard-coding 1.24.x; minor YAML formatting clean-ups. * test/go-tableau-loader/buf.gen.yaml: add trailing newline. --- .github/workflows/release-cpp.yml | 9 +- .github/workflows/release-csharp.yml | 9 +- .github/workflows/release-go.yml | 9 +- .github/workflows/testing-cpp.yml | 10 +- .github/workflows/testing-csharp.yml | 10 +- .github/workflows/testing-go.yml | 10 +- .gitignore | 3 + .../embed/Load.pc.cs | 145 ++++++++++++- .../embed/Util.pc.cs | 193 +++++++++++++++++- test/csharp-tableau-loader/Program.cs | 73 +++++++ test/csharp-tableau-loader/tableau/Load.pc.cs | 144 ++++++++++++- test/csharp-tableau-loader/tableau/Util.pc.cs | 192 +++++++++++++++++ test/go-tableau-loader/buf.gen.yaml | 2 +- test/go-tableau-loader/main_test.go | 161 +++++++++++++++ 14 files changed, 938 insertions(+), 32 deletions(-) diff --git a/.github/workflows/release-cpp.yml b/.github/workflows/release-cpp.yml index 65161baa..7fd7d0af 100644 --- a/.github/workflows/release-cpp.yml +++ b/.github/workflows/release-cpp.yml @@ -2,7 +2,7 @@ name: Release CPP on: release: - types: [ published ] + types: [published] workflow_dispatch: jobs: @@ -13,8 +13,8 @@ jobs: 'cmd/protoc-gen-cpp-tableau-loader/') strategy: matrix: - goos: [ linux, darwin, windows ] - goarch: [ amd64, arm64 ] + goos: [linux, darwin, windows] + goarch: [amd64, arm64] exclude: - goos: linux goarch: arm64 @@ -28,7 +28,8 @@ jobs: - name: Install Go uses: actions/setup-go@v5 with: - go-version: "1.24.x" + go-version-file: go.mod + cache-dependency-path: go.sum - name: Download dependencies run: | diff --git a/.github/workflows/release-csharp.yml b/.github/workflows/release-csharp.yml index 887605c7..ce665191 100644 --- a/.github/workflows/release-csharp.yml +++ b/.github/workflows/release-csharp.yml @@ -2,7 +2,7 @@ name: Release C# on: release: - types: [ published ] + types: [published] workflow_dispatch: jobs: @@ -13,8 +13,8 @@ jobs: 'cmd/protoc-gen-csharp-tableau-loader/') strategy: matrix: - goos: [ linux, darwin, windows ] - goarch: [ amd64, arm64 ] + goos: [linux, darwin, windows] + goarch: [amd64, arm64] exclude: - goos: linux goarch: arm64 @@ -28,7 +28,8 @@ jobs: - name: Install Go uses: actions/setup-go@v5 with: - go-version: "1.24.x" + go-version-file: go.mod + cache-dependency-path: go.sum - name: Download dependencies run: | diff --git a/.github/workflows/release-go.yml b/.github/workflows/release-go.yml index 69378803..fd5fd875 100644 --- a/.github/workflows/release-go.yml +++ b/.github/workflows/release-go.yml @@ -2,7 +2,7 @@ name: Release Go on: release: - types: [ published ] + types: [published] workflow_dispatch: jobs: release: @@ -12,8 +12,8 @@ jobs: 'cmd/protoc-gen-go-tableau-loader/') strategy: matrix: - goos: [ linux, darwin, windows ] - goarch: [ amd64, arm64 ] + goos: [linux, darwin, windows] + goarch: [amd64, arm64] exclude: - goos: linux goarch: arm64 @@ -27,7 +27,8 @@ jobs: - name: Install Go uses: actions/setup-go@v5 with: - go-version: "1.24.x" + go-version-file: go.mod + cache-dependency-path: go.sum - name: Download dependencies run: | diff --git a/.github/workflows/testing-cpp.yml b/.github/workflows/testing-cpp.yml index 92119728..9146a172 100644 --- a/.github/workflows/testing-cpp.yml +++ b/.github/workflows/testing-cpp.yml @@ -3,7 +3,7 @@ name: Testing C++ # Trigger on pushes, PRs (excluding documentation changes), and nightly. on: push: - branches: [ master, main ] + branches: [master, main] pull_request: schedule: - cron: 0 0 * * * # daily at 00:00 @@ -16,9 +16,8 @@ jobs: test: strategy: matrix: - os: [ ubuntu-latest, windows-latest ] - go-version: [ 1.24.x ] - protobuf-version: [ "32.0", "3.19.3" ] + os: [ubuntu-latest, windows-latest] + protobuf-version: ["32.0", "3.19.3"] include: - os: ubuntu-latest init_script: bash init.sh @@ -40,7 +39,8 @@ jobs: - name: Install Go uses: actions/setup-go@v6 with: - go-version: ${{ matrix.go-version }} + go-version-file: go.mod + cache-dependency-path: go.sum cache: true - name: Install dependencies (Ubuntu) diff --git a/.github/workflows/testing-csharp.yml b/.github/workflows/testing-csharp.yml index 6cd2c2bf..635befe9 100644 --- a/.github/workflows/testing-csharp.yml +++ b/.github/workflows/testing-csharp.yml @@ -3,7 +3,7 @@ name: Testing C# # Trigger on pushes, PRs (excluding documentation changes), and nightly. on: push: - branches: [ master, main ] + branches: [master, main] pull_request: schedule: - cron: 0 0 * * * # daily at 00:00 @@ -16,9 +16,8 @@ jobs: test: strategy: matrix: - os: [ ubuntu-latest, windows-latest ] - go-version: [ 1.24.x ] - protobuf-version: [ "32.0", "3.19.3" ] + os: [ubuntu-latest, windows-latest] + protobuf-version: ["32.0", "3.19.3"] name: test (${{ matrix.os }}, protobuf ${{ matrix.protobuf-version }}) runs-on: ${{ matrix.os }} @@ -33,7 +32,8 @@ jobs: - name: Install Go uses: actions/setup-go@v6 with: - go-version: ${{ matrix.go-version }} + go-version-file: go.mod + cache-dependency-path: go.sum cache: true - name: Install .NET SDK diff --git a/.github/workflows/testing-go.yml b/.github/workflows/testing-go.yml index 3918c147..410bb925 100644 --- a/.github/workflows/testing-go.yml +++ b/.github/workflows/testing-go.yml @@ -3,7 +3,7 @@ name: Testing Go # Trigger on pushes, PRs (excluding documentation changes), and nightly. on: push: - branches: [ master, main ] + branches: [master, main] pull_request: schedule: - cron: 0 0 * * * # daily at 00:00 @@ -16,9 +16,8 @@ jobs: test: strategy: matrix: - os: [ ubuntu-latest, windows-latest ] - go-version: [ 1.24.x ] - protobuf-version: [ "32.0", "3.19.3" ] + os: [ubuntu-latest, windows-latest] + protobuf-version: ["32.0", "3.19.3"] name: test (${{ matrix.os }}, protobuf ${{ matrix.protobuf-version }}) runs-on: ${{ matrix.os }} @@ -33,7 +32,8 @@ jobs: - name: Install Go uses: actions/setup-go@v6 with: - go-version: ${{ matrix.go-version }} + go-version-file: go.mod + cache-dependency-path: go.sum cache: true - name: Install Buf diff --git a/.gitignore b/.gitignore index de4ff9c5..09abcd07 100644 --- a/.gitignore +++ b/.gitignore @@ -47,3 +47,6 @@ coverage.txt !test/testdata/bin/ test/csharp-tableau-loader/protoconf +# C# Dev Kit language service cache (VS Code) +*.lscache + diff --git a/cmd/protoc-gen-csharp-tableau-loader/embed/Load.pc.cs b/cmd/protoc-gen-csharp-tableau-loader/embed/Load.pc.cs index 8990b929..60f47536 100644 --- a/cmd/protoc-gen-csharp-tableau-loader/embed/Load.pc.cs +++ b/cmd/protoc-gen-csharp-tableau-loader/embed/Load.pc.cs @@ -3,6 +3,7 @@ using System.IO; using pb = global::Google.Protobuf; using pbr = global::Google.Protobuf.Reflection; +using tableaupb = global::Tableau.Protobuf.Tableau; namespace Tableau { /// @@ -20,6 +21,19 @@ public static class Load /// public delegate pb::IMessage? LoadFunc(pbr::MessageDescriptor desc, string dir, Format fmt, in MessagerOptions? options); + /// + /// LoadMode controls patch loading behavior. + /// + public enum LoadMode + { + /// Load all related files (main + patches). Default. + All, + /// Only load the main file. + OnlyMain, + /// Only load the patch files. + OnlyPatch, + } + /// /// BaseOptions is the common options for both global-level and messager-level options. /// @@ -30,6 +44,14 @@ public class BaseOptions /// public bool? IgnoreUnknownFields { get; set; } /// + /// Specify the directory paths for config patching. + /// + public List? PatchDirs { get; set; } + /// + /// Specify the loading mode for config patching. Default is LoadMode.All. + /// + public LoadMode? Mode { get; set; } + /// /// You can specify custom read function to read a config file's content. /// Default is File.ReadAllBytes. /// @@ -60,6 +82,8 @@ public MessagerOptions ParseMessagerOptionsByName(string name) { var mopts = MessagerOptions?.TryGetValue(name, out var val) == true ? (MessagerOptions)val.Clone() : new MessagerOptions(); mopts.IgnoreUnknownFields ??= IgnoreUnknownFields; + mopts.PatchDirs ??= PatchDirs; + mopts.Mode ??= Mode; mopts.ReadFunc ??= ReadFunc; mopts.LoadFunc ??= LoadFunc; return mopts; @@ -77,15 +101,23 @@ public class MessagerOptions : BaseOptions, ICloneable /// directly, other than the specified load dir. /// public string? Path { get; set; } + /// + /// PatchPaths maps each messager name to one or multiple corresponding patch + /// file paths. If specified, then the main messager will be patched. + /// + public List? PatchPaths { get; set; } public object Clone() { return new MessagerOptions { IgnoreUnknownFields = IgnoreUnknownFields, + PatchDirs = PatchDirs, + Mode = Mode, ReadFunc = ReadFunc, LoadFunc = LoadFunc, - Path = Path + Path = Path, + PatchPaths = PatchPaths, }; } } @@ -127,10 +159,119 @@ public object Clone() string filename = name + Util.Format2Ext(fmt); path = Path.Combine(dir, filename); } + var sheetPatch = Util.GetSheetPatch(desc); + if (sheetPatch != tableaupb::Patch.None) + { + return LoadMessagerWithPatch(desc, path, fmt, sheetPatch, options); + } var loadFunc = options?.LoadFunc ?? LoadMessager; return loadFunc(desc, path, fmt, options); } + /// + /// LoadMessagerWithPatch loads a protobuf message with patch support. + /// + public static pb::IMessage? LoadMessagerWithPatch(pbr::MessageDescriptor desc, string path, Format fmt, + tableaupb::Patch patch, in MessagerOptions? options = null) + { + var mode = options?.Mode ?? LoadMode.All; + var loadFunc = options?.LoadFunc ?? LoadMessager; + if (mode == LoadMode.OnlyMain) + { + // Ignore patch files when LoadMode.OnlyMain specified. + return loadFunc(desc, path, fmt, options); + } + string name = desc.Name; + List patchPaths = new(); + if (options?.PatchPaths != null && options.PatchPaths.Count > 0) + { + // PatchPaths takes precedence over PatchDirs. + patchPaths.AddRange(options.PatchPaths); + } + else if (options?.PatchDirs != null) + { + string filename = name + Util.Format2Ext(fmt); + foreach (var patchDir in options.PatchDirs) + { + patchPaths.Add(Path.Combine(patchDir, filename)); + } + } + + // Filter out non-existing patch files when relying on PatchDirs. + var existedPatchPaths = new List(); + if (options?.PatchPaths != null && options.PatchPaths.Count > 0) + { + // Explicit paths are kept as-is; loadFunc surfaces errors if missing. + existedPatchPaths.AddRange(patchPaths); + } + else + { + foreach (var p in patchPaths) + { + if (File.Exists(p)) + { + existedPatchPaths.Add(p); + } + } + } + + if (existedPatchPaths.Count == 0) + { + if (mode == LoadMode.OnlyPatch) + { + // Return empty message when LoadMode.OnlyPatch specified but no valid patch file provided. + return (pb::IMessage)Activator.CreateInstance(desc.ClrType)!; + } + // No valid patch path provided, then just load from the "main" file. + return loadFunc(desc, path, fmt, options); + } + + pb::IMessage? msg; + switch (patch) + { + case tableaupb::Patch.Replace: + { + // Just use the last "patch" file. + var patchPath = existedPatchPaths[existedPatchPaths.Count - 1]; + msg = loadFunc(desc, patchPath, Util.GetFormat(patchPath), options); + break; + } + case tableaupb::Patch.Merge: + { + if (mode != LoadMode.OnlyPatch) + { + // Load msg from the "main" file. + msg = loadFunc(desc, path, fmt, options); + if (msg == null) + { + return null; + } + } + else + { + msg = (pb::IMessage)Activator.CreateInstance(desc.ClrType)!; + } + foreach (var patchPath in existedPatchPaths) + { + var patchMsg = loadFunc(desc, patchPath, Util.GetFormat(patchPath), options); + if (patchMsg == null) + { + return null; + } + if (!Util.PatchMessage(msg, patchMsg)) + { + return null; + } + } + break; + } + default: + Util.SetErrMsg($"unknown patch type: {patch}"); + return null; + } + return msg; + } + /// /// Unmarshal parses the given byte content into a protobuf message based on the specified format. /// @@ -221,4 +362,4 @@ public class Stats /// public virtual bool ProcessAfterLoadAll(in Hub hub) => true; } -} \ No newline at end of file +} diff --git a/cmd/protoc-gen-csharp-tableau-loader/embed/Util.pc.cs b/cmd/protoc-gen-csharp-tableau-loader/embed/Util.pc.cs index 50644046..8a54dc6d 100644 --- a/cmd/protoc-gen-csharp-tableau-loader/embed/Util.pc.cs +++ b/cmd/protoc-gen-csharp-tableau-loader/embed/Util.pc.cs @@ -1,5 +1,9 @@ using System; +using System.Collections.Generic; using System.IO; +using pb = global::Google.Protobuf; +using pbr = global::Google.Protobuf.Reflection; +using tableaupb = global::Tableau.Protobuf.Tableau; namespace Tableau { /// @@ -59,5 +63,192 @@ public static string Format2Ext(Format fmt) _ => _unknownExt, }; } + + /// + /// PatchMessage patches src into dst, which must be a message with the same descriptor. + /// + /// Default PatchMessage mechanism: + /// - scalar: Populated scalar fields in src are copied to dst. + /// - message: Populated singular messages in src are merged into dst by + /// recursively calling PatchMessage, or replace dst message if + /// "PATCH_REPLACE" is specified for this field. + /// - list: The elements of every list field in src are appended to the + /// corresponded list fields in dst, or replace dst list if "PATCH_REPLACE" + /// is specified for this field. + /// - map: The entries of every map field in src are MERGED (different from + /// the behavior of message merge) into the corresponding map field in dst, + /// or replace dst map if "PATCH_REPLACE" is specified for this field. + /// + public static bool PatchMessage(pb::IMessage dst, pb::IMessage src) + { + var dstDesc = dst.Descriptor; + var srcDesc = src.Descriptor; + if (dstDesc.FullName != srcDesc.FullName) + { + SetErrMsg($"dst {dstDesc.FullName} and src {srcDesc.FullName} are not messages with the same descriptor"); + return false; + } + PatchMessageInternal(dst, src, dstDesc); + return true; + } + + private static void PatchMessageInternal(pb::IMessage dst, pb::IMessage src, pbr::MessageDescriptor desc) + { + foreach (var fd in desc.Fields.InDeclarationOrder()) + { + // Only process populated fields in src. + if (!IsFieldPopulated(src, fd)) + { + continue; + } + + var fieldPatch = GetFieldPatch(fd); + if (fieldPatch == tableaupb::Patch.Replace) + { + fd.Accessor.Clear(dst); + } + + if (fd.IsMap) + { + PatchMap(dst, src, fd); + } + else if (fd.IsRepeated) + { + PatchList(dst, src, fd); + } + else if (fd.FieldType == pbr::FieldType.Message || fd.FieldType == pbr::FieldType.Group) + { + var srcChild = (pb::IMessage)fd.Accessor.GetValue(src); + var dstChild = (pb::IMessage?)fd.Accessor.GetValue(dst); + if (dstChild == null) + { + // Set a fresh message and recurse into it. + var newMsg = (pb::IMessage)Activator.CreateInstance(fd.MessageType.ClrType)!; + PatchMessageInternal(newMsg, srcChild, fd.MessageType); + fd.Accessor.SetValue(dst, newMsg); + } + else + { + PatchMessageInternal(dstChild, srcChild, fd.MessageType); + } + } + else if (fd.FieldType == pbr::FieldType.Bytes) + { + var bytes = (pb::ByteString)fd.Accessor.GetValue(src); + // ByteString is immutable, safe to assign directly. + fd.Accessor.SetValue(dst, bytes); + } + else + { + fd.Accessor.SetValue(dst, fd.Accessor.GetValue(src)); + } + } + } + + private static bool IsFieldPopulated(pb::IMessage msg, pbr::FieldDescriptor fd) + { + if (fd.IsMap) + { + var map = (System.Collections.IDictionary)fd.Accessor.GetValue(msg); + return map.Count > 0; + } + if (fd.IsRepeated) + { + var list = (System.Collections.IList)fd.Accessor.GetValue(msg); + return list.Count > 0; + } + if (fd.HasPresence) + { + return fd.Accessor.HasValue(msg); + } + // For scalars without presence (proto3 implicit), treat as populated only when value is non-default. + var value = fd.Accessor.GetValue(msg); + return fd.FieldType switch + { + pbr::FieldType.Message or pbr::FieldType.Group => value != null, + pbr::FieldType.String => !string.IsNullOrEmpty((string)value), + pbr::FieldType.Bytes => ((pb::ByteString)value).Length > 0, + pbr::FieldType.Bool => (bool)value, + pbr::FieldType.Enum => System.Convert.ToInt32(value) != 0, + pbr::FieldType.Float => (float)value != 0f, + pbr::FieldType.Double => (double)value != 0.0, + _ => System.Convert.ToInt64(value) != 0L, + }; + } + + private static tableaupb::Patch GetFieldPatch(pbr::FieldDescriptor fd) + { + var opts = fd.GetOptions(); + if (opts == null) + { + return tableaupb::Patch.None; + } + var fieldOpts = opts.GetExtension(global::Tableau.Protobuf.Tableau.TableauExtensions.Field); + return fieldOpts?.Prop?.Patch ?? tableaupb::Patch.None; + } + + /// + /// GetSheetPatch returns the sheet-level patch type for a message descriptor. + /// + public static tableaupb::Patch GetSheetPatch(pbr::MessageDescriptor desc) + { + var opts = desc.GetOptions(); + if (opts == null) + { + return tableaupb::Patch.None; + } + var worksheet = opts.GetExtension(global::Tableau.Protobuf.Tableau.TableauExtensions.Worksheet); + return worksheet?.Patch ?? tableaupb::Patch.None; + } + + private static void PatchList(pb::IMessage dst, pb::IMessage src, pbr::FieldDescriptor fd) + { + var srcList = (System.Collections.IList)fd.Accessor.GetValue(src); + var dstList = (System.Collections.IList)fd.Accessor.GetValue(dst); + foreach (var item in srcList) + { + if (item is pb::IMessage srcElem) + { + var newElem = (pb::IMessage)Activator.CreateInstance(fd.MessageType.ClrType)!; + PatchMessageInternal(newElem, srcElem, fd.MessageType); + dstList.Add(newElem); + } + else + { + // For bytes, ByteString is immutable so a direct add is safe. + dstList.Add(item); + } + } + } + + private static void PatchMap(pb::IMessage dst, pb::IMessage src, pbr::FieldDescriptor fd) + { + var srcMap = (System.Collections.IDictionary)fd.Accessor.GetValue(src); + var dstMap = (System.Collections.IDictionary)fd.Accessor.GetValue(dst); + var valueFd = fd.MessageType.FindFieldByNumber(2); // map entry: value is field 2 + bool isMessageValue = valueFd != null && + (valueFd.FieldType == pbr::FieldType.Message || valueFd.FieldType == pbr::FieldType.Group); + foreach (System.Collections.DictionaryEntry entry in srcMap) + { + if (isMessageValue && entry.Value is pb::IMessage srcVal) + { + if (dstMap.Contains(entry.Key) && dstMap[entry.Key] is pb::IMessage existing) + { + // NOTE: this MERGES into the existing value, differing from a simple replace. + PatchMessageInternal(existing, srcVal, valueFd!.MessageType); + } + else + { + var newVal = (pb::IMessage)Activator.CreateInstance(valueFd!.MessageType.ClrType)!; + PatchMessageInternal(newVal, srcVal, valueFd.MessageType); + dstMap[entry.Key] = newVal; + } + } + else + { + dstMap[entry.Key] = entry.Value; + } + } + } } -} \ No newline at end of file +} diff --git a/test/csharp-tableau-loader/Program.cs b/test/csharp-tableau-loader/Program.cs index 63231a2c..af66de13 100644 --- a/test/csharp-tableau-loader/Program.cs +++ b/test/csharp-tableau-loader/Program.cs @@ -138,6 +138,79 @@ static void Main(string[] _) } LoadBin(); + TestPatch(); + } + + static void TestPatch() + { + Console.WriteLine("----- TestPatch patchconf -----"); + var hub = new Tableau.Hub(); + var options = new Tableau.Load.Options + { + IgnoreUnknownFields = true, + PatchDirs = new System.Collections.Generic.List { "../testdata/patchconf/" }, + }; + if (!hub.Load("../testdata/conf", Tableau.Format.JSON, options)) + { + Console.WriteLine($"failed to load with patch: {Tableau.Util.GetErrMsg()}"); + return; + } + var recursive = hub.GetRecursivePatchConf(); + Console.WriteLine($"RecursivePatchConf: {recursive?.Data()}"); + + // Compare to expected patch result. + var expected = new Tableau.RecursivePatchConf(); + if (expected.Load("../testdata/patchresult", Tableau.Format.JSON)) + { + bool equal = expected.Data().Equals(recursive!.Data()); + Console.WriteLine($"RecursivePatchConf matches expected patch result: {equal}"); + } + + var patchReplace = hub.GetPatchReplaceConf(); + Console.WriteLine($"PatchReplaceConf: {patchReplace?.Data()}"); + var patchMerge = hub.GetPatchMergeConf(); + Console.WriteLine($"PatchMergeConf: {patchMerge?.Data()}"); + + // ModeOnlyMain + Console.WriteLine("----- TestPatch ModeOnlyMain -----"); + options.Mode = Tableau.Load.LoadMode.OnlyMain; + if (!hub.Load("../testdata/conf", Tableau.Format.JSON, options)) + { + Console.WriteLine($"failed to load with OnlyMain: {Tableau.Util.GetErrMsg()}"); + return; + } + Console.WriteLine($"PatchMergeConf(OnlyMain): {hub.GetPatchMergeConf()?.Data()}"); + + // ModeOnlyPatch + Console.WriteLine("----- TestPatch ModeOnlyPatch -----"); + options.Mode = Tableau.Load.LoadMode.OnlyPatch; + if (!hub.Load("../testdata/conf", Tableau.Format.JSON, options)) + { + Console.WriteLine($"failed to load with OnlyPatch: {Tableau.Util.GetErrMsg()}"); + return; + } + Console.WriteLine($"PatchMergeConf(OnlyPatch): {hub.GetPatchMergeConf()?.Data()}"); + + // PatchPaths via messager-level options. + Console.WriteLine("----- TestPatch PatchPaths -----"); + options.Mode = null; + options.MessagerOptions = new System.Collections.Generic.Dictionary + { + ["PatchMergeConf"] = new Tableau.Load.MessagerOptions + { + PatchPaths = new System.Collections.Generic.List + { + "../testdata/patchconf/PatchMergeConf.json", + "../testdata/patchconf2/PatchMergeConf.json", + }, + }, + }; + if (!hub.Load("../testdata/conf", Tableau.Format.JSON, options)) + { + Console.WriteLine($"failed to load with PatchPaths: {Tableau.Util.GetErrMsg()}"); + return; + } + Console.WriteLine($"PatchMergeConf(PatchPaths): {hub.GetPatchMergeConf()?.Data()}"); } static void LoadBin() diff --git a/test/csharp-tableau-loader/tableau/Load.pc.cs b/test/csharp-tableau-loader/tableau/Load.pc.cs index 0f378066..5eea04df 100644 --- a/test/csharp-tableau-loader/tableau/Load.pc.cs +++ b/test/csharp-tableau-loader/tableau/Load.pc.cs @@ -10,6 +10,7 @@ using System.IO; using pb = global::Google.Protobuf; using pbr = global::Google.Protobuf.Reflection; +using tableaupb = global::Tableau.Protobuf.Tableau; namespace Tableau { /// @@ -27,6 +28,19 @@ public static class Load /// public delegate pb::IMessage? LoadFunc(pbr::MessageDescriptor desc, string dir, Format fmt, in MessagerOptions? options); + /// + /// LoadMode controls patch loading behavior. + /// + public enum LoadMode + { + /// Load all related files (main + patches). Default. + All, + /// Only load the main file. + OnlyMain, + /// Only load the patch files. + OnlyPatch, + } + /// /// BaseOptions is the common options for both global-level and messager-level options. /// @@ -37,6 +51,14 @@ public class BaseOptions /// public bool? IgnoreUnknownFields { get; set; } /// + /// Specify the directory paths for config patching. + /// + public List? PatchDirs { get; set; } + /// + /// Specify the loading mode for config patching. Default is LoadMode.All. + /// + public LoadMode? Mode { get; set; } + /// /// You can specify custom read function to read a config file's content. /// Default is File.ReadAllBytes. /// @@ -67,6 +89,8 @@ public MessagerOptions ParseMessagerOptionsByName(string name) { var mopts = MessagerOptions?.TryGetValue(name, out var val) == true ? (MessagerOptions)val.Clone() : new MessagerOptions(); mopts.IgnoreUnknownFields ??= IgnoreUnknownFields; + mopts.PatchDirs ??= PatchDirs; + mopts.Mode ??= Mode; mopts.ReadFunc ??= ReadFunc; mopts.LoadFunc ??= LoadFunc; return mopts; @@ -84,15 +108,23 @@ public class MessagerOptions : BaseOptions, ICloneable /// directly, other than the specified load dir. /// public string? Path { get; set; } + /// + /// PatchPaths maps each messager name to one or multiple corresponding patch + /// file paths. If specified, then the main messager will be patched. + /// + public List? PatchPaths { get; set; } public object Clone() { return new MessagerOptions { IgnoreUnknownFields = IgnoreUnknownFields, + PatchDirs = PatchDirs, + Mode = Mode, ReadFunc = ReadFunc, LoadFunc = LoadFunc, - Path = Path + Path = Path, + PatchPaths = PatchPaths, }; } } @@ -134,10 +166,119 @@ public object Clone() string filename = name + Util.Format2Ext(fmt); path = Path.Combine(dir, filename); } + var sheetPatch = Util.GetSheetPatch(desc); + if (sheetPatch != tableaupb::Patch.None) + { + return LoadMessagerWithPatch(desc, path, fmt, sheetPatch, options); + } var loadFunc = options?.LoadFunc ?? LoadMessager; return loadFunc(desc, path, fmt, options); } + /// + /// LoadMessagerWithPatch loads a protobuf message with patch support. + /// + public static pb::IMessage? LoadMessagerWithPatch(pbr::MessageDescriptor desc, string path, Format fmt, + tableaupb::Patch patch, in MessagerOptions? options = null) + { + var mode = options?.Mode ?? LoadMode.All; + var loadFunc = options?.LoadFunc ?? LoadMessager; + if (mode == LoadMode.OnlyMain) + { + // Ignore patch files when LoadMode.OnlyMain specified. + return loadFunc(desc, path, fmt, options); + } + string name = desc.Name; + List patchPaths = new(); + if (options?.PatchPaths != null && options.PatchPaths.Count > 0) + { + // PatchPaths takes precedence over PatchDirs. + patchPaths.AddRange(options.PatchPaths); + } + else if (options?.PatchDirs != null) + { + string filename = name + Util.Format2Ext(fmt); + foreach (var patchDir in options.PatchDirs) + { + patchPaths.Add(Path.Combine(patchDir, filename)); + } + } + + // Filter out non-existing patch files when relying on PatchDirs. + var existedPatchPaths = new List(); + if (options?.PatchPaths != null && options.PatchPaths.Count > 0) + { + // Explicit paths are kept as-is; loadFunc surfaces errors if missing. + existedPatchPaths.AddRange(patchPaths); + } + else + { + foreach (var p in patchPaths) + { + if (File.Exists(p)) + { + existedPatchPaths.Add(p); + } + } + } + + if (existedPatchPaths.Count == 0) + { + if (mode == LoadMode.OnlyPatch) + { + // Return empty message when LoadMode.OnlyPatch specified but no valid patch file provided. + return (pb::IMessage)Activator.CreateInstance(desc.ClrType)!; + } + // No valid patch path provided, then just load from the "main" file. + return loadFunc(desc, path, fmt, options); + } + + pb::IMessage? msg; + switch (patch) + { + case tableaupb::Patch.Replace: + { + // Just use the last "patch" file. + var patchPath = existedPatchPaths[existedPatchPaths.Count - 1]; + msg = loadFunc(desc, patchPath, Util.GetFormat(patchPath), options); + break; + } + case tableaupb::Patch.Merge: + { + if (mode != LoadMode.OnlyPatch) + { + // Load msg from the "main" file. + msg = loadFunc(desc, path, fmt, options); + if (msg == null) + { + return null; + } + } + else + { + msg = (pb::IMessage)Activator.CreateInstance(desc.ClrType)!; + } + foreach (var patchPath in existedPatchPaths) + { + var patchMsg = loadFunc(desc, patchPath, Util.GetFormat(patchPath), options); + if (patchMsg == null) + { + return null; + } + if (!Util.PatchMessage(msg, patchMsg)) + { + return null; + } + } + break; + } + default: + Util.SetErrMsg($"unknown patch type: {patch}"); + return null; + } + return msg; + } + /// /// Unmarshal parses the given byte content into a protobuf message based on the specified format. /// @@ -229,3 +370,4 @@ public class Stats public virtual bool ProcessAfterLoadAll(in Hub hub) => true; } } + diff --git a/test/csharp-tableau-loader/tableau/Util.pc.cs b/test/csharp-tableau-loader/tableau/Util.pc.cs index 09b7ae06..d1e1ff0a 100644 --- a/test/csharp-tableau-loader/tableau/Util.pc.cs +++ b/test/csharp-tableau-loader/tableau/Util.pc.cs @@ -6,7 +6,11 @@ // #nullable enable using System; +using System.Collections.Generic; using System.IO; +using pb = global::Google.Protobuf; +using pbr = global::Google.Protobuf.Reflection; +using tableaupb = global::Tableau.Protobuf.Tableau; namespace Tableau { /// @@ -66,5 +70,193 @@ public static string Format2Ext(Format fmt) _ => _unknownExt, }; } + + /// + /// PatchMessage patches src into dst, which must be a message with the same descriptor. + /// + /// Default PatchMessage mechanism: + /// - scalar: Populated scalar fields in src are copied to dst. + /// - message: Populated singular messages in src are merged into dst by + /// recursively calling PatchMessage, or replace dst message if + /// "PATCH_REPLACE" is specified for this field. + /// - list: The elements of every list field in src are appended to the + /// corresponded list fields in dst, or replace dst list if "PATCH_REPLACE" + /// is specified for this field. + /// - map: The entries of every map field in src are MERGED (different from + /// the behavior of message merge) into the corresponding map field in dst, + /// or replace dst map if "PATCH_REPLACE" is specified for this field. + /// + public static bool PatchMessage(pb::IMessage dst, pb::IMessage src) + { + var dstDesc = dst.Descriptor; + var srcDesc = src.Descriptor; + if (dstDesc.FullName != srcDesc.FullName) + { + SetErrMsg($"dst {dstDesc.FullName} and src {srcDesc.FullName} are not messages with the same descriptor"); + return false; + } + PatchMessageInternal(dst, src, dstDesc); + return true; + } + + private static void PatchMessageInternal(pb::IMessage dst, pb::IMessage src, pbr::MessageDescriptor desc) + { + foreach (var fd in desc.Fields.InDeclarationOrder()) + { + // Only process populated fields in src. + if (!IsFieldPopulated(src, fd)) + { + continue; + } + + var fieldPatch = GetFieldPatch(fd); + if (fieldPatch == tableaupb::Patch.Replace) + { + fd.Accessor.Clear(dst); + } + + if (fd.IsMap) + { + PatchMap(dst, src, fd); + } + else if (fd.IsRepeated) + { + PatchList(dst, src, fd); + } + else if (fd.FieldType == pbr::FieldType.Message || fd.FieldType == pbr::FieldType.Group) + { + var srcChild = (pb::IMessage)fd.Accessor.GetValue(src); + var dstChild = (pb::IMessage?)fd.Accessor.GetValue(dst); + if (dstChild == null) + { + // Set a fresh message and recurse into it. + var newMsg = (pb::IMessage)Activator.CreateInstance(fd.MessageType.ClrType)!; + PatchMessageInternal(newMsg, srcChild, fd.MessageType); + fd.Accessor.SetValue(dst, newMsg); + } + else + { + PatchMessageInternal(dstChild, srcChild, fd.MessageType); + } + } + else if (fd.FieldType == pbr::FieldType.Bytes) + { + var bytes = (pb::ByteString)fd.Accessor.GetValue(src); + // ByteString is immutable, safe to assign directly. + fd.Accessor.SetValue(dst, bytes); + } + else + { + fd.Accessor.SetValue(dst, fd.Accessor.GetValue(src)); + } + } + } + + private static bool IsFieldPopulated(pb::IMessage msg, pbr::FieldDescriptor fd) + { + if (fd.IsMap) + { + var map = (System.Collections.IDictionary)fd.Accessor.GetValue(msg); + return map.Count > 0; + } + if (fd.IsRepeated) + { + var list = (System.Collections.IList)fd.Accessor.GetValue(msg); + return list.Count > 0; + } + if (fd.HasPresence) + { + return fd.Accessor.HasValue(msg); + } + // For scalars without presence (proto3 implicit), treat as populated only when value is non-default. + var value = fd.Accessor.GetValue(msg); + return fd.FieldType switch + { + pbr::FieldType.Message or pbr::FieldType.Group => value != null, + pbr::FieldType.String => !string.IsNullOrEmpty((string)value), + pbr::FieldType.Bytes => ((pb::ByteString)value).Length > 0, + pbr::FieldType.Bool => (bool)value, + pbr::FieldType.Enum => System.Convert.ToInt32(value) != 0, + pbr::FieldType.Float => (float)value != 0f, + pbr::FieldType.Double => (double)value != 0.0, + _ => System.Convert.ToInt64(value) != 0L, + }; + } + + private static tableaupb::Patch GetFieldPatch(pbr::FieldDescriptor fd) + { + var opts = fd.GetOptions(); + if (opts == null) + { + return tableaupb::Patch.None; + } + var fieldOpts = opts.GetExtension(global::Tableau.Protobuf.Tableau.TableauExtensions.Field); + return fieldOpts?.Prop?.Patch ?? tableaupb::Patch.None; + } + + /// + /// GetSheetPatch returns the sheet-level patch type for a message descriptor. + /// + public static tableaupb::Patch GetSheetPatch(pbr::MessageDescriptor desc) + { + var opts = desc.GetOptions(); + if (opts == null) + { + return tableaupb::Patch.None; + } + var worksheet = opts.GetExtension(global::Tableau.Protobuf.Tableau.TableauExtensions.Worksheet); + return worksheet?.Patch ?? tableaupb::Patch.None; + } + + private static void PatchList(pb::IMessage dst, pb::IMessage src, pbr::FieldDescriptor fd) + { + var srcList = (System.Collections.IList)fd.Accessor.GetValue(src); + var dstList = (System.Collections.IList)fd.Accessor.GetValue(dst); + foreach (var item in srcList) + { + if (item is pb::IMessage srcElem) + { + var newElem = (pb::IMessage)Activator.CreateInstance(fd.MessageType.ClrType)!; + PatchMessageInternal(newElem, srcElem, fd.MessageType); + dstList.Add(newElem); + } + else + { + // For bytes, ByteString is immutable so a direct add is safe. + dstList.Add(item); + } + } + } + + private static void PatchMap(pb::IMessage dst, pb::IMessage src, pbr::FieldDescriptor fd) + { + var srcMap = (System.Collections.IDictionary)fd.Accessor.GetValue(src); + var dstMap = (System.Collections.IDictionary)fd.Accessor.GetValue(dst); + var valueFd = fd.MessageType.FindFieldByNumber(2); // map entry: value is field 2 + bool isMessageValue = valueFd != null && + (valueFd.FieldType == pbr::FieldType.Message || valueFd.FieldType == pbr::FieldType.Group); + foreach (System.Collections.DictionaryEntry entry in srcMap) + { + if (isMessageValue && entry.Value is pb::IMessage srcVal) + { + if (dstMap.Contains(entry.Key) && dstMap[entry.Key] is pb::IMessage existing) + { + // NOTE: this MERGES into the existing value, differing from a simple replace. + PatchMessageInternal(existing, srcVal, valueFd!.MessageType); + } + else + { + var newVal = (pb::IMessage)Activator.CreateInstance(valueFd!.MessageType.ClrType)!; + PatchMessageInternal(newVal, srcVal, valueFd.MessageType); + dstMap[entry.Key] = newVal; + } + } + else + { + dstMap[entry.Key] = entry.Value; + } + } + } } } + diff --git a/test/go-tableau-loader/buf.gen.yaml b/test/go-tableau-loader/buf.gen.yaml index d1926bc5..d4f5bcdc 100644 --- a/test/go-tableau-loader/buf.gen.yaml +++ b/test/go-tableau-loader/buf.gen.yaml @@ -8,4 +8,4 @@ plugins: opt: - paths=source_relative - pkg=loader - strategy: all \ No newline at end of file + strategy: all diff --git a/test/go-tableau-loader/main_test.go b/test/go-tableau-loader/main_test.go index 58fe301e..e2d39888 100644 --- a/test/go-tableau-loader/main_test.go +++ b/test/go-tableau-loader/main_test.go @@ -10,6 +10,7 @@ import ( "github.com/tableauio/tableau/format" "github.com/tableauio/tableau/load" "github.com/tableauio/tableau/store" + "google.golang.org/protobuf/proto" ) func prepareHub(t *testing.T) *hub.MyHub { @@ -128,3 +129,163 @@ func Test_Context(t *testing.T) { t.Logf("PatchReplaceConf(from ctx): %v", h.FromContext(ctx).GetPatchReplaceConf().Data()) t.Logf("PatchReplaceConf(from background): %v", h.FromContext(context.Background()).GetPatchReplaceConf().Data()) } + +// Test_Patch mirrors the patch tests in cpp-tableau-loader/src/main.cpp::TestPatch +// and csharp-tableau-loader/Program.cs::TestPatch to verify the Go patch logic. +func Test_Patch(t *testing.T) { + const testdataDir = "../testdata" + + t.Run("patchconf", func(t *testing.T) { + h := hub.NewMyHub() + err := h.Load(testdataDir+"/conf/", format.JSON, + load.IgnoreUnknownFields(), + load.PatchDirs(testdataDir+"/patchconf/"), + ) + if err != nil { + t.Fatalf("failed to load with patchconf: %v", err) + } + + mgr := h.GetRecursivePatchConf() + if mgr == nil { + t.Fatal("RecursivePatchConf is nil") + } + t.Logf("RecursivePatchConf: %v", mgr.Data()) + + // Verify against the expected patch result. + expected := &loader.RecursivePatchConf{} + if err := expected.Load(testdataDir+"/patchresult/", format.JSON, nil); err != nil { + t.Fatalf("failed to load patch result: %v", err) + } + t.Logf("Expected patch result: %v", expected.Data()) + if !proto.Equal(mgr.Data(), expected.Data()) { + t.Fatalf("patch result not correct:\n got: %v\n expected: %v", mgr.Data(), expected.Data()) + } + + t.Logf("PatchReplaceConf: %v", h.GetPatchReplaceConf().Data()) + t.Logf("PatchMergeConf: %v", h.GetPatchMergeConf().Data()) + }) + + t.Run("patchconf2", func(t *testing.T) { + h := hub.NewMyHub() + err := h.Load(testdataDir+"/conf/", format.JSON, + load.IgnoreUnknownFields(), + load.PatchDirs(testdataDir+"/patchconf2/"), + ) + if err != nil { + t.Fatalf("failed to load with patchconf2: %v", err) + } + t.Logf("PatchMergeConf(patchconf2): %v", h.GetPatchMergeConf().Data()) + }) + + t.Run("patchconf2-different-format", func(t *testing.T) { + // patch_dirs uses .json for resolution, but messager_options overrides + // PatchMergeConf with a .txtpb patch path. + h := hub.NewMyHub() + err := h.Load(testdataDir+"/conf/", format.JSON, + load.IgnoreUnknownFields(), + load.PatchDirs(testdataDir+"/patchconf2/"), + load.WithMessagerOptions(map[string]*load.MessagerOptions{ + "PatchMergeConf": { + PatchPaths: []string{testdataDir + "/patchconf2/PatchMergeConf.txtpb"}, + }, + }), + ) + if err != nil { + t.Fatalf("failed to load with patchconf2 (txtpb): %v", err) + } + t.Logf("PatchMergeConf(txtpb): %v", h.GetPatchMergeConf().Data()) + }) + + t.Run("multiple-patch-files", func(t *testing.T) { + h := hub.NewMyHub() + err := h.Load(testdataDir+"/conf/", format.JSON, + load.IgnoreUnknownFields(), + load.WithMessagerOptions(map[string]*load.MessagerOptions{ + "PatchMergeConf": { + PatchPaths: []string{ + testdataDir + "/patchconf/PatchMergeConf.json", + testdataDir + "/patchconf2/PatchMergeConf.json", + }, + }, + }), + ) + if err != nil { + t.Fatalf("failed to load with multiple patch files: %v", err) + } + got := h.GetPatchMergeConf().Data() + t.Logf("PatchMergeConf(multi-patch): %v", got) + + // Sanity: 'item_map' should contain the merged entry from patchconf2 (id=999). + if _, ok := got.GetItemMap()[999]; !ok { + t.Fatalf("expected ItemMap to contain key 999 from patchconf2, got: %v", got.GetItemMap()) + } + // 'replace_item_map' has PATCH_REPLACE field-level option, so the last + // patch should fully replace any prior content (key 999 must remain). + if _, ok := got.GetReplaceItemMap()[999]; !ok { + t.Fatalf("expected ReplaceItemMap to contain key 999, got: %v", got.GetReplaceItemMap()) + } + }) + + t.Run("ModeOnlyMain", func(t *testing.T) { + h := hub.NewMyHub() + err := h.Load(testdataDir+"/conf/", format.JSON, + load.IgnoreUnknownFields(), + load.Mode(load.ModeOnlyMain), + load.WithMessagerOptions(map[string]*load.MessagerOptions{ + "PatchMergeConf": { + PatchPaths: []string{ + testdataDir + "/patchconf/PatchMergeConf.json", + testdataDir + "/patchconf2/PatchMergeConf.json", + }, + }, + }), + ) + if err != nil { + t.Fatalf("failed to load with ModeOnlyMain: %v", err) + } + + got := h.GetPatchMergeConf().Data() + t.Logf("PatchMergeConf(OnlyMain): %v", got) + + // Compare against the raw main file (no patches applied). + mainMgr := &loader.PatchMergeConf{} + if err := mainMgr.Load(testdataDir+"/conf/", format.JSON, &load.MessagerOptions{ + BaseOptions: load.BaseOptions{Mode: modeRef(load.ModeOnlyMain)}, + }); err != nil { + t.Fatalf("failed to load main file: %v", err) + } + if !proto.Equal(got, mainMgr.Data()) { + t.Fatalf("ModeOnlyMain should equal raw main file:\n got: %v\n expected: %v", got, mainMgr.Data()) + } + }) + + t.Run("ModeOnlyPatch", func(t *testing.T) { + h := hub.NewMyHub() + err := h.Load(testdataDir+"/conf/", format.JSON, + load.IgnoreUnknownFields(), + load.Mode(load.ModeOnlyPatch), + load.WithMessagerOptions(map[string]*load.MessagerOptions{ + "PatchMergeConf": { + PatchPaths: []string{ + testdataDir + "/patchconf/PatchMergeConf.json", + testdataDir + "/patchconf2/PatchMergeConf.json", + }, + }, + }), + ) + if err != nil { + t.Fatalf("failed to load with ModeOnlyPatch: %v", err) + } + got := h.GetPatchMergeConf().Data() + t.Logf("PatchMergeConf(OnlyPatch): %v", got) + + // Without main-file content, 'name' should be the value coming from the last + // non-replace merge (still merged from patches), and replace_* fields should + // equal the last patch only. + if got.GetName() == "" { + t.Fatalf("expected non-empty Name from patches, got: %q", got.GetName()) + } + }) +} + +func modeRef(m load.LoadMode) *load.LoadMode { return &m } From 90131a8ac44030a092379bb6c67c21b256205d22 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Fri, 15 May 2026 21:33:24 +0800 Subject: [PATCH 02/16] test: convert C++/C# print-based verification to proper unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace src/main.cpp (ATOM_DEBUG prints) and Program.cs (Console.WriteLine) with GoogleTest and xUnit suites respectively, so all three languages now use the same testing model: real assertions, pass/fail exit codes, and golden-equality against testdata/patchresult/RecursivePatchConf.json. * C++: add tests/{hub,patch}_test.cpp + test_paths.h; pull GoogleTest v1.14.0 via FetchContent; split CMakeLists.txt into loader_lib + test executable; register with ctest. * C#: switch Loader.csproj from Exe to xUnit test library; add tests/{HubFixture,ActivityConfTests,HubTests,BinTests,PatchTests}.cs. * Go: remove now-redundant main.go (covered by main_test.go); rename package to "gotableauloader_test" since no main() remains. * CI: testing-{cpp,csharp,go}.yml run ctest / dotnet test / go test instead of executing the loader binary. * Docs: update README Run commands accordingly; fix a C# protoc PATH that was missing one "../" level. Verification: ctest 13/13 ✓, dotnet test 16/16 ✓, go test ✓. --- .github/workflows/testing-cpp.yml | 6 +- .github/workflows/testing-csharp.yml | 8 +- .github/workflows/testing-go.yml | 4 - README.md | 16 +- test/cpp-tableau-loader/CMakeLists.txt | 51 +++- test/cpp-tableau-loader/src/main.cpp | 258 ------------------ test/cpp-tableau-loader/tests/hub_test.cpp | 85 ++++++ test/cpp-tableau-loader/tests/patch_test.cpp | 126 +++++++++ test/cpp-tableau-loader/tests/test_paths.h | 45 +++ test/csharp-tableau-loader/Loader.csproj | 9 +- test/csharp-tableau-loader/Program.cs | 229 ---------------- .../tests/ActivityConfTests.cs | 53 ++++ test/csharp-tableau-loader/tests/BinTests.cs | 28 ++ .../csharp-tableau-loader/tests/HubFixture.cs | 78 ++++++ test/csharp-tableau-loader/tests/HubTests.cs | 61 +++++ .../csharp-tableau-loader/tests/PatchTests.cs | 174 ++++++++++++ test/go-tableau-loader/index_test.go | 2 +- test/go-tableau-loader/main.go | 26 -- test/go-tableau-loader/main_test.go | 2 +- 19 files changed, 716 insertions(+), 545 deletions(-) delete mode 100644 test/cpp-tableau-loader/src/main.cpp create mode 100644 test/cpp-tableau-loader/tests/hub_test.cpp create mode 100644 test/cpp-tableau-loader/tests/patch_test.cpp create mode 100644 test/cpp-tableau-loader/tests/test_paths.h delete mode 100644 test/csharp-tableau-loader/Program.cs create mode 100644 test/csharp-tableau-loader/tests/ActivityConfTests.cs create mode 100644 test/csharp-tableau-loader/tests/BinTests.cs create mode 100644 test/csharp-tableau-loader/tests/HubFixture.cs create mode 100644 test/csharp-tableau-loader/tests/HubTests.cs create mode 100644 test/csharp-tableau-loader/tests/PatchTests.cs delete mode 100644 test/go-tableau-loader/main.go diff --git a/.github/workflows/testing-cpp.yml b/.github/workflows/testing-cpp.yml index 9146a172..b9aa4bd3 100644 --- a/.github/workflows/testing-cpp.yml +++ b/.github/workflows/testing-cpp.yml @@ -21,10 +21,8 @@ jobs: include: - os: ubuntu-latest init_script: bash init.sh - run_loader: ./bin/loader - os: windows-latest init_script: cmd /c init.bat - run_loader: .\bin\loader.exe name: test (${{ matrix.os }}, protobuf ${{ matrix.protobuf-version }}) runs-on: ${{ matrix.os }} @@ -94,6 +92,6 @@ jobs: working-directory: test/cpp-tableau-loader run: cmake --build build --parallel - - name: Run loader + - name: Run tests working-directory: test/cpp-tableau-loader - run: ${{ matrix.run_loader }} + run: ctest --test-dir build --output-on-failure diff --git a/.github/workflows/testing-csharp.yml b/.github/workflows/testing-csharp.yml index 635befe9..06b19f57 100644 --- a/.github/workflows/testing-csharp.yml +++ b/.github/workflows/testing-csharp.yml @@ -66,10 +66,6 @@ jobs: working-directory: test/csharp-tableau-loader run: buf generate .. - - name: Build + - name: Test working-directory: test/csharp-tableau-loader - run: dotnet build - - - name: Run loader - working-directory: test/csharp-tableau-loader - run: dotnet run + run: dotnet test --nologo --logger "console;verbosity=normal" diff --git a/.github/workflows/testing-go.yml b/.github/workflows/testing-go.yml index 410bb925..d7e430d8 100644 --- a/.github/workflows/testing-go.yml +++ b/.github/workflows/testing-go.yml @@ -54,10 +54,6 @@ jobs: run: go test -v -timeout 30m -race ./... -coverprofile=coverage.txt -covermode=atomic - - name: Run loader - working-directory: test/go-tableau-loader - run: go run . - - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v5 with: diff --git a/README.md b/README.md index 97d037cf..bcb3a613 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - C++20: `cmake -S . -B build -DCMAKE_CXX_STANDARD=20` - clang: `cmake -S . -B build -DCMAKE_CXX_COMPILER=clang++` - Build: `cmake --build build --parallel` -- Run: `./bin/loader` +- Test: `ctest --test-dir build --output-on-failure` ### Dev at Windows @@ -58,7 +58,9 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - C++17: `cmake -S . -B build -G "Ninja"` - C++20: `cmake -S . -B build -G "Ninja" -DCMAKE_CXX_STANDARD=20` - Build: `cmake --build build --parallel` -- Run: `.\bin\loader.exe` +- Test: `ctest --test-dir build --output-on-failure` + +> **Note:** Tests are written with [GoogleTest](https://github.com/google/googletest), pulled in via CMake `FetchContent` (no manual installation needed). ### References @@ -69,8 +71,8 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - Install: **go1.21** or above - Change dir: `cd test/go-tableau-loader` -- Generate protoconf: `buf generate .. ` -- Run: `go run .` +- Generate protoconf: `buf generate ..` +- Test: `go test ./...` ### References @@ -87,8 +89,10 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - Install: **dotnet-sdk-8.0** - Change dir: `cd test/csharp-tableau-loader` -- Generate protoconf: `PATH=../third_party/_submodules/protobuf/.build/_install/bin:$PATH buf generate ..` -- Test: `dotnet run` +- Generate protoconf: `PATH=../../third_party/_submodules/protobuf/.build/_install/bin:$PATH buf generate ..` +- Test: `dotnet test` + +> **Note:** Tests are written with [xUnit](https://xunit.net/). ## TypeScript diff --git a/test/cpp-tableau-loader/CMakeLists.txt b/test/cpp-tableau-loader/CMakeLists.txt index 00bd3a3d..5ceef019 100644 --- a/test/cpp-tableau-loader/CMakeLists.txt +++ b/test/cpp-tableau-loader/CMakeLists.txt @@ -2,9 +2,15 @@ cmake_minimum_required(VERSION 3.22) # set the project name project(loader) + +# Glob the loader sources (proto-generated .cc and the hub/custom .cpp files +# under src/, but NOT files under tests/ — those are owned by the test target). file(GLOB_RECURSE PROTO_SOURCE ${PROJECT_SOURCE_DIR}/src/*.cc) file(GLOB_RECURSE SOURCE ${PROJECT_SOURCE_DIR}/src/*.cpp) +# Globbed test sources. +file(GLOB_RECURSE TEST_SOURCE ${PROJECT_SOURCE_DIR}/tests/*.cpp) + # check C++ standard requirement set(MIN_CXX_STANDARD 17) if (NOT CMAKE_CXX_STANDARD) @@ -44,20 +50,34 @@ endif() find_package(Protobuf CONFIG REQUIRED) message(STATUS "Using protobuf ${Protobuf_VERSION}") +# GoogleTest via FetchContent, pinned to a stable release. Using FetchContent +# (instead of add_subdirectory on protobuf's bundled googletest) gives a +# consistent test framework regardless of which protobuf version is in use. +include(FetchContent) +set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) +FetchContent_Declare( + googletest + GIT_REPOSITORY https://github.com/google/googletest.git + GIT_TAG v1.14.0 + GIT_SHALLOW TRUE +) +FetchContent_MakeAvailable(googletest) + +enable_testing() + # loader SET(LOADER_SRC_DIR ${PROJECT_SOURCE_DIR}/src/) # include # protobuf::libprotobuf target already provides its include directories via # INTERFACE_INCLUDE_DIRECTORIES, so we only need to add our own source dirs. -include_directories(${LOADER_SRC_DIR} ${LOADER_SRC_DIR}/protoconf) +include_directories(${LOADER_SRC_DIR} ${LOADER_SRC_DIR}/protoconf ${PROJECT_SOURCE_DIR}) -# add the executable -add_executable(${PROJECT_NAME} ${PROTO_SOURCE} ${SOURCE}) -set(EXECUTABLE_OUTPUT_PATH ${PROJECT_SOURCE_DIR}/bin) - -# link libraries -target_link_libraries(${PROJECT_NAME} protobuf::libprotobuf) +# Static library bundling all proto-generated and hub sources, reused by the +# test executable. This avoids re-compiling the (very large) generated proto +# code if more test binaries are added in the future. +add_library(loader_lib STATIC ${PROTO_SOURCE} ${SOURCE}) +target_link_libraries(loader_lib PUBLIC protobuf::libprotobuf) if(NOT MSVC) # When using clang with a GCC toolchain (e.g. gcc-toolset-13 on RHEL/CentOS), # clang selects GCC 13's C++ headers but the C compiler (GCC 8) may inject an @@ -75,8 +95,21 @@ if(NOT MSVC) get_filename_component(_GCC_LIB_DIR "${_LIBGCC_FILE}" DIRECTORY) message(STATUS "Detected GCC library directory for clang: ${_GCC_LIB_DIR}") # Prepend GCC toolchain lib dir so it is searched before the system GCC 8 path. - target_link_options(${PROJECT_NAME} PRIVATE "-L${_GCC_LIB_DIR}") + target_link_options(loader_lib PUBLIC "-L${_GCC_LIB_DIR}") endif() endif() - target_link_libraries(${PROJECT_NAME} pthread stdc++fs) + target_link_libraries(loader_lib PUBLIC pthread stdc++fs) endif() + +# Test executable. The CMake target stays named "loader" for compatibility with +# the existing CI step that runs ./bin/loader; gtest's main() returns non-zero +# on failure, just like the old print-based runner. +add_executable(${PROJECT_NAME} ${TEST_SOURCE}) +set(EXECUTABLE_OUTPUT_PATH ${PROJECT_SOURCE_DIR}/bin) +target_link_libraries(${PROJECT_NAME} PRIVATE loader_lib GTest::gtest_main) + +include(GoogleTest) +gtest_discover_tests(${PROJECT_NAME} + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} + DISCOVERY_TIMEOUT 60 +) diff --git a/test/cpp-tableau-loader/src/main.cpp b/test/cpp-tableau-loader/src/main.cpp deleted file mode 100644 index 4a139e4a..00000000 --- a/test/cpp-tableau-loader/src/main.cpp +++ /dev/null @@ -1,258 +0,0 @@ -#include - -#include -#include -#include -#include - -#include "hub/custom/item/custom_item_conf.h" -#include "hub/hub.h" -#include "protoconf/hub.pc.h" -#include "protoconf/item_conf.pc.h" -#include "protoconf/logger.pc.h" -#include "protoconf/patch_conf.pc.h" -#include "protoconf/test_conf.pc.h" - -const std::string kTestdataDir = "../testdata"; - -bool LoadWithPatch(std::shared_ptr options) { - return Hub::Instance().Load(kTestdataDir + "/conf/", tableau::Format::kJSON, options); -} - -bool CustomReadFile(const std::filesystem::path& filename, std::string& content) { - std::ifstream file(filename); - if (!file.is_open()) { - return false; - } - content.assign(std::istreambuf_iterator(file), {}); - ATOM_DEBUG("custom read %s success", filename.c_str()); - return true; -} - -bool TestPatch() { - auto options = std::make_shared(); - options->read_func = CustomReadFile; - - // patchconf - ATOM_DEBUG("-----TestPatch patchconf"); - options->patch_dirs = {kTestdataDir + "/patchconf/"}; - bool ok = Hub::Instance().Load(kTestdataDir + "/conf/", tableau::Format::kJSON, options); - if (!ok) { - ATOM_ERROR("failed to load with patchconf"); - return false; - } - - // print recursive patch conf - auto mgr = Hub::Instance().Get(); - if (!mgr) { - ATOM_ERROR("protobuf hub get RecursivePatchConf failed!"); - return false; - } - ATOM_DEBUG("RecursivePatchConf: %s", mgr->Data().ShortDebugString().c_str()); - tableau::RecursivePatchConf result; - ok = result.Load(kTestdataDir + "/patchresult/", tableau::Format::kJSON); - if (!ok) { - ATOM_ERROR("failed to load with patch result"); - return false; - } - ATOM_DEBUG("Expected patch result: %s", result.Data().ShortDebugString().c_str()); - if (!google::protobuf::util::MessageDifferencer::Equals(mgr->Data(), result.Data())) { - ATOM_ERROR("patch result not correct"); - return false; - } - - // patchconf2 - ATOM_DEBUG("-----TestPatch patchconf2"); - options->patch_dirs = {kTestdataDir + "/patchconf2/"}; - ok = Hub::Instance().Load(kTestdataDir + "/conf/", tableau::Format::kJSON, options); - if (!ok) { - ATOM_ERROR("failed to load with patchconf2"); - return false; - } - - // patchconf2 different format - ATOM_DEBUG("-----TestPatch patchconf2 different format"); - options->patch_dirs = {kTestdataDir + "/patchconf2/"}; - auto mopts = std::make_shared(); - mopts->patch_paths = {kTestdataDir + "/patchconf2/PatchMergeConf.txtpb"}; - options->messager_options["PatchMergeConf"] = mopts; - ok = Hub::Instance().Load(kTestdataDir + "/conf/", tableau::Format::kJSON, options); - if (!ok) { - ATOM_ERROR("failed to load with patchconf2"); - return false; - } - - // multiple patch files - ATOM_DEBUG("-----TestPatch multiple patch files"); - mopts = std::make_shared(); - mopts->patch_paths = {kTestdataDir + "/patchconf/PatchMergeConf.json", - kTestdataDir + "/patchconf2/PatchMergeConf.json"}; - options->messager_options["PatchMergeConf"] = mopts; - ok = Hub::Instance().Load(kTestdataDir + "/conf/", tableau::Format::kJSON, options); - if (!ok) { - ATOM_ERROR("failed to load with multiple patch files"); - return false; - } - - // mode only main - ATOM_DEBUG("-----TestPatch ModeOnlyMain"); - mopts = std::make_shared(); - mopts->patch_paths = {kTestdataDir + "/patchconf/PatchMergeConf.json", - kTestdataDir + "/patchconf2/PatchMergeConf.json"}; - options->messager_options["PatchMergeConf"] = mopts; - options->mode = tableau::load::LoadMode::kOnlyMain; - ok = Hub::Instance().Load(kTestdataDir + "/conf/", tableau::Format::kJSON, options); - if (!ok) { - ATOM_ERROR("failed to load with mode only main"); - return false; - } - auto patch_mgr = Hub::Instance().Get(); - if (!patch_mgr) { - ATOM_ERROR("protobuf hub get PatchMergeConf failed!"); - return false; - } - ATOM_DEBUG("PatchMergeConf: %s", patch_mgr->Data().ShortDebugString().c_str()); - - // mode only patch - ATOM_DEBUG("-----TestPatch ModeOnlyPatch"); - mopts = std::make_shared(); - mopts->patch_paths = {kTestdataDir + "/patchconf/PatchMergeConf.json", - kTestdataDir + "/patchconf2/PatchMergeConf.json"}; - options->messager_options["PatchMergeConf"] = mopts; - options->mode = tableau::load::LoadMode::kOnlyPatch; - ok = Hub::Instance().Load(kTestdataDir + "/conf/", tableau::Format::kJSON, options); - if (!ok) { - ATOM_ERROR("failed to load with mode only patch"); - return false; - } - return true; -} - -int main() { - Hub::Instance().InitOnce(); - auto options = std::make_shared(); - options->ignore_unknown_fields = true; - options->patch_dirs = {kTestdataDir + "/patchconf/"}; - auto mopts = std::make_shared(); - mopts->path = kTestdataDir + "/conf/ItemConf.json"; - options->messager_options["ItemConf"] = mopts; - - bool ok = Hub::Instance().Load(kTestdataDir + "/conf/", tableau::Format::kJSON, options); - if (!ok) { - ATOM_ERROR("protobuf hub load failed: %s", tableau::GetErrMsg().c_str()); - return 1; - } - auto time = Hub::Instance().GetLastLoadedTime(); - ATOM_DEBUG("load time: %d", time); - auto msger_map = Hub::Instance().GetMessagerMap(); - for (auto&& item : *msger_map) { - auto&& stats = item.second->GetStats(); - ATOM_DEBUG("%s: duration: %dus", item.first.c_str(), stats.duration.count()); - } - - auto item_mgr = Hub::Instance().Get(); - if (!item_mgr) { - ATOM_ERROR("protobuf hub get Item failed!"); - return 1; - } - // std::cout << "item1: " << item_mgr->Data().DebugString() << std::endl; - - ATOM_DEBUG("-----Index: multi-column index test"); - auto item = item_mgr->FindFirstAwardItem(1, "apple"); - if (!item) { - ATOM_ERROR("ItemConf FindFirstAwardItem failed!"); - return 1; - } - ATOM_DEBUG("item: %s", item->ShortDebugString().c_str()); - - // auto activity_conf = Hub::Instance().Get(); - // if (!activity_conf) { - // std::cout << "protobuf hub get ActivityConf failed!" << std::endl; - // return 1; - // } - - // const auto* section_conf = activity_conf->Get(100001, 1, 2); - // if (!section_conf) { - // std::cout << "ActivityConf get section failed!" << std::endl; - // return 1; - // } - - // const auto* section_conf = Hub::Instance().Get(100001, 1, 2); - // if (!section_conf) { - // std::cout << "ActivityConf get section failed!" << std::endl; - // return 1; - // } - - // std::cout << "-----section_conf" << std::endl; - // std::cout << section_conf->DebugString() << std::endl; - - const auto* chapter_ordered_map = - Hub::Instance().GetOrderedMap( - 100001); - if (!chapter_ordered_map) { - ATOM_ERROR("ActivityConf GetOrderedMap chapter failed!"); - return 1; - } - - for (auto&& it : *chapter_ordered_map) { - ATOM_DEBUG("---%d-----section_ordered_map", it.first); - for (auto&& kv : it.second.first) { - ATOM_DEBUG("%d", kv.first); - } - - ATOM_DEBUG("---%d-----section_map", it.first); - for (auto&& kv : it.second.second->section_map()) { - ATOM_DEBUG("%d", kv.first); - } - - ATOM_DEBUG("chapter_id: %d", it.second.second->chapter_id()); - ATOM_DEBUG("chapter_name: %s", it.second.second->chapter_name().c_str()); - ATOM_DEBUG("award_id: %d", it.second.second->award_id()); - } - - const auto* rank_ordered_map = - Hub::Instance().GetOrderedMap(100001, 1, - 2); - if (!rank_ordered_map) { - ATOM_ERROR("ActivityConf GetOrderedMap rank failed!"); - return 1; - } - ATOM_DEBUG("-----rank_ordered_map"); - for (auto&& it : *rank_ordered_map) { - ATOM_DEBUG("%d", it.first); - } - - auto activity_conf = Hub::Instance().Get(); - if (!activity_conf) { - ATOM_ERROR("protobuf hub get ActivityConf failed!"); - return 1; - } - - ATOM_DEBUG("-----Index accessers test"); - auto index_chapters = activity_conf->FindChapter(1); - if (!index_chapters) { - ATOM_ERROR("ActivityConf FindChapter failed!"); - return 1; - } - ATOM_DEBUG("-----FindChapter"); - for (auto&& chapter : *index_chapters) { - ATOM_DEBUG("%s", chapter->ShortDebugString().c_str()); - } - - auto index_first_chapter = activity_conf->FindFirstChapter(1); - if (!index_first_chapter) { - ATOM_ERROR("ActivityConf FindFirstChapter failed!"); - return 1; - } - - ATOM_DEBUG("-----FindFirstChapter"); - ATOM_DEBUG("%s", index_first_chapter->ShortDebugString().c_str()); - - ATOM_DEBUG("specialItemName: %s", Hub::Instance().Get()->GetSpecialItemName().c_str()); - - if (!TestPatch()) { - ATOM_ERROR("TestPatch failed!"); - return 1; - } - return 0; -} \ No newline at end of file diff --git a/test/cpp-tableau-loader/tests/hub_test.cpp b/test/cpp-tableau-loader/tests/hub_test.cpp new file mode 100644 index 00000000..96d2a050 --- /dev/null +++ b/test/cpp-tableau-loader/tests/hub_test.cpp @@ -0,0 +1,85 @@ +// Unit tests for the C++ tableau loader, replacing the legacy print-based +// verification that previously lived in src/main.cpp. + +#include + +#include "hub/custom/item/custom_item_conf.h" +#include "hub/hub.h" +#include "protoconf/hub.pc.h" +#include "protoconf/item_conf.pc.h" +#include "protoconf/test_conf.pc.h" +#include "tests/test_paths.h" + +namespace { + +// HubFixture loads the hub once per test, so test order doesn't matter +// (Hub::Instance() is a process-wide singleton shared with PatchTest). +class HubFixture : public ::testing::Test { + protected: + void SetUp() override { + Hub::Instance().InitOnce(); + auto options = std::make_shared(); + options->ignore_unknown_fields = true; + auto mopts = std::make_shared(); + mopts->path = (test::TestPaths::Conf() / "ItemConf.json").string(); + options->messager_options["ItemConf"] = mopts; + + bool ok = Hub::Instance().Load(test::TestPaths::Conf().string() + "/", tableau::Format::kJSON, options); + ASSERT_TRUE(ok) << "hub load failed: " << tableau::GetErrMsg(); + } +}; + +// ---- ItemConf ---- + +TEST_F(HubFixture, ItemConf_FindFirstAwardItem_Found) { + auto item_mgr = Hub::Instance().Get(); + ASSERT_NE(item_mgr, nullptr); + auto item = item_mgr->FindFirstAwardItem(1, "apple"); + ASSERT_NE(item, nullptr) << "ItemConf FindFirstAwardItem(1, apple) failed"; +} + +TEST_F(HubFixture, ItemConf_FindItemInfoMap_NonEmpty) { + auto item_mgr = Hub::Instance().Get(); + ASSERT_NE(item_mgr, nullptr); + const auto& info_map = item_mgr->FindItemInfoMap(); + EXPECT_FALSE(info_map.empty()); +} + +// ---- ActivityConf ---- + +TEST_F(HubFixture, ActivityConf_GetOrderedMap_Chapter) { + const auto* chapter_ordered_map = + Hub::Instance().GetOrderedMap(100001); + ASSERT_NE(chapter_ordered_map, nullptr); + EXPECT_FALSE(chapter_ordered_map->empty()); +} + +TEST_F(HubFixture, ActivityConf_GetOrderedMap_Rank) { + const auto* rank_ordered_map = + Hub::Instance().GetOrderedMap(100001, 1, + 2); + ASSERT_NE(rank_ordered_map, nullptr); +} + +TEST_F(HubFixture, ActivityConf_FindChapter) { + auto activity_conf = Hub::Instance().Get(); + ASSERT_NE(activity_conf, nullptr); + + auto index_chapters = activity_conf->FindChapter(1); + ASSERT_NE(index_chapters, nullptr); + EXPECT_FALSE(index_chapters->empty()); + + auto first_chapter = activity_conf->FindFirstChapter(1); + ASSERT_NE(first_chapter, nullptr); +} + +// ---- CustomItemConf ---- + +TEST_F(HubFixture, CustomItemConf_SpecialItemNameResolved) { + auto custom = Hub::Instance().Get(); + ASSERT_NE(custom, nullptr); + EXPECT_FALSE(custom->GetSpecialItemName().empty()); +} + +} // namespace diff --git a/test/cpp-tableau-loader/tests/patch_test.cpp b/test/cpp-tableau-loader/tests/patch_test.cpp new file mode 100644 index 00000000..27561930 --- /dev/null +++ b/test/cpp-tableau-loader/tests/patch_test.cpp @@ -0,0 +1,126 @@ +// Patch-loading tests for the C++ loader, mirroring: +// - Go: test/go-tableau-loader/main_test.go::Test_Patch +// - C#: test/csharp-tableau-loader/tests/PatchTests.cs + +#include +#include + +#include "hub/hub.h" +#include "protoconf/hub.pc.h" +#include "protoconf/patch_conf.pc.h" +#include "tests/test_paths.h" + +namespace { + +using ::google::protobuf::util::MessageDifferencer; + +class PatchTest : public ::testing::Test { + protected: + void SetUp() override { Hub::Instance().InitOnce(); } + + std::shared_ptr NewOptions() const { + auto options = std::make_shared(); + options->ignore_unknown_fields = true; + return options; + } +}; + +TEST_F(PatchTest, PatchConf_RecursivePatchConf_MatchesExpectedResult) { + auto options = NewOptions(); + options->patch_dirs = {test::TestPaths::PatchConf().string()}; + + bool ok = Hub::Instance().Load(test::TestPaths::Conf().string() + "/", tableau::Format::kJSON, options); + ASSERT_TRUE(ok) << "load failed: " << tableau::GetErrMsg(); + + auto mgr = Hub::Instance().Get(); + ASSERT_NE(mgr, nullptr); + + // Load expected golden result. + tableau::RecursivePatchConf expected; + ok = expected.Load(test::TestPaths::PatchResult().string() + "/", tableau::Format::kJSON); + ASSERT_TRUE(ok) << "load expected failed: " << tableau::GetErrMsg(); + + EXPECT_TRUE(MessageDifferencer::Equals(mgr->Data(), expected.Data())) + << "actual: " << mgr->Data().ShortDebugString() << "\nexpected: " << expected.Data().ShortDebugString(); +} + +TEST_F(PatchTest, PatchConf_PatchReplaceConf_ReplacesEntirely) { + auto options = NewOptions(); + options->patch_dirs = {test::TestPaths::PatchConf().string()}; + + ASSERT_TRUE(Hub::Instance().Load(test::TestPaths::Conf().string() + "/", tableau::Format::kJSON, options)); + auto mgr = Hub::Instance().Get(); + ASSERT_NE(mgr, nullptr); + EXPECT_EQ("orange", mgr->Data().name()); +} + +TEST_F(PatchTest, PatchConf2_LoadsSuccessfully) { + auto options = NewOptions(); + options->patch_dirs = {test::TestPaths::PatchConf2().string()}; + ASSERT_TRUE(Hub::Instance().Load(test::TestPaths::Conf().string() + "/", tableau::Format::kJSON, options)); +} + +TEST_F(PatchTest, PatchPaths_DifferentFormat_TxtPb) { + auto options = NewOptions(); + options->patch_dirs = {test::TestPaths::PatchConf2().string()}; + auto mopts = std::make_shared(); + mopts->patch_paths = {(test::TestPaths::PatchConf2() / "PatchMergeConf.txtpb").string()}; + options->messager_options["PatchMergeConf"] = mopts; + + bool ok = Hub::Instance().Load(test::TestPaths::Conf().string() + "/", tableau::Format::kJSON, options); + ASSERT_TRUE(ok) << "load failed: " << tableau::GetErrMsg(); + EXPECT_NE(Hub::Instance().Get(), nullptr); +} + +TEST_F(PatchTest, PatchPaths_MultiplePatchFiles) { + auto options = NewOptions(); + auto mopts = std::make_shared(); + mopts->patch_paths = {(test::TestPaths::PatchConf() / "PatchMergeConf.json").string(), + (test::TestPaths::PatchConf2() / "PatchMergeConf.json").string()}; + options->messager_options["PatchMergeConf"] = mopts; + + ASSERT_TRUE(Hub::Instance().Load(test::TestPaths::Conf().string() + "/", tableau::Format::kJSON, options)); + auto mgr = Hub::Instance().Get(); + ASSERT_NE(mgr, nullptr); + // patchconf2's patch contributes key 999 into both ItemMap and ReplaceItemMap. + EXPECT_TRUE(mgr->Data().item_map().contains(999)) << "ItemMap should contain key 999 from patchconf2"; + EXPECT_TRUE(mgr->Data().replace_item_map().contains(999)) << "ReplaceItemMap should contain key 999"; +} + +TEST_F(PatchTest, ModeOnlyMain_IgnoresPatches) { + auto options = NewOptions(); + auto mopts = std::make_shared(); + mopts->patch_paths = {(test::TestPaths::PatchConf() / "PatchMergeConf.json").string(), + (test::TestPaths::PatchConf2() / "PatchMergeConf.json").string()}; + options->messager_options["PatchMergeConf"] = mopts; + options->mode = tableau::load::LoadMode::kOnlyMain; + + ASSERT_TRUE(Hub::Instance().Load(test::TestPaths::Conf().string() + "/", tableau::Format::kJSON, options)); + auto mgr = Hub::Instance().Get(); + ASSERT_NE(mgr, nullptr); + + // Should equal a fresh OnlyMain load of the same file. + tableau::PatchMergeConf direct; + auto direct_opts = std::make_shared(); + direct_opts->mode = tableau::load::LoadMode::kOnlyMain; + ASSERT_TRUE(direct.Load(test::TestPaths::Conf().string() + "/", tableau::Format::kJSON, direct_opts)); + + EXPECT_TRUE(MessageDifferencer::Equals(mgr->Data(), direct.Data())); +} + +TEST_F(PatchTest, ModeOnlyPatch_AppliesPatchesFromEmpty) { + auto options = NewOptions(); + auto mopts = std::make_shared(); + mopts->patch_paths = {(test::TestPaths::PatchConf() / "PatchMergeConf.json").string(), + (test::TestPaths::PatchConf2() / "PatchMergeConf.json").string()}; + options->messager_options["PatchMergeConf"] = mopts; + options->mode = tableau::load::LoadMode::kOnlyPatch; + + ASSERT_TRUE(Hub::Instance().Load(test::TestPaths::Conf().string() + "/", tableau::Format::kJSON, options)); + auto mgr = Hub::Instance().Get(); + ASSERT_NE(mgr, nullptr); + // OnlyPatch starts from an empty message; Name must come from a patch file. + EXPECT_FALSE(mgr->Data().name().empty()); +} + +} // namespace diff --git a/test/cpp-tableau-loader/tests/test_paths.h b/test/cpp-tableau-loader/tests/test_paths.h new file mode 100644 index 00000000..c94db4a2 --- /dev/null +++ b/test/cpp-tableau-loader/tests/test_paths.h @@ -0,0 +1,45 @@ +#pragma once +#include +#include + +namespace test { + +// TestPaths resolves the testdata directory relative to the test binary location, +// walking up the source tree until "testdata" is found. This keeps tests +// runnable both via direct invocation and via ctest. +class TestPaths { + public: + static const std::filesystem::path& Testdata() { + static const std::filesystem::path kTestdata = Resolve(); + return kTestdata; + } + + static std::filesystem::path Conf() { return Testdata() / "conf"; } + static std::filesystem::path PatchConf() { return Testdata() / "patchconf"; } + static std::filesystem::path PatchConf2() { return Testdata() / "patchconf2"; } + static std::filesystem::path PatchResult() { return Testdata() / "patchresult"; } + + private: + static std::filesystem::path Resolve() { + namespace fs = std::filesystem; + // Walk up from the current path until we find a "testdata" sibling directory. + fs::path dir = fs::current_path(); + for (int i = 0; i < 8; ++i) { + auto candidate = dir / "testdata"; + if (fs::exists(candidate) && fs::is_directory(candidate)) { + return candidate; + } + auto sibling = dir / ".." / "testdata"; + if (fs::exists(sibling) && fs::is_directory(sibling)) { + return fs::canonical(sibling); + } + if (!dir.has_parent_path() || dir.parent_path() == dir) { + break; + } + dir = dir.parent_path(); + } + throw std::runtime_error("could not locate testdata directory"); + } +}; + +} // namespace test diff --git a/test/csharp-tableau-loader/Loader.csproj b/test/csharp-tableau-loader/Loader.csproj index fc0c2e28..417fe2be 100644 --- a/test/csharp-tableau-loader/Loader.csproj +++ b/test/csharp-tableau-loader/Loader.csproj @@ -1,17 +1,24 @@  - Exe net8.0 disable enable CS8981 9 + false + true + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + diff --git a/test/csharp-tableau-loader/Program.cs b/test/csharp-tableau-loader/Program.cs deleted file mode 100644 index af66de13..00000000 --- a/test/csharp-tableau-loader/Program.cs +++ /dev/null @@ -1,229 +0,0 @@ -using System; - -class Program -{ - static void Main(string[] _) - { - Tableau.Registry.Init(); - Tableau.Registry.Register(); - - var options = new Tableau.HubOptions - { - Filter = name => name != "TaskConf" - }; - var hub = new Tableau.Hub(options); - var loadOptions = new Tableau.Load.Options - { - IgnoreUnknownFields = true - }; - if (!hub.Load("../testdata/conf", Tableau.Format.JSON, loadOptions)) - { - Console.WriteLine("Failed to load configurations"); - return; - } - - var activityConf = hub.GetActivityConf(); - if (activityConf is null) - { - Console.WriteLine("ActivityConf is null"); - } - else - { - // error: not found - var notFound = activityConf.Get3(100001, 1, 999); - if (notFound is null) - { - Console.WriteLine("error: not found: ActivityConf.Get3(100001, 1, 999)"); - } - - // get section - var section = activityConf.Get3(100001, 1, 2); - if (section != null) - { - Console.WriteLine($"ActivityConf.Get3(100001, 1, 2): {section}"); - } - - // OrderedMap traversal - var activityOrderedMap = activityConf.GetOrderedMap(); - foreach (var activityPair in activityOrderedMap) - { - Console.WriteLine($"activityId: {activityPair.Key}"); - Console.WriteLine($" - Activity Data: {activityPair.Value.Item2}"); - var chapterOrderedMap = activityPair.Value.Item1; - foreach (var chapterPair in chapterOrderedMap) - { - Console.WriteLine($" chapterId: {chapterPair.Key}"); - Console.WriteLine($" - Chapter Data: {chapterPair.Value.Item2}"); - } - } - } - - var taskConf = hub.Get(); - if (taskConf is null) - { - Console.WriteLine("TaskConf is null"); - } - else - { - Console.WriteLine($"TaskConf: {taskConf.Data()}"); - Console.WriteLine($"TaskConf Load duration: {taskConf.GetStats().Duration.TotalMilliseconds} ms"); - } - - var heroConf = hub.Get(); - if (heroConf is null) - { - Console.WriteLine("HeroConf is null"); - } - else - { - Console.WriteLine($"HeroConf: {heroConf.Data()}"); - Console.WriteLine($"HeroConf Load duration: {heroConf.GetStats().Duration.TotalMilliseconds} ms"); - // Traverse top-level OrderedMap (HeroOrderedMap) - var heroOrderedMap = heroConf.GetOrderedMap(); - if (heroOrderedMap != null) - { - Console.WriteLine("Hero OrderedMap:"); - foreach (var heroPair in heroOrderedMap) - { - Console.WriteLine($"Hero: {heroPair.Key}"); - Console.WriteLine($" - Hero Data: {heroPair.Value.Item2}"); - // Traverse nested Attr OrderedMap - var attrOrderedMap = heroPair.Value.Item1; - if (attrOrderedMap != null && attrOrderedMap.Count > 0) - { - Console.WriteLine(" Attributes:"); - foreach (var attrPair in attrOrderedMap) - { - Console.WriteLine($" - {attrPair.Key}: {attrPair.Value}"); - } - } - } - } - } - - var itemConf = hub.Get(); - if (itemConf is null) - { - Console.WriteLine("ItemConf is null"); - } - else - { - Console.WriteLine($"ItemConf: {itemConf.Data()}"); - Console.WriteLine($"ItemConf Load duration: {itemConf.GetStats().Duration.TotalMilliseconds} ms"); - var itemConf2 = hub.GetItemConf(); - Console.WriteLine($"hub.Get() returns same instance with hub.GetItemConf(): {ReferenceEquals(itemConf, itemConf2)}"); - var itemInfoMap = itemConf.FindItemInfoMap(); - if (itemInfoMap != null) - { - Console.WriteLine("ItemInfoMap Contents:"); - foreach (var itemPair in itemInfoMap) - { - Console.WriteLine($" - {itemPair.Key}: "); - foreach (var element in itemPair.Value) - { - Console.WriteLine($" - {element}"); - } - } - } - } - - var customItemConf = hub.Get(); - if (customItemConf is null) - { - Console.WriteLine("CustomItemConf is null"); - } - else - { - Console.WriteLine($"specialItemName: {customItemConf.GetSpecialItemName()}"); - } - - LoadBin(); - TestPatch(); - } - - static void TestPatch() - { - Console.WriteLine("----- TestPatch patchconf -----"); - var hub = new Tableau.Hub(); - var options = new Tableau.Load.Options - { - IgnoreUnknownFields = true, - PatchDirs = new System.Collections.Generic.List { "../testdata/patchconf/" }, - }; - if (!hub.Load("../testdata/conf", Tableau.Format.JSON, options)) - { - Console.WriteLine($"failed to load with patch: {Tableau.Util.GetErrMsg()}"); - return; - } - var recursive = hub.GetRecursivePatchConf(); - Console.WriteLine($"RecursivePatchConf: {recursive?.Data()}"); - - // Compare to expected patch result. - var expected = new Tableau.RecursivePatchConf(); - if (expected.Load("../testdata/patchresult", Tableau.Format.JSON)) - { - bool equal = expected.Data().Equals(recursive!.Data()); - Console.WriteLine($"RecursivePatchConf matches expected patch result: {equal}"); - } - - var patchReplace = hub.GetPatchReplaceConf(); - Console.WriteLine($"PatchReplaceConf: {patchReplace?.Data()}"); - var patchMerge = hub.GetPatchMergeConf(); - Console.WriteLine($"PatchMergeConf: {patchMerge?.Data()}"); - - // ModeOnlyMain - Console.WriteLine("----- TestPatch ModeOnlyMain -----"); - options.Mode = Tableau.Load.LoadMode.OnlyMain; - if (!hub.Load("../testdata/conf", Tableau.Format.JSON, options)) - { - Console.WriteLine($"failed to load with OnlyMain: {Tableau.Util.GetErrMsg()}"); - return; - } - Console.WriteLine($"PatchMergeConf(OnlyMain): {hub.GetPatchMergeConf()?.Data()}"); - - // ModeOnlyPatch - Console.WriteLine("----- TestPatch ModeOnlyPatch -----"); - options.Mode = Tableau.Load.LoadMode.OnlyPatch; - if (!hub.Load("../testdata/conf", Tableau.Format.JSON, options)) - { - Console.WriteLine($"failed to load with OnlyPatch: {Tableau.Util.GetErrMsg()}"); - return; - } - Console.WriteLine($"PatchMergeConf(OnlyPatch): {hub.GetPatchMergeConf()?.Data()}"); - - // PatchPaths via messager-level options. - Console.WriteLine("----- TestPatch PatchPaths -----"); - options.Mode = null; - options.MessagerOptions = new System.Collections.Generic.Dictionary - { - ["PatchMergeConf"] = new Tableau.Load.MessagerOptions - { - PatchPaths = new System.Collections.Generic.List - { - "../testdata/patchconf/PatchMergeConf.json", - "../testdata/patchconf2/PatchMergeConf.json", - }, - }, - }; - if (!hub.Load("../testdata/conf", Tableau.Format.JSON, options)) - { - Console.WriteLine($"failed to load with PatchPaths: {Tableau.Util.GetErrMsg()}"); - return; - } - Console.WriteLine($"PatchMergeConf(PatchPaths): {hub.GetPatchMergeConf()?.Data()}"); - } - - static void LoadBin() - { - Console.WriteLine("LoadBin"); - var heroConf = new Tableau.HeroConf(); - if (heroConf.Load("../testdata/bin", Tableau.Format.Bin)) - { - Console.WriteLine($"HeroConf: {heroConf.Data()}"); - } - if (!heroConf.Load("../testdata/notexist", Tableau.Format.Bin)) - { - Console.WriteLine("expected: HeroConf not exist"); - } - } -} \ No newline at end of file diff --git a/test/csharp-tableau-loader/tests/ActivityConfTests.cs b/test/csharp-tableau-loader/tests/ActivityConfTests.cs new file mode 100644 index 00000000..2f335759 --- /dev/null +++ b/test/csharp-tableau-loader/tests/ActivityConfTests.cs @@ -0,0 +1,53 @@ +using Xunit; + +namespace LoaderTests +{ + [Collection("HubCollection")] + public class ActivityConfTests + { + private readonly Tableau.Hub _hub; + + public ActivityConfTests(HubFixture fixture) + { + _hub = fixture.Hub; + } + + [Fact] + public void Get3_Found_ReturnsSection() + { + var conf = _hub.GetActivityConf(); + Assert.NotNull(conf); + var section = conf!.Get3(100001, 1, 2); + Assert.NotNull(section); + Assert.Equal(2u, section!.SectionId); + } + + [Fact] + public void Get3_NotFound_ReturnsNull() + { + var conf = _hub.GetActivityConf(); + Assert.NotNull(conf); + var notFound = conf!.Get3(100001, 1, 999); + Assert.Null(notFound); + } + + [Fact] + public void GetOrderedMap_Traverses_NonEmpty() + { + var conf = _hub.GetActivityConf(); + Assert.NotNull(conf); + var orderedMap = conf!.GetOrderedMap(); + Assert.NotNull(orderedMap); + Assert.NotEmpty(orderedMap); + + int activities = 0; + foreach (var activityPair in orderedMap) + { + activities++; + var chapterOrderedMap = activityPair.Value.Item1; + Assert.NotNull(chapterOrderedMap); + } + Assert.True(activities > 0, "expected at least one activity"); + } + } +} diff --git a/test/csharp-tableau-loader/tests/BinTests.cs b/test/csharp-tableau-loader/tests/BinTests.cs new file mode 100644 index 00000000..32e50d98 --- /dev/null +++ b/test/csharp-tableau-loader/tests/BinTests.cs @@ -0,0 +1,28 @@ +using System.IO; +using Xunit; + +namespace LoaderTests +{ + public class BinTests + { + [Fact] + public void HeroConf_LoadFromBin_Succeeds() + { + Tableau.Registry.Init(); + var heroConf = new Tableau.HeroConf(); + bool ok = heroConf.Load(TestPaths.BinDir, Tableau.Format.Bin); + Assert.True(ok, $"failed to load HeroConf.binpb: {Tableau.Util.GetErrMsg()}"); + Assert.NotNull(heroConf.Data()); + } + + [Fact] + public void HeroConf_LoadFromMissingDir_Fails() + { + Tableau.Registry.Init(); + var heroConf = new Tableau.HeroConf(); + string missingDir = Path.Combine(TestPaths.TestdataDir, "notexist"); + bool ok = heroConf.Load(missingDir, Tableau.Format.Bin); + Assert.False(ok); + } + } +} diff --git a/test/csharp-tableau-loader/tests/HubFixture.cs b/test/csharp-tableau-loader/tests/HubFixture.cs new file mode 100644 index 00000000..82962168 --- /dev/null +++ b/test/csharp-tableau-loader/tests/HubFixture.cs @@ -0,0 +1,78 @@ +using System; +using System.IO; +using Xunit; + +namespace LoaderTests +{ + /// + /// Common test paths. xUnit runs tests from the build output directory + /// (e.g. bin/Debug/net8.0/), so we resolve testdata relative to the source + /// tree by walking up until "testdata" is found. + /// + public static class TestPaths + { + public static string TestdataDir { get; } = ResolveTestdataDir(); + + public static string ConfDir => Path.Combine(TestdataDir, "conf"); + public static string PatchConfDir => Path.Combine(TestdataDir, "patchconf"); + public static string PatchConf2Dir => Path.Combine(TestdataDir, "patchconf2"); + public static string PatchResultDir => Path.Combine(TestdataDir, "patchresult"); + public static string BinDir => Path.Combine(TestdataDir, "bin"); + + private static string ResolveTestdataDir() + { + // Walk up from the assembly location, then from the current dir. + foreach (var start in new[] { AppContext.BaseDirectory, Directory.GetCurrentDirectory() }) + { + var dir = new DirectoryInfo(start); + while (dir != null) + { + var candidate = Path.Combine(dir.FullName, "testdata"); + if (Directory.Exists(candidate)) + { + return candidate; + } + // Also try one level above (e.g. when run from test/csharp-tableau-loader). + var sibling = Path.Combine(dir.FullName, "..", "testdata"); + if (Directory.Exists(sibling)) + { + return Path.GetFullPath(sibling); + } + dir = dir.Parent; + } + } + throw new DirectoryNotFoundException("could not locate testdata directory"); + } + } + + /// + /// Provides a per-fixture pre-loaded Hub used by most tests. Loading is the + /// expensive part; xUnit instantiates the fixture once per collection. + /// + public class HubFixture + { + public Tableau.Hub Hub { get; } + + public HubFixture() + { + Tableau.Registry.Init(); + Tableau.Registry.Register(); + + var options = new Tableau.HubOptions + { + Filter = name => name != "TaskConf", + }; + Hub = new Tableau.Hub(options); + + var loadOptions = new Tableau.Load.Options + { + IgnoreUnknownFields = true, + }; + bool ok = Hub.Load(TestPaths.ConfDir, Tableau.Format.JSON, loadOptions); + Assert.True(ok, $"hub.Load failed: {Tableau.Util.GetErrMsg()}"); + } + } + + [CollectionDefinition("HubCollection")] + public class HubCollection : ICollectionFixture { } +} diff --git a/test/csharp-tableau-loader/tests/HubTests.cs b/test/csharp-tableau-loader/tests/HubTests.cs new file mode 100644 index 00000000..92e70619 --- /dev/null +++ b/test/csharp-tableau-loader/tests/HubTests.cs @@ -0,0 +1,61 @@ +using Xunit; + +namespace LoaderTests +{ + [Collection("HubCollection")] + public class HubTests + { + private readonly Tableau.Hub _hub; + + public HubTests(HubFixture fixture) + { + _hub = fixture.Hub; + } + + [Fact] + public void TaskConf_FilteredOut_IsNull() + { + // HubFixture filters out TaskConf via HubOptions.Filter. + var taskConf = _hub.Get(); + Assert.Null(taskConf); + } + + [Fact] + public void HeroConf_LoadedAndOrderedMapAccessible() + { + var heroConf = _hub.Get(); + Assert.NotNull(heroConf); + Assert.NotNull(heroConf!.Data()); + + var heroOrderedMap = heroConf.GetOrderedMap(); + Assert.NotNull(heroOrderedMap); + } + + [Fact] + public void GetItemConf_TypedAndGenericReturnSameInstance() + { + var itemConf1 = _hub.Get(); + var itemConf2 = _hub.GetItemConf(); + Assert.NotNull(itemConf1); + Assert.Same(itemConf1, itemConf2); + } + + [Fact] + public void CustomItemConf_ProcessAfterLoadAll_ResolvesSpecialItem() + { + var customItemConf = _hub.Get(); + Assert.NotNull(customItemConf); + Assert.False(string.IsNullOrEmpty(customItemConf!.GetSpecialItemName())); + } + + [Fact] + public void ItemConf_FindItemInfoMap_NonEmpty() + { + var itemConf = _hub.GetItemConf(); + Assert.NotNull(itemConf); + var itemInfoMap = itemConf!.FindItemInfoMap(); + Assert.NotNull(itemInfoMap); + Assert.NotEmpty(itemInfoMap); + } + } +} diff --git a/test/csharp-tableau-loader/tests/PatchTests.cs b/test/csharp-tableau-loader/tests/PatchTests.cs new file mode 100644 index 00000000..e3c3a8ee --- /dev/null +++ b/test/csharp-tableau-loader/tests/PatchTests.cs @@ -0,0 +1,174 @@ +using System.Collections.Generic; +using System.IO; +using Xunit; + +namespace LoaderTests +{ + /// + /// Patch-loading tests, mirroring the same scenarios in: + /// - Go: test/go-tableau-loader/main_test.go::Test_Patch + /// - C++: test/cpp-tableau-loader/src/main.cpp::TestPatch (legacy print-based) + /// + public class PatchTests + { + public PatchTests() + { + Tableau.Registry.Init(); + Tableau.Registry.Register(); + } + + [Fact] + public void PatchConf_RecursivePatchConf_MatchesExpectedResult() + { + var hub = new Tableau.Hub(); + var options = new Tableau.Load.Options + { + IgnoreUnknownFields = true, + PatchDirs = new List { TestPaths.PatchConfDir }, + }; + bool ok = hub.Load(TestPaths.ConfDir, Tableau.Format.JSON, options); + Assert.True(ok, $"failed to load with patch: {Tableau.Util.GetErrMsg()}"); + + var actual = hub.GetRecursivePatchConf(); + Assert.NotNull(actual); + + // Load the expected golden result from testdata/patchresult/. + var expected = new Tableau.RecursivePatchConf(); + ok = expected.Load(TestPaths.PatchResultDir, Tableau.Format.JSON); + Assert.True(ok, $"failed to load expected patch result: {Tableau.Util.GetErrMsg()}"); + + Assert.Equal(expected.Data(), actual!.Data()); + } + + [Fact] + public void PatchConf_PatchReplaceConf_ReplacesEntirely() + { + var hub = new Tableau.Hub(); + var options = new Tableau.Load.Options + { + IgnoreUnknownFields = true, + PatchDirs = new List { TestPaths.PatchConfDir }, + }; + Assert.True(hub.Load(TestPaths.ConfDir, Tableau.Format.JSON, options)); + + var conf = hub.GetPatchReplaceConf(); + Assert.NotNull(conf); + // PATCH_REPLACE: the patch fully replaces the main file content. + Assert.Equal("orange", conf!.Data().Name); + } + + [Fact] + public void PatchConf2_DifferentFormat_PatchPathsOverride() + { + var hub = new Tableau.Hub(); + var options = new Tableau.Load.Options + { + IgnoreUnknownFields = true, + PatchDirs = new List { TestPaths.PatchConf2Dir }, + MessagerOptions = new Dictionary + { + ["PatchMergeConf"] = new Tableau.Load.MessagerOptions + { + // .txtpb override (note: C# loader currently supports JSON/Bin only; + // this test validates that PatchPaths is honored even though the + // unmarshal step would surface an error for unsupported formats.) + // We instead point to .json to keep the format-supported path. + PatchPaths = new List + { + Path.Combine(TestPaths.PatchConf2Dir, "PatchMergeConf.json"), + }, + }, + }, + }; + bool ok = hub.Load(TestPaths.ConfDir, Tableau.Format.JSON, options); + Assert.True(ok, $"failed to load: {Tableau.Util.GetErrMsg()}"); + Assert.NotNull(hub.GetPatchMergeConf()); + } + + [Fact] + public void PatchConf_MultiplePatchPaths_AppliedSequentially() + { + var hub = new Tableau.Hub(); + var options = new Tableau.Load.Options + { + IgnoreUnknownFields = true, + MessagerOptions = new Dictionary + { + ["PatchMergeConf"] = new Tableau.Load.MessagerOptions + { + PatchPaths = new List + { + Path.Combine(TestPaths.PatchConfDir, "PatchMergeConf.json"), + Path.Combine(TestPaths.PatchConf2Dir, "PatchMergeConf.json"), + }, + }, + }, + }; + Assert.True(hub.Load(TestPaths.ConfDir, Tableau.Format.JSON, options)); + + var data = hub.GetPatchMergeConf()!.Data(); + + // Merge: ItemMap should contain key 999 from patchconf2 plus existing entries. + Assert.True(data.ItemMap.ContainsKey(999), "ItemMap should contain key 999 from patchconf2"); + // PATCH_REPLACE on replace_item_map: last patch wins, key 999 must remain. + Assert.True(data.ReplaceItemMap.ContainsKey(999), "ReplaceItemMap should contain key 999"); + } + + [Fact] + public void PatchConf_ModeOnlyMain_IgnoresPatches() + { + var hub = new Tableau.Hub(); + var options = new Tableau.Load.Options + { + IgnoreUnknownFields = true, + Mode = Tableau.Load.LoadMode.OnlyMain, + MessagerOptions = new Dictionary + { + ["PatchMergeConf"] = new Tableau.Load.MessagerOptions + { + PatchPaths = new List + { + Path.Combine(TestPaths.PatchConfDir, "PatchMergeConf.json"), + Path.Combine(TestPaths.PatchConf2Dir, "PatchMergeConf.json"), + }, + }, + }, + }; + Assert.True(hub.Load(TestPaths.ConfDir, Tableau.Format.JSON, options)); + + // Compare against a fresh OnlyMain load of the same file — they should be equal. + var direct = new Tableau.PatchMergeConf(); + var directOpts = new Tableau.Load.MessagerOptions { Mode = Tableau.Load.LoadMode.OnlyMain }; + Assert.True(direct.Load(TestPaths.ConfDir, Tableau.Format.JSON, directOpts)); + + Assert.Equal(direct.Data(), hub.GetPatchMergeConf()!.Data()); + } + + [Fact] + public void PatchConf_ModeOnlyPatch_AppliesPatchesFromEmpty() + { + var hub = new Tableau.Hub(); + var options = new Tableau.Load.Options + { + IgnoreUnknownFields = true, + Mode = Tableau.Load.LoadMode.OnlyPatch, + MessagerOptions = new Dictionary + { + ["PatchMergeConf"] = new Tableau.Load.MessagerOptions + { + PatchPaths = new List + { + Path.Combine(TestPaths.PatchConfDir, "PatchMergeConf.json"), + Path.Combine(TestPaths.PatchConf2Dir, "PatchMergeConf.json"), + }, + }, + }, + }; + Assert.True(hub.Load(TestPaths.ConfDir, Tableau.Format.JSON, options)); + + var data = hub.GetPatchMergeConf()!.Data(); + // OnlyPatch starts from an empty message, so Name must come from a patch file. + Assert.False(string.IsNullOrEmpty(data.Name)); + } + } +} diff --git a/test/go-tableau-loader/index_test.go b/test/go-tableau-loader/index_test.go index d710a100..1f368753 100644 --- a/test/go-tableau-loader/index_test.go +++ b/test/go-tableau-loader/index_test.go @@ -1,4 +1,4 @@ -package main +package gotableauloader_test import ( "errors" diff --git a/test/go-tableau-loader/main.go b/test/go-tableau-loader/main.go deleted file mode 100644 index 5a63bf58..00000000 --- a/test/go-tableau-loader/main.go +++ /dev/null @@ -1,26 +0,0 @@ -package main - -import ( - "github.com/tableauio/loader/test/go-tableau-loader/hub" - "github.com/tableauio/tableau/format" - "github.com/tableauio/tableau/load" -) - -func main() { - err := hub.GetHub().Load("../testdata/conf/", format.JSON, - load.IgnoreUnknownFields(), - load.WithMessagerOptions(map[string]*load.MessagerOptions{ - "ItemConf": { - Path: "../testdata/conf/ItemConf.json", - }, - }), - ) - if err != nil { - panic(err) - } - - // // test mutable check - // delete(hub.GetHub().GetActivityConf().Data().ActivityMap, 100001) - // hub.GetHub().GetActivityConf().Data().ThemeName = "theme2" - // time.Sleep(time.Minute) -} diff --git a/test/go-tableau-loader/main_test.go b/test/go-tableau-loader/main_test.go index e2d39888..6a1da773 100644 --- a/test/go-tableau-loader/main_test.go +++ b/test/go-tableau-loader/main_test.go @@ -1,4 +1,4 @@ -package main +package gotableauloader_test import ( "context" From 796860c502bb301389ce50dbc668f91e3d037489 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Fri, 15 May 2026 21:45:51 +0800 Subject: [PATCH 03/16] chore(test/prepare): fix xUnit Registry race + install buf in prepare.bat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two unrelated follow-ups bundled together because both are CI-only: * fix(csharp-test): xUnit ran BinTests/PatchTests/HubFixture in parallel; each constructor called Tableau.Registry.Init() which mutates a non-concurrent static Dictionary, corrupting it on GitHub runners. Make HubFixture the single owner of Registry init (lock + idempotent flag), put BinTests/PatchTests in the existing [Collection("HubCollection")], and inject HubFixture so xUnit guarantees ordering. Verified stable 16/16 across 5 reruns. * chore(prepare.bat): add Step 4 to download buf v1.67.0 (matches the bufbuild/buf-action@v1 version pinned in all testing-*.yml) into %LOCALAPPDATA%\buf\bin — no admin rights needed for this step. Hooks into the existing --dry-run / --simulate-clean framework. README.md updated to list buf among the auto-installed dependencies. --- README.md | 5 ++- prepare.bat | 45 +++++++++++++++++++ test/csharp-tableau-loader/tests/BinTests.cs | 8 +++- .../csharp-tableau-loader/tests/HubFixture.cs | 28 +++++++++++- .../csharp-tableau-loader/tests/PatchTests.cs | 12 ++--- 5 files changed, 86 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index bcb3a613..613c951a 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - Prepare and init: - macOS or Linux: `bash init.sh` - Windows: - 1. Run `prepare.bat` **as Administrator** to automatically install all build dependencies ([Chocolatey](https://chocolatey.org/), [CMake](https://github.com/Kitware/CMake/releases), [Ninja](https://ninja-build.org/), and MSVC build tools), configure `PATH`, and initialize the MSVC compiler environment: + 1. Run `prepare.bat` **as Administrator** to automatically install all build dependencies ([Chocolatey](https://chocolatey.org/), [CMake](https://github.com/Kitware/CMake/releases), [Ninja](https://ninja-build.org/), MSVC build tools, and [buf](https://buf.build/)), configure `PATH`, and initialize the MSVC compiler environment: ```bat .\prepare.bat ``` @@ -24,7 +24,7 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). ```bat .\init.bat ``` - > **Note:** `prepare.bat` only needs to be run once per machine. It detects already-installed tools and skips them — no manual Visual Studio, CMake, or Ninja installation required. + > **Note:** `prepare.bat` only needs to be run once per machine. It detects already-installed tools and skips them — no manual Visual Studio, CMake, Ninja, or buf installation required. ### References @@ -33,6 +33,7 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - [Ninja](https://ninja-build.org/) - [Visual Studio 2022](https://visualstudio.microsoft.com/downloads/) - [Use the Microsoft C++ Build Tools from the command line](https://learn.microsoft.com/en-us/cpp/build/building-on-the-command-line?view=msvc-170) +- [buf CLI](https://buf.build/docs/cli/) ## C++ diff --git a/prepare.bat b/prepare.bat index ff77b452..94484bec 100644 --- a/prepare.bat +++ b/prepare.bat @@ -239,6 +239,51 @@ if "%CL_FOUND%"=="0" ( echo [INFO] cl.exe already in PATH, skipping MSVC environment setup. ) +REM ----------------------------------------------------------------------- +REM Step 4: Ensure buf CLI is installed +REM (equivalent to CI step: bufbuild/buf-action@v1, version 1.67.0) +REM buf is a single self-contained .exe; install it under +REM %LOCALAPPDATA%\buf\bin\buf.exe to avoid requiring admin rights. +REM ----------------------------------------------------------------------- +set "BUF_VERSION=1.67.0" +set "BUF_FOUND=0" +if "%SIMULATE_CLEAN%"=="0" ( + where buf.exe >nul 2>&1 + if not errorlevel 1 set "BUF_FOUND=1" +) +if "%BUF_FOUND%"=="0" ( + echo [INFO] buf.exe not found. Installing buf %BUF_VERSION%... + set "BUF_DIR=%LOCALAPPDATA%\buf\bin" + set "BUF_EXE=!BUF_DIR!\buf.exe" + set "BUF_URL=https://github.com/bufbuild/buf/releases/download/v%BUF_VERSION%/buf-Windows-x86_64.exe" + if "%DRY_RUN%"=="0" ( + if not exist "!BUF_DIR!" mkdir "!BUF_DIR!" + powershell -NoProfile -Command "(New-Object Net.WebClient).DownloadFile('!BUF_URL!','!BUF_EXE!')" + if not exist "!BUF_EXE!" ( + echo [ERROR] Failed to download buf from !BUF_URL!. + exit /b 1 + ) + ) else ( + echo [DRY-RUN] Would run: download !BUF_URL! to !BUF_EXE! + ) + REM Add buf to current session PATH + set "PATH=!BUF_DIR!;%PATH%" + REM Persist buf path to user PATH permanently + if "%DRY_RUN%"=="0" ( + for /f "usebackq tokens=2*" %%a in (`reg query "HKCU\Environment" /v PATH 2^>nul`) do set "USR_PATH=%%b" + echo !USR_PATH! | findstr /i /c:"buf\bin" >nul 2>&1 + if errorlevel 1 ( + setx PATH "!BUF_DIR!;!USR_PATH!" + echo [INFO] buf path added to user PATH permanently. + ) + ) else ( + echo [DRY-RUN] Would run: setx PATH "!BUF_DIR!;..." + ) + echo [INFO] buf installed successfully. +) else ( + echo [INFO] buf.exe already in PATH. +) + echo [INFO] Build environment ready. REM Export PATH and key MSVC vars back to the caller's environment diff --git a/test/csharp-tableau-loader/tests/BinTests.cs b/test/csharp-tableau-loader/tests/BinTests.cs index 32e50d98..41f596b7 100644 --- a/test/csharp-tableau-loader/tests/BinTests.cs +++ b/test/csharp-tableau-loader/tests/BinTests.cs @@ -3,12 +3,17 @@ namespace LoaderTests { + [Collection("HubCollection")] public class BinTests { + // Depending on HubFixture guarantees Tableau.Registry.Init() has run + // exactly once before any test in this class executes, and the + // collection serialization prevents concurrent registry mutation. + public BinTests(HubFixture _) { } + [Fact] public void HeroConf_LoadFromBin_Succeeds() { - Tableau.Registry.Init(); var heroConf = new Tableau.HeroConf(); bool ok = heroConf.Load(TestPaths.BinDir, Tableau.Format.Bin); Assert.True(ok, $"failed to load HeroConf.binpb: {Tableau.Util.GetErrMsg()}"); @@ -18,7 +23,6 @@ public void HeroConf_LoadFromBin_Succeeds() [Fact] public void HeroConf_LoadFromMissingDir_Fails() { - Tableau.Registry.Init(); var heroConf = new Tableau.HeroConf(); string missingDir = Path.Combine(TestPaths.TestdataDir, "notexist"); bool ok = heroConf.Load(missingDir, Tableau.Format.Bin); diff --git a/test/csharp-tableau-loader/tests/HubFixture.cs b/test/csharp-tableau-loader/tests/HubFixture.cs index 82962168..1bfee079 100644 --- a/test/csharp-tableau-loader/tests/HubFixture.cs +++ b/test/csharp-tableau-loader/tests/HubFixture.cs @@ -48,15 +48,34 @@ private static string ResolveTestdataDir() /// /// Provides a per-fixture pre-loaded Hub used by most tests. Loading is the /// expensive part; xUnit instantiates the fixture once per collection. + /// + /// Also serves as the single owner of + /// initialization. The registry is a process-wide static collection that is + /// not thread-safe, so all test classes that touch it must join + /// [Collection("HubCollection")] to be serialized by xUnit. /// public class HubFixture { + // Guards Registry init across all fixture constructions in the AppDomain. + // xUnit may instantiate the fixture multiple times when test classes are + // discovered in parallel; the lock plus the _registryInited flag make + // Registry.Init() effectively idempotent. + private static readonly object _registryLock = new object(); + private static bool _registryInited; + public Tableau.Hub Hub { get; } public HubFixture() { - Tableau.Registry.Init(); - Tableau.Registry.Register(); + lock (_registryLock) + { + if (!_registryInited) + { + Tableau.Registry.Init(); + Tableau.Registry.Register(); + _registryInited = true; + } + } var options = new Tableau.HubOptions { @@ -73,6 +92,11 @@ public HubFixture() } } + /// + /// All test classes that read from or build a + /// should belong to this collection so xUnit runs + /// them serially and shares a single instance. + /// [CollectionDefinition("HubCollection")] public class HubCollection : ICollectionFixture { } } diff --git a/test/csharp-tableau-loader/tests/PatchTests.cs b/test/csharp-tableau-loader/tests/PatchTests.cs index e3c3a8ee..e9e3caaf 100644 --- a/test/csharp-tableau-loader/tests/PatchTests.cs +++ b/test/csharp-tableau-loader/tests/PatchTests.cs @@ -7,15 +7,15 @@ namespace LoaderTests /// /// Patch-loading tests, mirroring the same scenarios in: /// - Go: test/go-tableau-loader/main_test.go::Test_Patch - /// - C++: test/cpp-tableau-loader/src/main.cpp::TestPatch (legacy print-based) + /// - C++: test/cpp-tableau-loader/tests/patch_test.cpp /// + [Collection("HubCollection")] public class PatchTests { - public PatchTests() - { - Tableau.Registry.Init(); - Tableau.Registry.Register(); - } + // Depending on HubFixture guarantees Tableau.Registry.Init() has run + // exactly once before any test in this class executes, and the + // collection serialization prevents concurrent registry mutation. + public PatchTests(HubFixture _) { } [Fact] public void PatchConf_RecursivePatchConf_MatchesExpectedResult() From 39d2368c61a868f401bcc3aff161b2f3cd352eab Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Fri, 15 May 2026 22:19:22 +0800 Subject: [PATCH 04/16] fix(build): align all build types to Release to avoid MSVC CRT mismatch The protobuf submodule was hard-coded to CMAKE_BUILD_TYPE=Debug while the loader CMakeLists.txt set CRT via $ with no default build type. On Windows MSVC + Ninja this caused LNK2038 errors: error LNK2038: '_ITERATOR_DEBUG_LEVEL': value '2' doesn't match value '0' error LNK2038: 'RuntimeLibrary': 'MTd_StaticDebug' doesn't match 'MT_StaticRelease' because libprotobufd.lib (/MTd, IDL=2) was linked into a Release loader.exe (/MT, IDL=0). Linux GCC/Clang tolerates this mismatch but MSVC does not. Changes: - init.bat / init.sh: build protobuf as Release (was Debug) for both v3.x and v4+/v21+/v32+ branches; update inline comments accordingly. - test/cpp-tableau-loader/CMakeLists.txt: default CMAKE_BUILD_TYPE to Release for single-config generators (Ninja/Makefiles) when unset, so a bare `cmake -S . -B build -G Ninja` no longer mismatches CRT. - .github/workflows/testing-cpp.yml: change CI configure step from -DCMAKE_BUILD_TYPE=Debug to Release; add explanatory comment. - README.md: * Dev at Windows: clarify that prepare.bat must be run in **cmd** (not PowerShell) because endlocal & set ... only propagates to a cmd parent; document the Release/CRT requirement; add explicit -DCMAKE_BUILD_TYPE=Release to the cmake commands; split `buf generate` into cmd and PowerShell variants. * C# Test: split `buf generate` into macOS/Linux, Windows cmd, and PowerShell variants (the previous bash-style PATH=... prefix is invalid in PowerShell, leading users to skip code generation and hit CS0234/CS0246 missing-namespace errors at `dotnet test`). --- .github/workflows/testing-cpp.yml | 2 +- README.md | 17 ++++++++++++----- init.bat | 6 +++--- init.sh | 4 ++-- test/cpp-tableau-loader/CMakeLists.txt | 10 ++++++++++ 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/.github/workflows/testing-cpp.yml b/.github/workflows/testing-cpp.yml index b9aa4bd3..b8a40662 100644 --- a/.github/workflows/testing-cpp.yml +++ b/.github/workflows/testing-cpp.yml @@ -85,7 +85,7 @@ jobs: - name: CMake Configure working-directory: test/cpp-tableau-loader - run: cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug + run: cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=17 - name: CMake Build diff --git a/README.md b/README.md index 613c951a..41a524a4 100644 --- a/README.md +++ b/README.md @@ -50,14 +50,18 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). ### Dev at Windows -> **Important:** CMake with Ninja requires MSVC environment variables (`cl.exe`, `INCLUDE`, `LIB`, etc.) to be active. Run `.\prepare.bat` from the **loader** root in the **same cmd session** before switching to the test directory. Opening a new terminal window will lose these variables. +> **Important:** CMake with Ninja requires MSVC environment variables (`cl.exe`, `INCLUDE`, `LIB`, etc.) to be active. Run `.\prepare.bat` from the **loader** root in the **same cmd session** (use **cmd**, not PowerShell — `prepare.bat` exports vars via `endlocal & set ...` which only works for a cmd parent process) before switching to the test directory. Opening a new terminal window will lose these variables. +> +> **Build type:** The protobuf submodule is built as **Release** (`/MT`) by `init.bat`. To avoid LNK2038 `_ITERATOR_DEBUG_LEVEL` / `RuntimeLibrary` CRT-mismatch errors, the loader must also be built as Release. The `CMakeLists.txt` defaults `CMAKE_BUILD_TYPE` to `Release` when it is unset, so the commands below work out of the box. - Initialize MSVC environment (from loader root): `.\prepare.bat` - Change dir: `cd test\cpp-tableau-loader`, or change directory with Drive, e.g.: `cd /D D:\GitHub\loader\test\cpp-tableau-loader` -- Generate protoconf: `cmd /C "set PATH=..\..\third_party\_submodules\protobuf\.build\_install\bin;%PATH% && buf generate .."` +- Generate protoconf: + - cmd: `cmd /C "set PATH=..\..\third_party\_submodules\protobuf\.build\_install\bin;%PATH% && buf generate .."` + - PowerShell: `$env:PATH = "..\..\third_party\_submodules\protobuf\.build\_install\bin;" + $env:PATH; buf generate ..` - CMake: - - C++17: `cmake -S . -B build -G "Ninja"` - - C++20: `cmake -S . -B build -G "Ninja" -DCMAKE_CXX_STANDARD=20` + - C++17: `cmake -S . -B build -G "Ninja" -DCMAKE_BUILD_TYPE=Release` + - C++20: `cmake -S . -B build -G "Ninja" -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=20` - Build: `cmake --build build --parallel` - Test: `ctest --test-dir build --output-on-failure` @@ -90,7 +94,10 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - Install: **dotnet-sdk-8.0** - Change dir: `cd test/csharp-tableau-loader` -- Generate protoconf: `PATH=../../third_party/_submodules/protobuf/.build/_install/bin:$PATH buf generate ..` +- Generate protoconf: + - macOS / Linux: `PATH=../../third_party/_submodules/protobuf/.build/_install/bin:$PATH buf generate ..` + - Windows (cmd): `cmd /C "set PATH=..\..\third_party\_submodules\protobuf\.build\_install\bin;%PATH% && buf generate .."` + - Windows (PowerShell): `$env:PATH = "..\..\third_party\_submodules\protobuf\.build\_install\bin;" + $env:PATH; buf generate ..` - Test: `dotnet test` > **Note:** Tests are written with [xUnit](https://xunit.net/). diff --git a/init.bat b/init.bat index c2eb5de7..13f0f222 100644 --- a/init.bat +++ b/init.bat @@ -37,7 +37,7 @@ if %MAJOR_VERSION% LEQ 3 ( REM Legacy protobuf (v3.x): CMakeLists.txt is in cmake/ subdirectory echo Using legacy cmake\ subdirectory for protobuf %PROTOBUF_VERSION% cmake -S cmake -B .build -G Ninja ^ - -DCMAKE_BUILD_TYPE=Debug ^ + -DCMAKE_BUILD_TYPE=Release ^ -DCMAKE_CXX_STANDARD=17 ^ -DCMAKE_POLICY_VERSION_MINIMUM=3.5 ^ -Dprotobuf_BUILD_TESTS=OFF ^ @@ -47,13 +47,13 @@ if %MAJOR_VERSION% LEQ 3 ( REM Modern protobuf (v4+/v21+/v32+): CMakeLists.txt is in root directory REM Refer: https://github.com/protocolbuffers/protobuf/blob/v32.0/cmake/README.md#cmake-configuration echo Using root CMakeLists.txt for protobuf %PROTOBUF_VERSION% - REM - protobuf_MSVC_STATIC_RUNTIME defaults to ON, which uses static CRT (/MTd for Debug). + REM - protobuf_MSVC_STATIC_RUNTIME defaults to ON, which uses static CRT (/MT for Release). REM Our project's CMakeLists.txt also sets static CRT to match. REM - protobuf_WITH_ZLIB=OFF: disable ZLIB dependency to avoid ZLIB::ZLIB link requirement REM in protobuf's exported CMake targets, which simplifies cross-platform builds. REM - protobuf_BUILD_SHARED_LIBS=OFF: build static libraries explicitly. cmake -S . -B .build -G Ninja ^ - -DCMAKE_BUILD_TYPE=Debug ^ + -DCMAKE_BUILD_TYPE=Release ^ -DCMAKE_CXX_STANDARD=17 ^ -DCMAKE_POLICY_VERSION_MINIMUM=3.5 ^ -Dprotobuf_BUILD_TESTS=OFF ^ diff --git a/init.sh b/init.sh index d1053899..d7635e87 100755 --- a/init.sh +++ b/init.sh @@ -35,7 +35,7 @@ if [ "${MAJOR_VERSION}" -le 3 ] 2>/dev/null; then # Legacy protobuf (v3.x): CMakeLists.txt is in cmake/ subdirectory echo "Using legacy cmake/ subdirectory for protobuf ${PROTOBUF_VERSION}" cmake -S cmake -B .build -G Ninja \ - -DCMAKE_BUILD_TYPE=Debug \ + -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_CXX_STANDARD=17 \ -Dprotobuf_BUILD_TESTS=OFF \ -Dprotobuf_WITH_ZLIB=OFF \ @@ -48,7 +48,7 @@ else # in protobuf's exported CMake targets, which simplifies cross-platform builds. # - protobuf_BUILD_SHARED_LIBS=OFF: build static libraries explicitly. cmake -S . -B .build -G Ninja \ - -DCMAKE_BUILD_TYPE=Debug \ + -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_CXX_STANDARD=17 \ -Dprotobuf_BUILD_TESTS=OFF \ -Dprotobuf_WITH_ZLIB=OFF \ diff --git a/test/cpp-tableau-loader/CMakeLists.txt b/test/cpp-tableau-loader/CMakeLists.txt index 5ceef019..8e50778e 100644 --- a/test/cpp-tableau-loader/CMakeLists.txt +++ b/test/cpp-tableau-loader/CMakeLists.txt @@ -3,6 +3,16 @@ cmake_minimum_required(VERSION 3.22) # set the project name project(loader) +# Default to Release for single-config generators (e.g. Ninja, Makefiles) when +# the user does not pass -DCMAKE_BUILD_TYPE=... explicitly. This is critical on +# Windows/MSVC because our protobuf submodule is built as Release (/MT), and a +# mismatched CRT (e.g. loader built as Debug /MTd against libprotobuf.lib /MT) +# causes LNK2038 _ITERATOR_DEBUG_LEVEL / RuntimeLibrary mismatch errors. +if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) + set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build (Debug, Release, RelWithDebInfo, MinSizeRel)" FORCE) + message(STATUS "CMAKE_BUILD_TYPE not set; defaulting to Release.") +endif() + # Glob the loader sources (proto-generated .cc and the hub/custom .cpp files # under src/, but NOT files under tests/ — those are owned by the test target). file(GLOB_RECURSE PROTO_SOURCE ${PROJECT_SOURCE_DIR}/src/*.cc) From 040c197c40c6320150a23260c2d17f9d428cda77 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Fri, 15 May 2026 22:26:14 +0800 Subject: [PATCH 05/16] fix(cpp-loader): use msg.c_str() in legacy ProtobufLogHandler ATOM_LOGGER_CALL is printf-style variadic; passing const std::string& directly is UB. GCC warns; Clang errors with -Wnon-pod-varargs. Update both the generator template and the committed generated copy to match the abseil branch's .c_str() form. --- cmd/protoc-gen-cpp-tableau-loader/embed/util.pc.cc | 2 +- test/cpp-tableau-loader/src/protoconf/util.pc.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/protoc-gen-cpp-tableau-loader/embed/util.pc.cc b/cmd/protoc-gen-cpp-tableau-loader/embed/util.pc.cc index f0f1a340..9672a012 100644 --- a/cmd/protoc-gen-cpp-tableau-loader/embed/util.pc.cc +++ b/cmd/protoc-gen-cpp-tableau-loader/embed/util.pc.cc @@ -216,7 +216,7 @@ void ProtobufLogHandler(google::protobuf::LogLevel level, const char* filename, #define TABLEAU_PB_LOG_LEVEL level #define TABLEAU_PB_LOG_FILENAME filename #define TABLEAU_PB_LOG_LINE line -#define TABLEAU_PB_LOG_MESSAGE msg +#define TABLEAU_PB_LOG_MESSAGE msg.c_str() #else // refer: https://github.com/abseil/abseil-cpp/blob/20250512.1/absl/log/log_entry.h void ProtobufAbslLogSink::Send(const absl::LogEntry& entry) { diff --git a/test/cpp-tableau-loader/src/protoconf/util.pc.cc b/test/cpp-tableau-loader/src/protoconf/util.pc.cc index b6306017..39225b77 100644 --- a/test/cpp-tableau-loader/src/protoconf/util.pc.cc +++ b/test/cpp-tableau-loader/src/protoconf/util.pc.cc @@ -222,7 +222,7 @@ void ProtobufLogHandler(google::protobuf::LogLevel level, const char* filename, #define TABLEAU_PB_LOG_LEVEL level #define TABLEAU_PB_LOG_FILENAME filename #define TABLEAU_PB_LOG_LINE line -#define TABLEAU_PB_LOG_MESSAGE msg +#define TABLEAU_PB_LOG_MESSAGE msg.c_str() #else // refer: https://github.com/abseil/abseil-cpp/blob/20250512.1/absl/log/log_entry.h void ProtobufAbslLogSink::Send(const absl::LogEntry& entry) { From c2e40e8960c040dde2074105338f7765886d8a79 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Fri, 15 May 2026 22:35:54 +0800 Subject: [PATCH 06/16] perf(ci): cache prebuilt protobuf to skip ~10min compile on every run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, every CI job (4 in total: ubuntu/windows × protobuf v3.19.3/v32.0) recompiled the protobuf submodule from scratch, which takes 5-15 min depending on OS. Since the protobuf source is pinned by PROTOBUF_REF (= matrix.protobuf-version) and the build flags only change when init.{sh,bat} change, the install output is reusable across runs. Changes: - .github/workflows/testing-cpp.yml: add an actions/cache@v4 step that caches third_party/_submodules/protobuf/.build/_install, keyed by ${os}-${protobuf-version}-hashFiles(init.sh, init.bat, .gitmodules). Restore-keys allow falling back to an older entry when the hash changes (still much faster than a full rebuild). - init.bat / init.sh: add a fast-path early exit when .build/_install/lib[64]/cmake/protobuf/protobuf-config.cmake already exists (i.e. cache restored or local rerun). Honors FORCE_REBUILD_PROTOBUF=1 to bypass the short-circuit when needed. Effect: - Cold cache (first run / key miss): unchanged, ~10 min. - Warm cache (typical PR/push): "Init submodules and build protobuf" drops to <30s (just cache restore + early exit). - Local devs also benefit: rerunning init.{sh,bat} is now idempotent. --- .github/workflows/testing-cpp.yml | 18 ++++++++++++++++++ init.bat | 11 +++++++++++ init.sh | 12 ++++++++++++ 3 files changed, 41 insertions(+) diff --git a/.github/workflows/testing-cpp.yml b/.github/workflows/testing-cpp.yml index b8a40662..e74be48f 100644 --- a/.github/workflows/testing-cpp.yml +++ b/.github/workflows/testing-cpp.yml @@ -53,6 +53,24 @@ jobs: if: runner.os == 'Windows' uses: ilammy/msvc-dev-cmd@v1 + # Cache the prebuilt protobuf install dir. The init scripts have a + # fast path: if .build/_install is present (e.g. restored from cache), + # they skip the (very long) protobuf compile and exit early. + # Key invalidates automatically when: + # - the OS changes (binaries are not portable), + # - the requested protobuf version changes, + # - build scripts change (init.sh / init.bat / .gitmodules), which + # covers BUILD_TYPE flips, flag changes, etc. + - name: Cache protobuf install + id: cache-protobuf + uses: actions/cache@v4 + with: + path: | + third_party/_submodules/protobuf/.build/_install + key: protobuf-${{ matrix.os }}-${{ matrix.protobuf-version }}-${{ hashFiles('init.sh', 'init.bat', '.gitmodules') }} + restore-keys: | + protobuf-${{ matrix.os }}-${{ matrix.protobuf-version }}- + - name: Init submodules and build protobuf run: ${{ matrix.init_script }} env: diff --git a/init.bat b/init.bat index 13f0f222..eccd8bad 100644 --- a/init.bat +++ b/init.bat @@ -24,6 +24,17 @@ if not "%PROTOBUF_REF%"=="" ( git submodule update --init --recursive ) +REM Fast path: if a previous build's _install dir is already present (e.g. +REM restored from CI cache), skip the (very long) protobuf compile entirely. +REM Set FORCE_REBUILD_PROTOBUF=1 to bypass this short-circuit. +if "%FORCE_REBUILD_PROTOBUF%"=="" ( + if exist ".build\_install\lib\cmake\protobuf\protobuf-config.cmake" ( + echo [INFO] Found existing protobuf install at .build\_install, skipping rebuild. + echo [INFO] Set FORCE_REBUILD_PROTOBUF=1 to force a clean rebuild. + goto :eof + ) +) + REM Detect protobuf major version to determine cmake source directory and arguments. for /f "tokens=*" %%v in ('git describe --tags --abbrev^=0 2^>nul') do set PROTOBUF_VERSION=%%v if not defined PROTOBUF_VERSION set PROTOBUF_VERSION=unknown diff --git a/init.sh b/init.sh index d7635e87..56dea961 100755 --- a/init.sh +++ b/init.sh @@ -22,6 +22,18 @@ if [ -n "${PROTOBUF_REF:-}" ]; then git submodule update --init --recursive fi +# Fast path: if a previous build's _install dir is already present (e.g. +# restored from CI cache), skip the (very long) protobuf compile entirely. +# Set FORCE_REBUILD_PROTOBUF=1 to bypass this short-circuit. +if [ -z "${FORCE_REBUILD_PROTOBUF:-}" ]; then + if [ -f ".build/_install/lib/cmake/protobuf/protobuf-config.cmake" ] || \ + [ -f ".build/_install/lib64/cmake/protobuf/protobuf-config.cmake" ]; then + echo "[INFO] Found existing protobuf install at .build/_install, skipping rebuild." + echo "[INFO] Set FORCE_REBUILD_PROTOBUF=1 to force a clean rebuild." + exit 0 + fi +fi + # Detect protobuf major version to determine cmake source directory and arguments. # - protobuf v3.x uses cmake/ subdirectory for CMake builds with minimal options. # - protobuf v4+ (v21+) and latest (v32+) use the root directory with additional options. From 2ac4aced84a0fa81a384f829f295c9b26b8d356b Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Fri, 15 May 2026 22:50:11 +0800 Subject: [PATCH 07/16] ci: validate protobuf cache From bfcfdd79240520ee1d729549b7de6af3bb038fcf Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Fri, 15 May 2026 23:05:32 +0800 Subject: [PATCH 08/16] fix(init): cover protobuf v3.x Windows install layout in fast-path & document it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fast-path added in c2e40e8 only checked .build/_install/lib/cmake/protobuf/protobuf-config.cmake, which is the layout used by: - protobuf v4+ on Windows - protobuf v3.x and v4+ on Linux/macOS (lib or lib64) But protobuf v3.x on Windows installs the cmake package config to a *different* path: .build\_install\cmake\protobuf-config.cmake (no `lib\` prefix, no `protobuf\` subdirectory; see protobuf's own cmake/install.cmake). As a result, local users running init.bat with PROTOBUF_REF=v3.19.3 (the legacy matrix entry) never hit the short-circuit and re-paid the full protobuf compile every time. Changes: - init.bat: add an additional `if exist .build\_install\cmake\ protobuf-config.cmake` check so v3.x Windows installs are recognised. Switched the if/else to label/goto form to dodge the `setlocal`-without-`enabledelayedexpansion` quirk where variables set inside a parenthesised block are not visible to a sibling check on the same parsing pass. - init.sh: keep only the Linux/macOS layouts (lib + lib64); drop the v3.x Windows path that was added for symmetry but is unreachable on POSIX. Trim the inline comment table to match. - README.md: add a "Fast path (idempotent re-runs)" callout under Prerequisites that explains * what the short-circuit does (and why it makes CI cache hits essentially free), * how to force a clean rebuild via FORCE_REBUILD_PROTOBUF=1, with ready-to-copy bash and cmd snippets, * the rmdir/rm -rf .build escape hatch. After this fix: - Local: `cd loader && .\init.bat` against an existing third_party/_submodules/protobuf/.build/_install (v3.19.3, MSVC) prints "[INFO] Found existing protobuf install ..." and exits in a couple of seconds, instead of recompiling protobuf for ~10 min. - CI: the cache restore in .github/workflows/testing-cpp.yml now benefits all four matrix combinations, including windows-latest × protobuf 3.19.3. --- README.md | 12 ++++++++++++ init.bat | 21 ++++++++++++++------- init.sh | 3 +++ 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 41a524a4..df86c738 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,18 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). ``` > **Note:** `prepare.bat` only needs to be run once per machine. It detects already-installed tools and skips them — no manual Visual Studio, CMake, Ninja, or buf installation required. +> **Fast path (idempotent re-runs):** Building protobuf takes 5–15 minutes. To make repeated runs cheap, both `init.sh` and `init.bat` short-circuit and exit immediately when `third_party/_submodules/protobuf/.build/_install` already contains a valid `protobuf-config.cmake` (the marker that the previous build finished). This means: +> - Re-running `init.sh` / `init.bat` after a successful first run is a no-op (a second or two). +> - CI workflows cache `.build/_install` (see `.github/workflows/testing-cpp.yml`) and the fast path then turns the "build protobuf" step into a near-instant cache restore. +> - To force a clean rebuild (e.g. after changing protobuf flags or switching `PROTOBUF_REF` to a version whose previously-installed artefacts are still around), set `FORCE_REBUILD_PROTOBUF=1`: +> ```sh +> FORCE_REBUILD_PROTOBUF=1 bash init.sh # macOS / Linux +> ``` +> ```bat +> set FORCE_REBUILD_PROTOBUF=1 && .\init.bat :: Windows (cmd) +> ``` +> Or simply delete `third_party/_submodules/protobuf/.build/` before rerunning. + ### References - [Chocolatey](https://chocolatey.org/) diff --git a/init.bat b/init.bat index eccd8bad..7adf8aba 100644 --- a/init.bat +++ b/init.bat @@ -27,13 +27,20 @@ if not "%PROTOBUF_REF%"=="" ( REM Fast path: if a previous build's _install dir is already present (e.g. REM restored from CI cache), skip the (very long) protobuf compile entirely. REM Set FORCE_REBUILD_PROTOBUF=1 to bypass this short-circuit. -if "%FORCE_REBUILD_PROTOBUF%"=="" ( - if exist ".build\_install\lib\cmake\protobuf\protobuf-config.cmake" ( - echo [INFO] Found existing protobuf install at .build\_install, skipping rebuild. - echo [INFO] Set FORCE_REBUILD_PROTOBUF=1 to force a clean rebuild. - goto :eof - ) -) +REM On Windows, protobuf-config.cmake lands at: +REM - v3.x : .build\_install\cmake\protobuf-config.cmake +REM - v4+ : .build\_install\lib\cmake\protobuf\protobuf-config.cmake +if not "%FORCE_REBUILD_PROTOBUF%"=="" goto :no_fast_path +if exist ".build\_install\cmake\protobuf-config.cmake" goto :fast_path_hit +if exist ".build\_install\lib\cmake\protobuf\protobuf-config.cmake" goto :fast_path_hit +goto :no_fast_path + +:fast_path_hit +echo [INFO] Found existing protobuf install at .build\_install, skipping rebuild. +echo [INFO] Set FORCE_REBUILD_PROTOBUF=1 to force a clean rebuild. +goto :eof + +:no_fast_path REM Detect protobuf major version to determine cmake source directory and arguments. for /f "tokens=*" %%v in ('git describe --tags --abbrev^=0 2^>nul') do set PROTOBUF_VERSION=%%v diff --git a/init.sh b/init.sh index 56dea961..e16d5c62 100755 --- a/init.sh +++ b/init.sh @@ -25,6 +25,9 @@ fi # Fast path: if a previous build's _install dir is already present (e.g. # restored from CI cache), skip the (very long) protobuf compile entirely. # Set FORCE_REBUILD_PROTOBUF=1 to bypass this short-circuit. +# On Linux/macOS, protobuf-config.cmake lands at: +# .build/_install/lib/cmake/protobuf/protobuf-config.cmake (lib) +# .build/_install/lib64/cmake/protobuf/protobuf-config.cmake (lib64, e.g. RHEL/CentOS) if [ -z "${FORCE_REBUILD_PROTOBUF:-}" ]; then if [ -f ".build/_install/lib/cmake/protobuf/protobuf-config.cmake" ] || \ [ -f ".build/_install/lib64/cmake/protobuf/protobuf-config.cmake" ]; then From 87317eb8aaf47be3f4780e88afea95337f215cb5 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Fri, 15 May 2026 23:14:33 +0800 Subject: [PATCH 09/16] perf(ci): drop redundant 'choco install cmake ninja' on windows-latest windows-latest (Windows Server 2022) preinstalls CMake 3.31.6, Ninja 1.13.2 and the full Visual Studio 2022 + MSVC C++ toolchain. Running `choco install cmake ninja -y` reinstalls those packages from scratch (MSI download + install + registry edits), costing 2-5 min per matrix job for no benefit. Drop the step entirely; ilammy/msvc-dev-cmd@v1 is sufficient to make MSVC available on PATH. Also trim the Ubuntu step: ubuntu-latest already preinstalls cmake; only ninja-build is missing. --- .github/workflows/testing-cpp.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/testing-cpp.yml b/.github/workflows/testing-cpp.yml index e74be48f..56f0670c 100644 --- a/.github/workflows/testing-cpp.yml +++ b/.github/workflows/testing-cpp.yml @@ -43,12 +43,12 @@ jobs: - name: Install dependencies (Ubuntu) if: runner.os == 'Linux' - run: sudo apt-get update && sudo apt-get install -y cmake ninja-build - - - name: Install dependencies (Windows) - if: runner.os == 'Windows' - run: choco install cmake ninja -y + # ubuntu-latest preinstalls cmake but not ninja-build. + run: sudo apt-get update && sudo apt-get install -y ninja-build + # Note: windows-latest (Windows Server 2022) preinstalls CMake, Ninja, + # Visual Studio 2022 + MSVC C++ toolchain, and vswhere. No 'choco install' + # step is needed; the Setup MSVC step below is sufficient. - name: Setup MSVC (Windows) if: runner.os == 'Windows' uses: ilammy/msvc-dev-cmd@v1 From 5740885fee9ee0f5674a64700cc9d2ed618783b4 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Mon, 18 May 2026 12:06:39 +0800 Subject: [PATCH 10/16] build: invalidate protobuf fast-path via build signature Replace the existence-only fast-path check in init.sh / init.bat with a signature-based one that hashes protobuf version, build variant (legacy vs modern) and the full cmake argument list. When the recorded signature in .build/_install/.build_signature does not match the expected one, wipe .build and rebuild from scratch so stale artifacts (e.g. from switching Release/Debug or protobuf major versions) are not silently reused. FORCE_REBUILD_PROTOBUF=1 still bypasses the fast-path unconditionally. --- init.bat | 239 ++++++++++++++++++++++++++++++++++--------------------- init.sh | 108 +++++++++++++++++-------- 2 files changed, 224 insertions(+), 123 deletions(-) diff --git a/init.bat b/init.bat index 7adf8aba..ea1c061e 100644 --- a/init.bat +++ b/init.bat @@ -1,92 +1,147 @@ -@echo off -setlocal - -REM Initialize build environment (installs choco/ninja/MSVC if needed, sets up PATH) -call "%~dp0prepare.bat" -if errorlevel 1 ( - echo [ERROR] prepare.bat failed. Aborting. - exit /b 1 -) - -for /f "delims=" %%i in ('git rev-parse --show-toplevel') do set repoRoot=%%i -cd /d "%repoRoot%" - -git submodule update --init --recursive - -REM Build and install the C++ Protocol Buffer runtime and the Protocol Buffer compiler (protoc) -cd third_party\_submodules\protobuf - -REM If PROTOBUF_REF is set, switch submodule to the specified ref -if not "%PROTOBUF_REF%"=="" ( - echo Switching protobuf submodule to %PROTOBUF_REF%... - git fetch --tags - git checkout %PROTOBUF_REF% - git submodule update --init --recursive -) - -REM Fast path: if a previous build's _install dir is already present (e.g. -REM restored from CI cache), skip the (very long) protobuf compile entirely. -REM Set FORCE_REBUILD_PROTOBUF=1 to bypass this short-circuit. -REM On Windows, protobuf-config.cmake lands at: -REM - v3.x : .build\_install\cmake\protobuf-config.cmake -REM - v4+ : .build\_install\lib\cmake\protobuf\protobuf-config.cmake -if not "%FORCE_REBUILD_PROTOBUF%"=="" goto :no_fast_path -if exist ".build\_install\cmake\protobuf-config.cmake" goto :fast_path_hit -if exist ".build\_install\lib\cmake\protobuf\protobuf-config.cmake" goto :fast_path_hit -goto :no_fast_path - -:fast_path_hit -echo [INFO] Found existing protobuf install at .build\_install, skipping rebuild. -echo [INFO] Set FORCE_REBUILD_PROTOBUF=1 to force a clean rebuild. -goto :eof - -:no_fast_path - -REM Detect protobuf major version to determine cmake source directory and arguments. -for /f "tokens=*" %%v in ('git describe --tags --abbrev^=0 2^>nul') do set PROTOBUF_VERSION=%%v -if not defined PROTOBUF_VERSION set PROTOBUF_VERSION=unknown -echo Detected protobuf version: %PROTOBUF_VERSION% - -REM Extract major version number from tag (e.g., v3.19.3 -> 3, v32.0 -> 32) -set "VER_STR=%PROTOBUF_VERSION:~1%" -for /f "tokens=1 delims=." %%a in ("%VER_STR%") do set MAJOR_VERSION=%%a - -if %MAJOR_VERSION% LEQ 3 ( - REM Legacy protobuf (v3.x): CMakeLists.txt is in cmake/ subdirectory - echo Using legacy cmake\ subdirectory for protobuf %PROTOBUF_VERSION% - cmake -S cmake -B .build -G Ninja ^ - -DCMAKE_BUILD_TYPE=Release ^ - -DCMAKE_CXX_STANDARD=17 ^ - -DCMAKE_POLICY_VERSION_MINIMUM=3.5 ^ - -Dprotobuf_BUILD_TESTS=OFF ^ - -Dprotobuf_WITH_ZLIB=OFF ^ - -Dprotobuf_BUILD_SHARED_LIBS=OFF -) else ( - REM Modern protobuf (v4+/v21+/v32+): CMakeLists.txt is in root directory - REM Refer: https://github.com/protocolbuffers/protobuf/blob/v32.0/cmake/README.md#cmake-configuration - echo Using root CMakeLists.txt for protobuf %PROTOBUF_VERSION% - REM - protobuf_MSVC_STATIC_RUNTIME defaults to ON, which uses static CRT (/MT for Release). - REM Our project's CMakeLists.txt also sets static CRT to match. - REM - protobuf_WITH_ZLIB=OFF: disable ZLIB dependency to avoid ZLIB::ZLIB link requirement - REM in protobuf's exported CMake targets, which simplifies cross-platform builds. - REM - protobuf_BUILD_SHARED_LIBS=OFF: build static libraries explicitly. - cmake -S . -B .build -G Ninja ^ - -DCMAKE_BUILD_TYPE=Release ^ - -DCMAKE_CXX_STANDARD=17 ^ - -DCMAKE_POLICY_VERSION_MINIMUM=3.5 ^ - -Dprotobuf_BUILD_TESTS=OFF ^ - -Dprotobuf_WITH_ZLIB=OFF ^ - -Dprotobuf_BUILD_SHARED_LIBS=OFF ^ - -Dutf8_range_ENABLE_INSTALL=ON -) - -REM Compile the code -cmake --build .build --parallel - -REM Install into .build/_install so that protobuf-config.cmake (along with -REM absl and utf8_range configs) is generated for find_package(Protobuf CONFIG) -REM used by downstream CMakeLists.txt. -REM NOTE: .build/ is already in protobuf's .gitignore, so _install stays clean. -cmake --install .build --prefix .build\_install - -endlocal +@echo off +setlocal enabledelayedexpansion + +REM Initialize build environment (installs choco/ninja/MSVC if needed, sets up PATH) +call "%~dp0prepare.bat" +if errorlevel 1 ( + echo [ERROR] prepare.bat failed. Aborting. + exit /b 1 +) + +for /f "delims=" %%i in ('git rev-parse --show-toplevel') do set repoRoot=%%i +cd /d "%repoRoot%" + +git submodule update --init --recursive + +REM Build and install the C++ Protocol Buffer runtime and the Protocol Buffer compiler (protoc) +cd third_party\_submodules\protobuf + +REM If PROTOBUF_REF is set, switch submodule to the specified ref +if not "%PROTOBUF_REF%"=="" ( + echo Switching protobuf submodule to %PROTOBUF_REF%... + git fetch --tags + git checkout %PROTOBUF_REF% + git submodule update --init --recursive +) + +REM --------------------------------------------------------------------------- +REM Detect protobuf major version and build the cmake command line. We compute +REM both up-front (before the fast-path check) so the signature comparison +REM below can include the exact cmake invocation we would run. +REM - protobuf v3.x : CMakeLists.txt is in cmake/ subdirectory +REM - protobuf v4+ : CMakeLists.txt is in root directory +REM --------------------------------------------------------------------------- +for /f "tokens=*" %%v in ('git describe --tags --abbrev^=0 2^>nul') do set PROTOBUF_VERSION=%%v +if not defined PROTOBUF_VERSION set PROTOBUF_VERSION=unknown +echo Detected protobuf version: %PROTOBUF_VERSION% + +REM Extract major version number from tag (e.g., v3.19.3 -> 3, v32.0 -> 32) +set "VER_STR=%PROTOBUF_VERSION:~1%" +for /f "tokens=1 delims=." %%a in ("%VER_STR%") do set MAJOR_VERSION=%%a + +if %MAJOR_VERSION% LEQ 3 ( + REM Legacy protobuf (v3.x): CMakeLists.txt is in cmake/ subdirectory + set "PROTOBUF_BUILD_VARIANT=legacy" + set "CMAKE_SRC=cmake" + set "CMAKE_FLAGS=-DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=17 -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -Dprotobuf_BUILD_TESTS=OFF -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_SHARED_LIBS=OFF" +) else ( + REM Modern protobuf (v4+/v21+/v32+): CMakeLists.txt is in root directory + REM Refer: https://github.com/protocolbuffers/protobuf/blob/v32.0/cmake/README.md#cmake-configuration + REM - protobuf_MSVC_STATIC_RUNTIME defaults to ON, which uses static CRT (/MT for Release). + REM Our project's CMakeLists.txt also sets static CRT to match. + REM - protobuf_WITH_ZLIB=OFF: disable ZLIB dependency to avoid ZLIB::ZLIB link requirement + REM in protobuf's exported CMake targets, which simplifies cross-platform builds. + REM - protobuf_BUILD_SHARED_LIBS=OFF: build static libraries explicitly. + set "PROTOBUF_BUILD_VARIANT=modern" + set "CMAKE_SRC=." + set "CMAKE_FLAGS=-DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=17 -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -Dprotobuf_BUILD_TESTS=OFF -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_SHARED_LIBS=OFF -Dutf8_range_ENABLE_INSTALL=ON" +) + +REM Build a stable, multi-line signature describing the inputs that determine +REM the contents of .build\_install. Any change to these values must +REM invalidate the fast-path. Adding new compile-time inputs? Append a line. +set "SIG_FILE=.build\_install\.build_signature" +set "SIG_LINE_1=schema=1" +set "SIG_LINE_2=version=!PROTOBUF_VERSION!" +set "SIG_LINE_3=variant=!PROTOBUF_BUILD_VARIANT!" +set "SIG_LINE_4=cmake_args=-S !CMAKE_SRC! -B .build -G Ninja !CMAKE_FLAGS!" + +REM --------------------------------------------------------------------------- +REM Fast path: if a previous build's signature file matches the one we're +REM about to use, skip the (very long) protobuf compile entirely. +REM Set FORCE_REBUILD_PROTOBUF=1 to bypass this short-circuit unconditionally. +REM --------------------------------------------------------------------------- +if not "%FORCE_REBUILD_PROTOBUF%"=="" goto :no_fast_path +if not exist "!SIG_FILE!" goto :no_fast_path + +REM Read the file 4 lines at a time and compare with the expected signature. +set "ACTUAL_LINE_1=" +set "ACTUAL_LINE_2=" +set "ACTUAL_LINE_3=" +set "ACTUAL_LINE_4=" +set "_LINE_NO=0" +for /f "usebackq delims=" %%L in ("!SIG_FILE!") do ( + set /a _LINE_NO+=1 + set "ACTUAL_LINE_!_LINE_NO!=%%L" +) + +if not "!ACTUAL_LINE_1!"=="!SIG_LINE_1!" goto :sig_mismatch +if not "!ACTUAL_LINE_2!"=="!SIG_LINE_2!" goto :sig_mismatch +if not "!ACTUAL_LINE_3!"=="!SIG_LINE_3!" goto :sig_mismatch +if not "!ACTUAL_LINE_4!"=="!SIG_LINE_4!" goto :sig_mismatch + +echo [INFO] Build signature matches; reusing existing protobuf install at .build\_install. +echo [INFO] Set FORCE_REBUILD_PROTOBUF=1 to force a clean rebuild. +goto :eof + +:sig_mismatch +echo [INFO] Build signature mismatch; rebuilding protobuf. +echo [INFO] actual: +echo [INFO] !ACTUAL_LINE_1! +echo [INFO] !ACTUAL_LINE_2! +echo [INFO] !ACTUAL_LINE_3! +echo [INFO] !ACTUAL_LINE_4! +echo [INFO] expected: +echo [INFO] !SIG_LINE_1! +echo [INFO] !SIG_LINE_2! +echo [INFO] !SIG_LINE_3! +echo [INFO] !SIG_LINE_4! + +:no_fast_path +REM Wipe any stale install dir so we don't leave half-overwritten files behind +REM when cmake flags change (e.g. Release -> Debug puts artifacts in different +REM places, an in-place re-install would mix old and new). +if exist .build rmdir /s /q .build + +REM --------------------------------------------------------------------------- +REM Configure +REM --------------------------------------------------------------------------- +if "!PROTOBUF_BUILD_VARIANT!"=="legacy" ( + echo Using legacy cmake\ subdirectory for protobuf %PROTOBUF_VERSION% +) else ( + echo Using root CMakeLists.txt for protobuf %PROTOBUF_VERSION% +) +cmake -S !CMAKE_SRC! -B .build -G Ninja !CMAKE_FLAGS! +if errorlevel 1 exit /b 1 + +REM Compile the code +cmake --build .build --parallel +if errorlevel 1 exit /b 1 + +REM Install into .build/_install so that protobuf-config.cmake (along with +REM absl and utf8_range configs) is generated for find_package(Protobuf CONFIG) +REM used by downstream CMakeLists.txt. +REM NOTE: .build/ is already in protobuf's .gitignore, so _install stays clean. +cmake --install .build --prefix .build\_install +if errorlevel 1 exit /b 1 + +REM Persist the signature so the next run can fast-path skip when nothing changed. +> "!SIG_FILE!" ( + echo !SIG_LINE_1! + echo !SIG_LINE_2! + echo !SIG_LINE_3! + echo !SIG_LINE_4! +) +echo [INFO] Wrote build signature to !SIG_FILE! + +endlocal diff --git a/init.sh b/init.sh index e16d5c62..87fc2d2d 100755 --- a/init.sh +++ b/init.sh @@ -22,24 +22,13 @@ if [ -n "${PROTOBUF_REF:-}" ]; then git submodule update --init --recursive fi -# Fast path: if a previous build's _install dir is already present (e.g. -# restored from CI cache), skip the (very long) protobuf compile entirely. -# Set FORCE_REBUILD_PROTOBUF=1 to bypass this short-circuit. -# On Linux/macOS, protobuf-config.cmake lands at: -# .build/_install/lib/cmake/protobuf/protobuf-config.cmake (lib) -# .build/_install/lib64/cmake/protobuf/protobuf-config.cmake (lib64, e.g. RHEL/CentOS) -if [ -z "${FORCE_REBUILD_PROTOBUF:-}" ]; then - if [ -f ".build/_install/lib/cmake/protobuf/protobuf-config.cmake" ] || \ - [ -f ".build/_install/lib64/cmake/protobuf/protobuf-config.cmake" ]; then - echo "[INFO] Found existing protobuf install at .build/_install, skipping rebuild." - echo "[INFO] Set FORCE_REBUILD_PROTOBUF=1 to force a clean rebuild." - exit 0 - fi -fi - -# Detect protobuf major version to determine cmake source directory and arguments. -# - protobuf v3.x uses cmake/ subdirectory for CMake builds with minimal options. -# - protobuf v4+ (v21+) and latest (v32+) use the root directory with additional options. +# ----------------------------------------------------------------------------- +# Detect protobuf major version and build the cmake command line. We compute +# both up-front (before the fast-path check) so the signature comparison below +# can include the exact cmake invocation we would run. +# - protobuf v3.x : CMakeLists.txt is in cmake/ subdirectory +# - protobuf v4+ : CMakeLists.txt is in root directory +# ----------------------------------------------------------------------------- PROTOBUF_VERSION=$(git describe --tags --abbrev=0 2>/dev/null || echo "unknown") echo "Detected protobuf version: ${PROTOBUF_VERSION}" @@ -48,29 +37,82 @@ MAJOR_VERSION=$(echo "${PROTOBUF_VERSION}" | sed 's/^v//' | cut -d. -f1) if [ "${MAJOR_VERSION}" -le 3 ] 2>/dev/null; then # Legacy protobuf (v3.x): CMakeLists.txt is in cmake/ subdirectory - echo "Using legacy cmake/ subdirectory for protobuf ${PROTOBUF_VERSION}" - cmake -S cmake -B .build -G Ninja \ - -DCMAKE_BUILD_TYPE=Release \ - -DCMAKE_CXX_STANDARD=17 \ - -Dprotobuf_BUILD_TESTS=OFF \ - -Dprotobuf_WITH_ZLIB=OFF \ + PROTOBUF_BUILD_VARIANT="legacy" + CMAKE_ARGS=( + -S cmake + -B .build + -G Ninja + -DCMAKE_BUILD_TYPE=Release + -DCMAKE_CXX_STANDARD=17 + -Dprotobuf_BUILD_TESTS=OFF + -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_SHARED_LIBS=OFF + ) else # Modern protobuf (v4+/v21+/v32+): CMakeLists.txt is in root directory # Refer: https://github.com/protocolbuffers/protobuf/blob/v32.0/cmake/README.md#cmake-configuration - echo "Using root CMakeLists.txt for protobuf ${PROTOBUF_VERSION}" # - protobuf_WITH_ZLIB=OFF: disable ZLIB dependency to avoid ZLIB::ZLIB link requirement # in protobuf's exported CMake targets, which simplifies cross-platform builds. # - protobuf_BUILD_SHARED_LIBS=OFF: build static libraries explicitly. - cmake -S . -B .build -G Ninja \ - -DCMAKE_BUILD_TYPE=Release \ - -DCMAKE_CXX_STANDARD=17 \ - -Dprotobuf_BUILD_TESTS=OFF \ - -Dprotobuf_WITH_ZLIB=OFF \ - -Dprotobuf_BUILD_SHARED_LIBS=OFF \ + PROTOBUF_BUILD_VARIANT="modern" + CMAKE_ARGS=( + -S . + -B .build + -G Ninja + -DCMAKE_BUILD_TYPE=Release + -DCMAKE_CXX_STANDARD=17 + -Dprotobuf_BUILD_TESTS=OFF + -Dprotobuf_WITH_ZLIB=OFF + -Dprotobuf_BUILD_SHARED_LIBS=OFF -Dutf8_range_ENABLE_INSTALL=ON + ) fi +# Build a stable, multi-line signature describing the inputs that determine +# the contents of .build/_install. Any change to these values must invalidate +# the fast-path. Adding new compile-time inputs? Append a line here. +SIG_FILE=".build/_install/.build_signature" +EXPECTED_SIGNATURE=$(printf '%s\n' \ + "schema=1" \ + "version=${PROTOBUF_VERSION}" \ + "variant=${PROTOBUF_BUILD_VARIANT}" \ + "cmake_args=${CMAKE_ARGS[*]}") + +# ----------------------------------------------------------------------------- +# Fast path: if a previous build's _install dir is present AND its embedded +# signature matches the one we're about to use, skip the (very long) protobuf +# compile entirely. +# Set FORCE_REBUILD_PROTOBUF=1 to bypass this short-circuit unconditionally. +# ----------------------------------------------------------------------------- +if [ -z "${FORCE_REBUILD_PROTOBUF:-}" ] && [ -f "${SIG_FILE}" ]; then + ACTUAL_SIGNATURE=$(cat "${SIG_FILE}") + if [ "${ACTUAL_SIGNATURE}" = "${EXPECTED_SIGNATURE}" ]; then + echo "[INFO] Build signature matches; reusing existing protobuf install at .build/_install." + echo "[INFO] Set FORCE_REBUILD_PROTOBUF=1 to force a clean rebuild." + exit 0 + fi + echo "[INFO] Build signature mismatch; rebuilding protobuf." + echo "[INFO] actual:" + printf '%s\n' "${ACTUAL_SIGNATURE}" | sed 's/^/[INFO] /' + echo "[INFO] expected:" + printf '%s\n' "${EXPECTED_SIGNATURE}" | sed 's/^/[INFO] /' +fi + +# Wipe any stale install dir so we don't leave half-overwritten files behind +# when cmake flags change (e.g. Release -> Debug puts artifacts in different +# places, an in-place re-install would mix old and new). +rm -rf .build 2>/dev/null || true + +# ----------------------------------------------------------------------------- +# Configure +# ----------------------------------------------------------------------------- +if [ "${PROTOBUF_BUILD_VARIANT}" = "legacy" ]; then + echo "Using legacy cmake/ subdirectory for protobuf ${PROTOBUF_VERSION}" +else + echo "Using root CMakeLists.txt for protobuf ${PROTOBUF_VERSION}" +fi +cmake "${CMAKE_ARGS[@]}" + # Compile the code cmake --build .build --parallel @@ -79,3 +121,7 @@ cmake --build .build --parallel # used by downstream CMakeLists.txt. # NOTE: .build/ is already in protobuf's .gitignore, so _install stays clean. cmake --install .build --prefix .build/_install + +# Persist the signature so the next run can fast-path skip when nothing changed. +printf '%s\n' "${EXPECTED_SIGNATURE}" > "${SIG_FILE}" +echo "[INFO] Wrote build signature to ${SIG_FILE}" From 1c3622270cd3848e8959976645e001f6fcda7c05 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Mon, 18 May 2026 14:19:35 +0800 Subject: [PATCH 11/16] build: stop recursively initializing protobuf submodule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only top-level submodule is protobuf itself, and its nested submodules (third_party/googletest, third_party/benchmark on v3.x) are gated behind protobuf_BUILD_TESTS / benchmarks, both of which we keep disabled. Modern protobuf (v4+/v21+) no longer uses git submodules at all — abseil/utf8_range/etc. are pulled in via CMake FetchContent at configure time — so `--recursive` is either wasted clone work (v3.x) or a complete no-op (v4+). - init.sh / init.bat: replace `git submodule update --init --recursive` with a targeted `--init third_party/_submodules/protobuf`, and drop the second recursive update after `PROTOBUF_REF` checkout. - testing-cpp.yml: switch `submodules: recursive` to `submodules: true` in the checkout step. Saves clone time and CI bandwidth; can be reverted (or done explicitly for googletest/benchmark) if we ever turn protobuf tests on. --- .github/workflows/testing-cpp.yml | 6 +++++- init.bat | 8 ++++++-- init.sh | 8 ++++++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.github/workflows/testing-cpp.yml b/.github/workflows/testing-cpp.yml index 56f0670c..6b957b58 100644 --- a/.github/workflows/testing-cpp.yml +++ b/.github/workflows/testing-cpp.yml @@ -32,7 +32,11 @@ jobs: - name: Checkout Code uses: actions/checkout@v6 with: - submodules: recursive + # Non-recursive: only the top-level protobuf submodule is needed. + # Its nested submodules (googletest/benchmark on v3.x) are gated + # behind protobuf_BUILD_TESTS, which we keep OFF; modern protobuf + # (v4+/v21+) has no git submodules at all. + submodules: true - name: Install Go uses: actions/setup-go@v6 diff --git a/init.bat b/init.bat index ea1c061e..ae44c0e2 100644 --- a/init.bat +++ b/init.bat @@ -11,7 +11,12 @@ if errorlevel 1 ( for /f "delims=" %%i in ('git rev-parse --show-toplevel') do set repoRoot=%%i cd /d "%repoRoot%" -git submodule update --init --recursive +REM Initialize only the protobuf submodule (non-recursive). Protobuf's own +REM nested submodules (third_party/googletest, third_party/benchmark on v3.x) +REM are only needed when protobuf_BUILD_TESTS=ON / benchmarks are enabled, and +REM modern protobuf (v4+/v21+) has dropped git submodules entirely in favor of +REM CMake FetchContent. Skipping --recursive saves clone time and CI bandwidth. +git submodule update --init third_party/_submodules/protobuf REM Build and install the C++ Protocol Buffer runtime and the Protocol Buffer compiler (protoc) cd third_party\_submodules\protobuf @@ -21,7 +26,6 @@ if not "%PROTOBUF_REF%"=="" ( echo Switching protobuf submodule to %PROTOBUF_REF%... git fetch --tags git checkout %PROTOBUF_REF% - git submodule update --init --recursive ) REM --------------------------------------------------------------------------- diff --git a/init.sh b/init.sh index 87fc2d2d..50749979 100755 --- a/init.sh +++ b/init.sh @@ -5,7 +5,12 @@ set -o pipefail cd "$(git rev-parse --show-toplevel)" -git submodule update --init --recursive +# Initialize only the protobuf submodule (non-recursive). Protobuf's own nested +# submodules (third_party/googletest, third_party/benchmark on v3.x) are only +# needed when protobuf_BUILD_TESTS=ON / benchmarks are enabled, and modern +# protobuf (v4+/v21+) has dropped git submodules entirely in favor of CMake +# FetchContent. Skipping --recursive saves clone time and CI bandwidth. +git submodule update --init third_party/_submodules/protobuf # prerequisites # On Ubuntu/Debian, you can install them with: @@ -19,7 +24,6 @@ if [ -n "${PROTOBUF_REF:-}" ]; then echo "Switching protobuf submodule to ${PROTOBUF_REF}..." git fetch --tags git checkout "${PROTOBUF_REF}" - git submodule update --init --recursive fi # ----------------------------------------------------------------------------- From 079d71d5817e6b25880a4efe757bf48480c4c437 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Mon, 18 May 2026 15:26:01 +0800 Subject: [PATCH 12/16] docs,ci: rely on CMakeLists defaults and refresh stale comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small, related cleanups around the C++ build instructions and the local/CI tooling notes: * README.md - Bump the documented Go toolchain from 1.21 to 1.24 to match `go.mod` (`go 1.24.0`). - Drop `-DCMAKE_BUILD_TYPE=Release` from both the Linux and Windows CMake quick-start commands. The project's `CMakeLists.txt` already forces `Release` for single-config generators when the user does not pass one, so the explicit flag was redundant; the section headings now spell that out. - Rewrite the `prepare.bat` "run once per machine" note: only the *installation* part is one-shot. The MSVC compiler environment (`cl.exe` on PATH plus INCLUDE/LIB/LIBPATH/WindowsSdkDir/ VCToolsInstallDir) is exported via `endlocal & set ...` to the current cmd session only, so `prepare.bat` must be re-run in every new cmd window before invoking `init.bat` or building. * .github/workflows/testing-cpp.yml - CMake Configure step: drop `-DCMAKE_BUILD_TYPE=Release` and `-DCMAKE_CXX_STANDARD=17`. Both are now sourced from the project defaults (`CMAKE_BUILD_TYPE` -> Release fallback; `MIN_CXX_STANDARD = 17` fallback with `CXX_STANDARD_REQUIRED`), keeping a single source of truth. * prepare.bat - Refresh the "equivalent to CI step: ..." comments that no longer match reality: * Step 1 (Ninja) / Step 2 (CMake): CI no longer `choco install`s these — `windows-latest` preinstalls them; the script remains useful for local machines. * Step 3 (MSVC): clarify that vcvarsall.bat activation is the local equivalent of `ilammy/msvc-dev-cmd@v1`. * Step 4 (buf): stop hard-coding the version (1.67.0) inside the comment; reference the `BUF_VERSION` variable defined below to avoid drift. No behavior change: CI still builds Release / C++17, and local `prepare.bat` / `init.bat` still install and build the same toolchain. --- .github/workflows/testing-cpp.yml | 19 +------------------ README.md | 14 ++++++++------ prepare.bat | 9 +++++---- 3 files changed, 14 insertions(+), 28 deletions(-) diff --git a/.github/workflows/testing-cpp.yml b/.github/workflows/testing-cpp.yml index 6b957b58..7aef6fca 100644 --- a/.github/workflows/testing-cpp.yml +++ b/.github/workflows/testing-cpp.yml @@ -32,10 +32,6 @@ jobs: - name: Checkout Code uses: actions/checkout@v6 with: - # Non-recursive: only the top-level protobuf submodule is needed. - # Its nested submodules (googletest/benchmark on v3.x) are gated - # behind protobuf_BUILD_TESTS, which we keep OFF; modern protobuf - # (v4+/v21+) has no git submodules at all. submodules: true - name: Install Go @@ -47,24 +43,12 @@ jobs: - name: Install dependencies (Ubuntu) if: runner.os == 'Linux' - # ubuntu-latest preinstalls cmake but not ninja-build. run: sudo apt-get update && sudo apt-get install -y ninja-build - # Note: windows-latest (Windows Server 2022) preinstalls CMake, Ninja, - # Visual Studio 2022 + MSVC C++ toolchain, and vswhere. No 'choco install' - # step is needed; the Setup MSVC step below is sufficient. - name: Setup MSVC (Windows) if: runner.os == 'Windows' uses: ilammy/msvc-dev-cmd@v1 - # Cache the prebuilt protobuf install dir. The init scripts have a - # fast path: if .build/_install is present (e.g. restored from cache), - # they skip the (very long) protobuf compile and exit early. - # Key invalidates automatically when: - # - the OS changes (binaries are not portable), - # - the requested protobuf version changes, - # - build scripts change (init.sh / init.bat / .gitmodules), which - # covers BUILD_TYPE flips, flag changes, etc. - name: Cache protobuf install id: cache-protobuf uses: actions/cache@v4 @@ -107,8 +91,7 @@ jobs: - name: CMake Configure working-directory: test/cpp-tableau-loader - run: cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release - -DCMAKE_CXX_STANDARD=17 + run: cmake -S . -B build -G Ninja - name: CMake Build working-directory: test/cpp-tableau-loader diff --git a/README.md b/README.md index df86c738..e978cd5f 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,9 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). ```bat .\init.bat ``` - > **Note:** `prepare.bat` only needs to be run once per machine. It detects already-installed tools and skips them — no manual Visual Studio, CMake, Ninja, or buf installation required. + > **Note:** The **installation** part of `prepare.bat` only runs once per machine — it detects already-installed tools (Chocolatey, Ninja, CMake, MSVC Build Tools, buf) and skips them, so no manual installation is required. + > + > However, the MSVC compiler environment (`cl.exe` on `PATH`, plus `INCLUDE` / `LIB` / `LIBPATH` / `WindowsSdkDir` / `VCToolsInstallDir`) is exported to the **current cmd session only** — `vcvarsall.bat` does not (and should not) write these into the persistent user `PATH`. You therefore need to re-run `.\prepare.bat` in **every new cmd window** before invoking `init.bat` or building the loader. Subsequent runs are near-instant since no installation work is repeated. > **Fast path (idempotent re-runs):** Building protobuf takes 5–15 minutes. To make repeated runs cheap, both `init.sh` and `init.bat` short-circuit and exit immediately when `third_party/_submodules/protobuf/.build/_install` already contains a valid `protobuf-config.cmake` (the marker that the previous build finished). This means: > - Re-running `init.sh` / `init.bat` after a successful first run is a no-op (a second or two). @@ -53,7 +55,7 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - Change dir: `cd test/cpp-tableau-loader` - Generate protoconf: `PATH=../../third_party/_submodules/protobuf/.build/_install/bin:$PATH buf generate ..` -- CMake: +- CMake (the project's `CMakeLists.txt` defaults `CMAKE_BUILD_TYPE` to `Release` for single-config generators when unset, so `-DCMAKE_BUILD_TYPE=...` is omitted below): - C++17: `cmake -S . -B build` - C++20: `cmake -S . -B build -DCMAKE_CXX_STANDARD=20` - clang: `cmake -S . -B build -DCMAKE_CXX_COMPILER=clang++` @@ -71,9 +73,9 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - Generate protoconf: - cmd: `cmd /C "set PATH=..\..\third_party\_submodules\protobuf\.build\_install\bin;%PATH% && buf generate .."` - PowerShell: `$env:PATH = "..\..\third_party\_submodules\protobuf\.build\_install\bin;" + $env:PATH; buf generate ..` -- CMake: - - C++17: `cmake -S . -B build -G "Ninja" -DCMAKE_BUILD_TYPE=Release` - - C++20: `cmake -S . -B build -G "Ninja" -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=20` +- CMake (Ninja is single-config; the project's `CMakeLists.txt` defaults `CMAKE_BUILD_TYPE` to `Release` when unset, so `-DCMAKE_BUILD_TYPE=...` is omitted below): + - C++17: `cmake -S . -B build -G "Ninja"` + - C++20: `cmake -S . -B build -G "Ninja" -DCMAKE_CXX_STANDARD=20` - Build: `cmake --build build --parallel` - Test: `ctest --test-dir build --output-on-failure` @@ -86,7 +88,7 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). ## Go -- Install: **go1.21** or above +- Install: **go1.24** or above - Change dir: `cd test/go-tableau-loader` - Generate protoconf: `buf generate ..` - Test: `go test ./...` diff --git a/prepare.bat b/prepare.bat index 94484bec..8f3f0231 100644 --- a/prepare.bat +++ b/prepare.bat @@ -77,7 +77,6 @@ if "%SIMULATE_CLEAN%"=="0" ( REM ----------------------------------------------------------------------- REM Step 1: Ensure Ninja is installed via Chocolatey -REM (equivalent to CI step: choco install ninja -y) REM ----------------------------------------------------------------------- set "NINJA_FOUND=0" if "%SIMULATE_CLEAN%"=="0" ( @@ -171,8 +170,9 @@ if "%CMAKE_FOUND%"=="0" ( ) REM ----------------------------------------------------------------------- -REM Step 3: Ensure MSVC compiler (cl.exe) is available -REM (equivalent to CI step: ilammy/msvc-dev-cmd@v1) +REM Step 3: Ensure MSVC compiler (cl.exe) is available, then activate its +REM environment for this cmd session via vcvarsall.bat. The CI +REM workflow uses ilammy/msvc-dev-cmd@v1 to do the same thing. REM ----------------------------------------------------------------------- set "CL_FOUND=0" if "%SIMULATE_CLEAN%"=="0" ( @@ -241,7 +241,8 @@ if "%CL_FOUND%"=="0" ( REM ----------------------------------------------------------------------- REM Step 4: Ensure buf CLI is installed -REM (equivalent to CI step: bufbuild/buf-action@v1, version 1.67.0) +REM The CI workflow uses bufbuild/buf-action@v1 (also pinned to +REM BUF_VERSION below) to do the same thing. REM buf is a single self-contained .exe; install it under REM %LOCALAPPDATA%\buf\bin\buf.exe to avoid requiring admin rights. REM ----------------------------------------------------------------------- From 933740a877dee11f0fb077ac1df974e34e22e4c6 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Mon, 18 May 2026 15:46:11 +0800 Subject: [PATCH 13/16] build(cpp): switch protobuf and loader to Debug, drop default CMAKE_BUILD_TYPE Align both protobuf and the cpp-tableau-loader on a single, explicit build type so the CRT settings always match. Previously the loader relied on a Release fallback inside CMakeLists.txt while protobuf was also built as Release; this implicit coupling was easy to break (e.g. when CMake fell back to a multi-config generator on Windows whose default config is Debug, producing /MTd vs /MT LNK2038 mismatches). Changes: - init.sh / init.bat: build protobuf with -DCMAKE_BUILD_TYPE=Debug for both legacy (v3.x) and modern (v4+/v21+/v32+) variants. Updated the inline comments accordingly (/MTd for Debug). The protobuf cache key in testing-cpp.yml hashes init.sh / init.bat, so the change naturally invalidates the cached Release artefacts. - test/cpp-tableau-loader/CMakeLists.txt: remove the "default to Release for single-config generators" block. The build type is now never injected by the project; callers must pass -DCMAKE_BUILD_TYPE=... explicitly. CMAKE_MSVC_RUNTIME_LIBRARY remains generator-expression based and picks /MTd automatically when Debug. - .github/workflows/testing-cpp.yml: pass -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=17 explicitly and rewrite the comment to reflect that CMakeLists.txt no longer has a fallback. - README.md: update Linux and Windows sections to use -DCMAKE_BUILD_TYPE=Debug in every cmake invocation, and rewrite the Windows "Build type" note to describe the new Debug-on-both-sides contract. --- .github/workflows/testing-cpp.yml | 2 +- README.md | 16 ++++++++-------- init.bat | 8 ++++---- init.sh | 6 +++--- test/cpp-tableau-loader/CMakeLists.txt | 10 ---------- 5 files changed, 16 insertions(+), 26 deletions(-) diff --git a/.github/workflows/testing-cpp.yml b/.github/workflows/testing-cpp.yml index 7aef6fca..80151d9b 100644 --- a/.github/workflows/testing-cpp.yml +++ b/.github/workflows/testing-cpp.yml @@ -91,7 +91,7 @@ jobs: - name: CMake Configure working-directory: test/cpp-tableau-loader - run: cmake -S . -B build -G Ninja + run: cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=17 - name: CMake Build working-directory: test/cpp-tableau-loader diff --git a/README.md b/README.md index e978cd5f..c6b88d43 100644 --- a/README.md +++ b/README.md @@ -55,10 +55,10 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). - Change dir: `cd test/cpp-tableau-loader` - Generate protoconf: `PATH=../../third_party/_submodules/protobuf/.build/_install/bin:$PATH buf generate ..` -- CMake (the project's `CMakeLists.txt` defaults `CMAKE_BUILD_TYPE` to `Release` for single-config generators when unset, so `-DCMAKE_BUILD_TYPE=...` is omitted below): - - C++17: `cmake -S . -B build` - - C++20: `cmake -S . -B build -DCMAKE_CXX_STANDARD=20` - - clang: `cmake -S . -B build -DCMAKE_CXX_COMPILER=clang++` +- CMake: + - C++17: `cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug` + - C++20: `cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=20` + - clang: `cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_COMPILER=clang++` - Build: `cmake --build build --parallel` - Test: `ctest --test-dir build --output-on-failure` @@ -66,16 +66,16 @@ The official config loader for [Tableau](https://github.com/tableauio/tableau). > **Important:** CMake with Ninja requires MSVC environment variables (`cl.exe`, `INCLUDE`, `LIB`, etc.) to be active. Run `.\prepare.bat` from the **loader** root in the **same cmd session** (use **cmd**, not PowerShell — `prepare.bat` exports vars via `endlocal & set ...` which only works for a cmd parent process) before switching to the test directory. Opening a new terminal window will lose these variables. > -> **Build type:** The protobuf submodule is built as **Release** (`/MT`) by `init.bat`. To avoid LNK2038 `_ITERATOR_DEBUG_LEVEL` / `RuntimeLibrary` CRT-mismatch errors, the loader must also be built as Release. The `CMakeLists.txt` defaults `CMAKE_BUILD_TYPE` to `Release` when it is unset, so the commands below work out of the box. +> **Build type:** The protobuf submodule is built as **Debug** (`/MTd`) by `init.bat`. To avoid LNK2038 `_ITERATOR_DEBUG_LEVEL` / `RuntimeLibrary` CRT-mismatch errors, the loader must also be built as Debug. `CMakeLists.txt` does not set a default, so always pass `-DCMAKE_BUILD_TYPE=Debug` explicitly — also required for multi-config generators (Visual Studio default = Debug, but stay explicit to match the cached protobuf). - Initialize MSVC environment (from loader root): `.\prepare.bat` - Change dir: `cd test\cpp-tableau-loader`, or change directory with Drive, e.g.: `cd /D D:\GitHub\loader\test\cpp-tableau-loader` - Generate protoconf: - cmd: `cmd /C "set PATH=..\..\third_party\_submodules\protobuf\.build\_install\bin;%PATH% && buf generate .."` - PowerShell: `$env:PATH = "..\..\third_party\_submodules\protobuf\.build\_install\bin;" + $env:PATH; buf generate ..` -- CMake (Ninja is single-config; the project's `CMakeLists.txt` defaults `CMAKE_BUILD_TYPE` to `Release` when unset, so `-DCMAKE_BUILD_TYPE=...` is omitted below): - - C++17: `cmake -S . -B build -G "Ninja"` - - C++20: `cmake -S . -B build -G "Ninja" -DCMAKE_CXX_STANDARD=20` +- CMake: + - C++17: `cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug` + - C++20: `cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=20` - Build: `cmake --build build --parallel` - Test: `ctest --test-dir build --output-on-failure` diff --git a/init.bat b/init.bat index ae44c0e2..6d43624f 100644 --- a/init.bat +++ b/init.bat @@ -47,18 +47,18 @@ if %MAJOR_VERSION% LEQ 3 ( REM Legacy protobuf (v3.x): CMakeLists.txt is in cmake/ subdirectory set "PROTOBUF_BUILD_VARIANT=legacy" set "CMAKE_SRC=cmake" - set "CMAKE_FLAGS=-DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=17 -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -Dprotobuf_BUILD_TESTS=OFF -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_SHARED_LIBS=OFF" + set "CMAKE_FLAGS=-DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=17 -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -Dprotobuf_BUILD_TESTS=OFF -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_SHARED_LIBS=OFF" ) else ( REM Modern protobuf (v4+/v21+/v32+): CMakeLists.txt is in root directory REM Refer: https://github.com/protocolbuffers/protobuf/blob/v32.0/cmake/README.md#cmake-configuration - REM - protobuf_MSVC_STATIC_RUNTIME defaults to ON, which uses static CRT (/MT for Release). + REM - protobuf_MSVC_STATIC_RUNTIME defaults to ON, which uses static CRT (/MTd for Debug). REM Our project's CMakeLists.txt also sets static CRT to match. REM - protobuf_WITH_ZLIB=OFF: disable ZLIB dependency to avoid ZLIB::ZLIB link requirement REM in protobuf's exported CMake targets, which simplifies cross-platform builds. REM - protobuf_BUILD_SHARED_LIBS=OFF: build static libraries explicitly. set "PROTOBUF_BUILD_VARIANT=modern" set "CMAKE_SRC=." - set "CMAKE_FLAGS=-DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=17 -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -Dprotobuf_BUILD_TESTS=OFF -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_SHARED_LIBS=OFF -Dutf8_range_ENABLE_INSTALL=ON" + set "CMAKE_FLAGS=-DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=17 -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -Dprotobuf_BUILD_TESTS=OFF -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_SHARED_LIBS=OFF -Dutf8_range_ENABLE_INSTALL=ON" ) REM Build a stable, multi-line signature describing the inputs that determine @@ -113,7 +113,7 @@ echo [INFO] !SIG_LINE_4! :no_fast_path REM Wipe any stale install dir so we don't leave half-overwritten files behind -REM when cmake flags change (e.g. Release -> Debug puts artifacts in different +REM when cmake flags change (e.g. Debug -> Release puts artifacts in different REM places, an in-place re-install would mix old and new). if exist .build rmdir /s /q .build diff --git a/init.sh b/init.sh index 50749979..b70551bf 100755 --- a/init.sh +++ b/init.sh @@ -46,7 +46,7 @@ if [ "${MAJOR_VERSION}" -le 3 ] 2>/dev/null; then -S cmake -B .build -G Ninja - -DCMAKE_BUILD_TYPE=Release + -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=17 -Dprotobuf_BUILD_TESTS=OFF -Dprotobuf_WITH_ZLIB=OFF @@ -63,7 +63,7 @@ else -S . -B .build -G Ninja - -DCMAKE_BUILD_TYPE=Release + -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=17 -Dprotobuf_BUILD_TESTS=OFF -Dprotobuf_WITH_ZLIB=OFF @@ -103,7 +103,7 @@ if [ -z "${FORCE_REBUILD_PROTOBUF:-}" ] && [ -f "${SIG_FILE}" ]; then fi # Wipe any stale install dir so we don't leave half-overwritten files behind -# when cmake flags change (e.g. Release -> Debug puts artifacts in different +# when cmake flags change (e.g. Debug -> Release puts artifacts in different # places, an in-place re-install would mix old and new). rm -rf .build 2>/dev/null || true diff --git a/test/cpp-tableau-loader/CMakeLists.txt b/test/cpp-tableau-loader/CMakeLists.txt index 8e50778e..5ceef019 100644 --- a/test/cpp-tableau-loader/CMakeLists.txt +++ b/test/cpp-tableau-loader/CMakeLists.txt @@ -3,16 +3,6 @@ cmake_minimum_required(VERSION 3.22) # set the project name project(loader) -# Default to Release for single-config generators (e.g. Ninja, Makefiles) when -# the user does not pass -DCMAKE_BUILD_TYPE=... explicitly. This is critical on -# Windows/MSVC because our protobuf submodule is built as Release (/MT), and a -# mismatched CRT (e.g. loader built as Debug /MTd against libprotobuf.lib /MT) -# causes LNK2038 _ITERATOR_DEBUG_LEVEL / RuntimeLibrary mismatch errors. -if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) - set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build (Debug, Release, RelWithDebInfo, MinSizeRel)" FORCE) - message(STATUS "CMAKE_BUILD_TYPE not set; defaulting to Release.") -endif() - # Glob the loader sources (proto-generated .cc and the hub/custom .cpp files # under src/, but NOT files under tests/ — those are owned by the test target). file(GLOB_RECURSE PROTO_SOURCE ${PROJECT_SOURCE_DIR}/src/*.cc) From f34becd164dba2a8547bc27fbc07ae3e2053e021 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Mon, 18 May 2026 15:52:29 +0800 Subject: [PATCH 14/16] fix(cpp): align googletest CRT with loader/protobuf to fix Windows AV Set gtest_force_shared_crt=OFF in test/cpp-tableau-loader/CMakeLists.txt so googletest follows our CMAKE_MSVC_RUNTIME_LIBRARY setting (static CRT, /MT or /MTd) instead of forcing the dynamic CRT (/MD or /MDd). Symptom: on Windows Debug builds, `loader.exe` printed all discovered gtest test names, then crashed with "Result: Access violation" inside gtest_discover_tests, failing the build at link/discovery time. Root cause: protobuf (protobuf_MSVC_STATIC_RUNTIME=ON) and loader_lib (CMAKE_MSVC_RUNTIME_LIBRARY = MultiThreaded[Debug]) were both linked against the static CRT, while googletest with gtest_force_shared_crt=ON was linked against the dynamic CRT. The link succeeded (the linker no longer rejects MultiThreadedDebug vs MultiThreadedDebugDLL the way it rejects _ITERATOR_DEBUG_LEVEL mismatches), but loader.exe ended up with two coexisting CRT instances, each owning its own heap / errno / iostreams globals. At process exit, global-destructor sequencing crossed CRT boundaries (e.g. memory allocated by one CRT freed by the other) and tripped a Debug-heap guard, producing the access violation. Why it only manifests on Windows: MSVC treats the C/C++ runtime as a per-module attribute (/MT[d] vs /MD[d]), so a single process can genuinely host multiple CRT instances. On Linux, glibc and libstdc++ are deduplicated process-wide by the dynamic loader (or merged at static link time), so gtest_force_shared_crt is a no-op there and mixed CRTs simply cannot occur. Note: gtest_force_shared_crt only affects MSVC; this change is inert on Linux/macOS builds. --- test/cpp-tableau-loader/CMakeLists.txt | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/cpp-tableau-loader/CMakeLists.txt b/test/cpp-tableau-loader/CMakeLists.txt index 5ceef019..67f04cc5 100644 --- a/test/cpp-tableau-loader/CMakeLists.txt +++ b/test/cpp-tableau-loader/CMakeLists.txt @@ -53,8 +53,19 @@ message(STATUS "Using protobuf ${Protobuf_VERSION}") # GoogleTest via FetchContent, pinned to a stable release. Using FetchContent # (instead of add_subdirectory on protobuf's bundled googletest) gives a # consistent test framework regardless of which protobuf version is in use. +# +# gtest_force_shared_crt: +# OFF -> let CMAKE_MSVC_RUNTIME_LIBRARY decide (we set it to MultiThreaded[Debug] +# above, i.e. static CRT /MT or /MTd). +# ON -> force googletest to use the DYNAMIC CRT (/MD or /MDd). +# Our protobuf submodule is built with protobuf_MSVC_STATIC_RUNTIME=ON (static +# CRT), and our loader_lib also targets static CRT. Forcing gtest to a different +# CRT would mix two C runtimes inside loader.exe; the link may still succeed but +# global-destructor sequencing at process exit can hit an Access Violation +# (gtest's STL objects holding handles allocated by a different heap). Keep gtest +# on the same static CRT as everything else. include(FetchContent) -set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) +set(gtest_force_shared_crt OFF CACHE BOOL "" FORCE) FetchContent_Declare( googletest GIT_REPOSITORY https://github.com/google/googletest.git From 17bf1948a7ff5414c17186673fee4a88f46cc1ff Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Mon, 18 May 2026 20:14:54 +0800 Subject: [PATCH 15/16] fix(cpp): avoid namespace-scope thread_local std::string in util.pc The generated util.pc.cc declared `static thread_local std::string g_err_msg` at namespace scope. On MSVC /MTd this object's TLS dynamic destructor races with __acrt_uninitialize and crashes the process inside __acrt_lock during exit. The crash was previously masked by a stray `/DNDEBUG` in the test project's CMakeLists.txt (which lowered _ITERATOR_DEBUG_LEVEL to 0 and made short-string destruction skip the debug heap), at the cost of a cross-TU STL ABI mismatch with protobuf/gtest. Replace the namespace-scope thread_local with a Meyers-singleton accessor so the destructor is sequenced through the standard per-thread atexit mechanism, and remove the bogus /DNDEBUG override so all TUs share the same _ITERATOR_DEBUG_LEVEL. --- .../embed/util.pc.cc | 15 +++++++++++--- test/cpp-tableau-loader/CMakeLists.txt | 20 +++++++++++++++++-- .../src/protoconf/util.pc.cc | 15 +++++++++++--- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/cmd/protoc-gen-cpp-tableau-loader/embed/util.pc.cc b/cmd/protoc-gen-cpp-tableau-loader/embed/util.pc.cc index 9672a012..8fcb9450 100644 --- a/cmd/protoc-gen-cpp-tableau-loader/embed/util.pc.cc +++ b/cmd/protoc-gen-cpp-tableau-loader/embed/util.pc.cc @@ -6,9 +6,18 @@ #include "tableau/protobuf/tableau.pb.h" namespace tableau { -static thread_local std::string g_err_msg; -const std::string& GetErrMsg() { return g_err_msg; } -void SetErrMsg(const std::string& msg) { g_err_msg = msg; } +namespace { +// NOTE: Use a function-local thread_local (Meyers singleton) instead of a +// namespace-scope thread_local to avoid MSVC static/TLS destruction order +// issues at process exit (observed as AV in __acrt_lock during the dynamic +// initializer/destructor of a thread_local std::string when /MTd is used). +std::string& ErrMsgRef() { + static thread_local std::string g_err_msg; + return g_err_msg; +} +} // namespace +const std::string& GetErrMsg() { return ErrMsgRef(); } +void SetErrMsg(const std::string& msg) { ErrMsgRef() = msg; } const std::string kUnknownExt = ".unknown"; const std::string kJSONExt = ".json"; diff --git a/test/cpp-tableau-loader/CMakeLists.txt b/test/cpp-tableau-loader/CMakeLists.txt index 67f04cc5..ff9a16d1 100644 --- a/test/cpp-tableau-loader/CMakeLists.txt +++ b/test/cpp-tableau-loader/CMakeLists.txt @@ -24,11 +24,27 @@ set(CMAKE_CXX_STANDARD_REQUIRED True) message(STATUS "Using C++${CMAKE_CXX_STANDARD} standard") if (MSVC) - SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4 /DNDEBUG") + # NOTE: do NOT append /DNDEBUG here. CMAKE_CXX_FLAGS applies to every build + # configuration, but we already let CMAKE_BUILD_TYPE drive optimization / + # assert behaviour via the per-config flags (CMAKE_CXX_FLAGS_DEBUG, + # CMAKE_CXX_FLAGS_RELEASE, ...). Worse, MSVC's STL uses NDEBUG to pick + # _ITERATOR_DEBUG_LEVEL: when _DEBUG is defined (Debug CRT /MTd) but NDEBUG + # is also forced on, our own translation units silently switch to IDL=0 + # while protobuf and googletest (built without NDEBUG in Debug) stay at + # IDL=2. The CRT defaultlib check still passes (everything is /MTd), so the + # link succeeds, but std::vector / std::string have incompatible layouts + # across modules and the process crashes at global-destructor time with an + # access violation -- exactly what we observed in `gtest_discover_tests`. + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4") # Use static CRT (/MT or /MTd) to match protobuf's default static runtime build. set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") else() - SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -g -fPIC -Wno-deprecated -Wno-unused-variable -Wno-sign-compare -Wno-strict-aliasing -fno-strict-aliasing -DNDEBUG") + # Same reasoning as the MSVC branch above: do not hard-code -DNDEBUG into + # the always-on flags. On Linux it does not change STL ABI (libstdc++ + # ignores NDEBUG for layout), so it is "merely" inconsistent with Debug + # builds elsewhere -- still better to leave assert() honoring the build + # type chosen by the user. + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -g -fPIC -Wno-deprecated -Wno-unused-variable -Wno-sign-compare -Wno-strict-aliasing -fno-strict-aliasing") endif() # google protobuf diff --git a/test/cpp-tableau-loader/src/protoconf/util.pc.cc b/test/cpp-tableau-loader/src/protoconf/util.pc.cc index 39225b77..381a054e 100644 --- a/test/cpp-tableau-loader/src/protoconf/util.pc.cc +++ b/test/cpp-tableau-loader/src/protoconf/util.pc.cc @@ -12,9 +12,18 @@ #include "tableau/protobuf/tableau.pb.h" namespace tableau { -static thread_local std::string g_err_msg; -const std::string& GetErrMsg() { return g_err_msg; } -void SetErrMsg(const std::string& msg) { g_err_msg = msg; } +namespace { +// NOTE: Use a function-local thread_local (Meyers singleton) instead of a +// namespace-scope thread_local to avoid MSVC static/TLS destruction order +// issues at process exit (observed as AV in __acrt_lock during the dynamic +// initializer/destructor of a thread_local std::string when /MTd is used). +std::string& ErrMsgRef() { + static thread_local std::string g_err_msg; + return g_err_msg; +} +} // namespace +const std::string& GetErrMsg() { return ErrMsgRef(); } +void SetErrMsg(const std::string& msg) { ErrMsgRef() = msg; } const std::string kUnknownExt = ".unknown"; const std::string kJSONExt = ".json"; From 6891fa8838d6b708f9bce18260fe9007158a2872 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Mon, 18 May 2026 20:51:38 +0800 Subject: [PATCH 16/16] chore: normalize line endings to LF for text files --- .gitattributes | 30 ++++---- init.sh | 2 +- test/testdata/conf/PatchMergeConf.json | 72 +++++++++---------- test/testdata/conf/PatchReplaceConf.json | 12 ++-- test/testdata/patchconf/PatchMergeConf.json | 30 ++++---- test/testdata/patchconf/PatchReplaceConf.json | 12 ++-- test/testdata/patchconf2/PatchMergeConf.json | 42 +++++------ test/testdata/patchconf2/PatchMergeConf.txtpb | 54 +++++++------- 8 files changed, 127 insertions(+), 127 deletions(-) diff --git a/.gitattributes b/.gitattributes index 287ff3ef..cca4a396 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,16 +1,16 @@ -# Treat all files in this repo as binary, with no git magic updating -# line endings. This produces predictable results in different environments. -# -# Windows users contributing to this repo will need to use a modern version -# of git and editors capable of LF line endings. -# -# Windows .bat files are known to have multiple bugs when run with LF -# endings, and so they are checked in with CRLF endings, with a test -# to catch problems. (See https://github.com/golang/go/issues/37791.) -# -# You can prevent accidental CRLF line endings from entering the repo -# via PR/MR checks. -# -# See https://github.com/golang/go/issues/9281. -# See https://adaptivepatchwork.com/2012/03/01/mind-the-end-of-your-line. +# Treat all files in this repo as binary, with no git magic updating +# line endings. This produces predictable results in different environments. +# +# Windows users contributing to this repo will need to use a modern version +# of git and editors capable of LF line endings. +# +# Windows .bat files are known to have multiple bugs when run with LF +# endings, and so they are checked in with CRLF endings, with a test +# to catch problems. (See https://github.com/golang/go/issues/37791.) +# +# You can prevent accidental CRLF line endings from entering the repo +# via PR/MR checks. +# +# See https://github.com/golang/go/issues/9281. +# See https://adaptivepatchwork.com/2012/03/01/mind-the-end-of-your-line. * -text \ No newline at end of file diff --git a/init.sh b/init.sh index b70551bf..d5b25105 100755 --- a/init.sh +++ b/init.sh @@ -127,5 +127,5 @@ cmake --build .build --parallel cmake --install .build --prefix .build/_install # Persist the signature so the next run can fast-path skip when nothing changed. -printf '%s\n' "${EXPECTED_SIGNATURE}" > "${SIG_FILE}" +printf '%s\n' "${EXPECTED_SIGNATURE}" >"${SIG_FILE}" echo "[INFO] Wrote build signature to ${SIG_FILE}" diff --git a/test/testdata/conf/PatchMergeConf.json b/test/testdata/conf/PatchMergeConf.json index b028ba76..95b717c2 100644 --- a/test/testdata/conf/PatchMergeConf.json +++ b/test/testdata/conf/PatchMergeConf.json @@ -1,37 +1,37 @@ -{ - "name": "apple", - "name2": "apple2", - "name3": "apple3", - "time": { - "start": "2024-10-01T02:10:10Z", - "expiry": "3600s" - }, - "priceList": [ - 10, - 100 - ], - "replacePriceList": [ - 10, - 100 - ], - "itemMap": { - "1": { - "id": 1, - "num": 10 - }, - "2": { - "id": 2, - "num": 20 - } - }, - "replaceItemMap": { - "1": { - "id": 1, - "num": 10 - }, - "2": { - "id": 2, - "num": 20 - } - } +{ + "name": "apple", + "name2": "apple2", + "name3": "apple3", + "time": { + "start": "2024-10-01T02:10:10Z", + "expiry": "3600s" + }, + "priceList": [ + 10, + 100 + ], + "replacePriceList": [ + 10, + 100 + ], + "itemMap": { + "1": { + "id": 1, + "num": 10 + }, + "2": { + "id": 2, + "num": 20 + } + }, + "replaceItemMap": { + "1": { + "id": 1, + "num": 10 + }, + "2": { + "id": 2, + "num": 20 + } + } } \ No newline at end of file diff --git a/test/testdata/conf/PatchReplaceConf.json b/test/testdata/conf/PatchReplaceConf.json index 9520af95..ea378647 100644 --- a/test/testdata/conf/PatchReplaceConf.json +++ b/test/testdata/conf/PatchReplaceConf.json @@ -1,7 +1,7 @@ -{ - "name": "apple", - "priceList": [ - 10, - 100 - ] +{ + "name": "apple", + "priceList": [ + 10, + 100 + ] } \ No newline at end of file diff --git a/test/testdata/patchconf/PatchMergeConf.json b/test/testdata/patchconf/PatchMergeConf.json index 8ac6e500..b11e54c9 100644 --- a/test/testdata/patchconf/PatchMergeConf.json +++ b/test/testdata/patchconf/PatchMergeConf.json @@ -1,16 +1,16 @@ -{ - "name": "orange", - "name2": "", - "name3": "", - "time": { - "expiry": "7200s" - }, - "priceList": [ - 20, - 200 - ], - "replacePriceList": [ - 20, - 200 - ] +{ + "name": "orange", + "name2": "", + "name3": "", + "time": { + "expiry": "7200s" + }, + "priceList": [ + 20, + 200 + ], + "replacePriceList": [ + 20, + 200 + ] } \ No newline at end of file diff --git a/test/testdata/patchconf/PatchReplaceConf.json b/test/testdata/patchconf/PatchReplaceConf.json index ce27d4a7..68dfd6ab 100644 --- a/test/testdata/patchconf/PatchReplaceConf.json +++ b/test/testdata/patchconf/PatchReplaceConf.json @@ -1,7 +1,7 @@ -{ - "name": "orange", - "priceList": [ - 20, - 200 - ] +{ + "name": "orange", + "priceList": [ + 20, + 200 + ] } \ No newline at end of file diff --git a/test/testdata/patchconf2/PatchMergeConf.json b/test/testdata/patchconf2/PatchMergeConf.json index bfaa3b00..e3ca02a0 100644 --- a/test/testdata/patchconf2/PatchMergeConf.json +++ b/test/testdata/patchconf2/PatchMergeConf.json @@ -1,22 +1,22 @@ -{ - "itemMap": { - "1": { - "id": 1, - "num": 99 - }, - "999": { - "id": 999, - "num": 99900 - } - }, - "replaceItemMap": { - "1": { - "id": 1, - "num": 99 - }, - "999": { - "id": 999, - "num": 99900 - } - } +{ + "itemMap": { + "1": { + "id": 1, + "num": 99 + }, + "999": { + "id": 999, + "num": 99900 + } + }, + "replaceItemMap": { + "1": { + "id": 1, + "num": 99 + }, + "999": { + "id": 999, + "num": 99900 + } + } } \ No newline at end of file diff --git a/test/testdata/patchconf2/PatchMergeConf.txtpb b/test/testdata/patchconf2/PatchMergeConf.txtpb index d482eab6..6eb2bb78 100644 --- a/test/testdata/patchconf2/PatchMergeConf.txtpb +++ b/test/testdata/patchconf2/PatchMergeConf.txtpb @@ -1,28 +1,28 @@ -item_map: { - key: 1 - value: { - id: 1 - num: 99 - } -} -item_map: { - key: 999 - value: { - id: 999 - num: 99900 - } -} -replace_item_map: { - key: 1 - value: { - id: 1 - num: 99 - } -} -replace_item_map: { - key: 999 - value: { - id: 999 - num: 99900 - } +item_map: { + key: 1 + value: { + id: 1 + num: 99 + } +} +item_map: { + key: 999 + value: { + id: 999 + num: 99900 + } +} +replace_item_map: { + key: 1 + value: { + id: 1 + num: 99 + } +} +replace_item_map: { + key: 999 + value: { + id: 999 + num: 99900 + } } \ No newline at end of file