Better support for repeated fields in the config language. Add a helper

for showing all config fields in a proto.

lib/config/common.proto

@@ -2,28 +2,35 @@ syntax = "proto2";
 
 import "google/protobuf/descriptor.proto";
 
-message RangeProto {
+extend google.protobuf.FieldOptions
-	optional int32 start = 1 [default = 0, (help) = "start value"];
+{
-	optional int32 step = 2 [default = 1, (help) = "amount to step by (positive)"];
+    optional string help = 50000;
-	optional int32 end = 3 [(help) = "inclusive end value, defaulting to the start value"];
 }
 
-extend google.protobuf.FieldOptions {
+extend google.protobuf.MessageOptions
-  optional string help = 50000;
+{
     optional bool recurse = 50001 [default = true];
 }
 
-enum IndexMode {
+message RangeProto
-	INDEXMODE_DRIVE = 0;
+{
-	INDEXMODE_300 = 1;
+    option(recurse) = false;
-	INDEXMODE_360 = 2;
+
+    optional int32 start = 1 [default = 0, (help) = "start value"];
+    optional int32 step =
+        2 [default = 1, (help) = "amount to step by (positive)"];
+    optional int32 end =
+        3 [(help) = "inclusive end value, defaulting to the start value"];
 }
 
-enum FluxSourceSinkType {
+enum IndexMode
-	FLUXTYPE_NOT_SET = 0;
+{
-	FLUXTYPE_A2R = 1;
+    INDEXMODE_DRIVE = 0; INDEXMODE_300 = 1; INDEXMODE_360 = 2;
-	FLUXTYPE_AU = 2;
+}
-	FLUXTYPE_CWF = 3;
+
+enum FluxSourceSinkType
+{
+    FLUXTYPE_NOT_SET = 0; FLUXTYPE_A2R = 1; FLUXTYPE_AU = 2; FLUXTYPE_CWF = 3;
     FLUXTYPE_DRIVE = 4;
     FLUXTYPE_ERASE = 5;
     FLUXTYPE_FLUX = 6;
@@ -35,10 +42,9 @@ enum FluxSourceSinkType {
     FLUXTYPE_DMK = 12;
 }
 
-enum ImageReaderWriterType {
+enum ImageReaderWriterType
-	IMAGETYPE_NOT_SET = 0;
+{
-	IMAGETYPE_D64 = 1;
+    IMAGETYPE_NOT_SET = 0; IMAGETYPE_D64 = 1; IMAGETYPE_D88 = 2;
-	IMAGETYPE_D88 = 2;
     IMAGETYPE_DIM = 3;
     IMAGETYPE_DISKCOPY = 4;
     IMAGETYPE_FDI = 5;
@@ -51,5 +57,3 @@ enum ImageReaderWriterType {
     IMAGETYPE_RAW = 12;
     IMAGETYPE_TD0 = 13;
 }
-
-

lib/config/config.proto

@@ -14,14 +14,14 @@ import "lib/config/layout.proto";
 
 enum SupportStatus
 {
-    UNSUPPORTED = 0;
+    UNSUPPORTED = 0; DINOSAUR = 1; UNICORN = 2;
-    DINOSAUR = 1;
-    UNICORN = 2;
 }
 
 // NEXT_TAG: 27
 message ConfigProto
 {
+    option(recurse) = false;
+
     optional string shortname = 1;
     optional string comment = 2;
     optional bool is_extension = 3;
@@ -61,15 +61,14 @@ message OptionProto
 {
     optional string name = 1 [(help) = "option name"];
     optional string comment = 2 [(help) = "help text for option"];
-    optional string message = 3
+    optional string message =
-        [ (help) = "message to display when option is in use" ];
+        3 [(help) = "message to display when option is in use"];
-    optional bool set_by_default = 6
+    optional bool set_by_default =
-        [ (help) = "this option is applied by default", default = false ];
+        6 [(help) = "this option is applied by default", default = false];
-    repeated OptionPrerequisiteProto prerequisite = 7
+    repeated OptionPrerequisiteProto prerequisite =
-        [ (help) = "prerequisites for this option" ];
+        7 [(help) = "prerequisites for this option"];
 
-    optional ConfigProto config = 4
+    optional ConfigProto config = 4 [(help) = "option data"];
-        [ (help) = "option data", (recurse) = false ];
 }
 
 message OptionGroupProto

lib/config/flags.cc

@@ -264,7 +264,7 @@ static void doShowConfig()
 
 static void doDoc()
 {
-    const auto fields = findAllProtoFields(globalConfig().base());
+    const auto fields = findAllPossibleProtoFields(globalConfig().base()->GetDescriptor());
     for (const auto field : fields)
     {
         const std::string& path = field.first;

lib/config/proto.cc

@@ -107,6 +107,16 @@ static ProtoField resolveProtoPath(
     std::stringstream ss(leading);
     while (std::getline(ss, item, '.'))
     {
+        static const std::regex INDEX_REGEX("(\\w+)\\[([0-9]+)\\]");
+
+        int index = -1;
+        std::smatch dmatch;
+        if (std::regex_match(item, dmatch, INDEX_REGEX))
+        {
+            item = dmatch[1];
+            index = std::stoi(dmatch[2]);
+        }
+
         const auto* field = descriptor->FindFieldByName(item);
         if (!field)
             throw ProtoPathNotFoundException(
@@ -116,6 +126,14 @@ static ProtoField resolveProtoPath(
                 "config field '{}' in '{}' is not a message", item, path));
 
         const auto* reflection = message->GetReflection();
+        if ((field->label() !=
+                google::protobuf::FieldDescriptor::LABEL_REPEATED) &&
+            (index != -1))
+            throw ProtoPathNotFoundException(fmt::format(
+                "config field '{}[{}]' is indexed, but not repeated",
+                item,
+                index));
+
         switch (field->label())
         {
             case google::protobuf::FieldDescriptor::LABEL_OPTIONAL:
@@ -127,16 +145,15 @@ static ProtoField resolveProtoPath(
                 break;
 
             case google::protobuf::FieldDescriptor::LABEL_REPEATED:
-                if (reflection->FieldSize(*message, field) == 0)
+                if (index == -1)
-                {
+                    throw ProtoPathNotFoundException(fmt::format(
-                    if (create)
+                        "config field '{}' is repeated and must be indexed",
-                        message = reflection->AddMessage(message, field);
+                        item));
-                    else
+                while (reflection->FieldSize(*message, field) <= index)
-                        fail();
+                    reflection->AddMessage(message, field);
-                }
+
-                else
                 message =
-                        reflection->MutableRepeatedMessage(message, field, 0);
+                    reflection->MutableRepeatedMessage(message, field, index);
                 break;
 
             default:
@@ -343,11 +360,17 @@ std::set<unsigned> iterate(unsigned start, unsigned count)
     return set;
 }
 
+static bool shouldRecurse(const google::protobuf::FieldDescriptor* f)
+{
+    if (f->type() != google::protobuf::FieldDescriptor::TYPE_MESSAGE)
+		return false;
+	return f->message_type()->options().GetExtension(::recurse);
+}
+
 std::map<std::string, const google::protobuf::FieldDescriptor*>
-findAllProtoFields(google::protobuf::Message* message)
+findAllPossibleProtoFields(const google::protobuf::Descriptor* descriptor)
 {
     std::map<std::string, const google::protobuf::FieldDescriptor*> fields;
-    const auto* descriptor = message->GetDescriptor();
 
     std::function<void(const google::protobuf::Descriptor*, const std::string&)>
         recurse = [&](auto* d, const auto& s)
@@ -357,8 +380,10 @@ findAllProtoFields(google::protobuf::Message* message)
             const google::protobuf::FieldDescriptor* f = d->field(i);
             std::string n = s + f->name();
 
-            if (f->options().GetExtension(::recurse) &&
+            if (f->label() == google::protobuf::FieldDescriptor::LABEL_REPEATED)
-                (f->type() == google::protobuf::FieldDescriptor::TYPE_MESSAGE))
+                n += "[]";
+
+            if (shouldRecurse(f))
                 recurse(f->message_type(), n + ".");
 
             fields[n] = f;
@@ -369,10 +394,47 @@ findAllProtoFields(google::protobuf::Message* message)
     return fields;
 }
 
-ConfigProto parseConfigBytes(const std::string_view& data)
+std::map<std::string, const google::protobuf::FieldDescriptor*>
+findAllProtoFields(const google::protobuf::Message& message)
 {
-    ConfigProto proto;
+    std::map<std::string, const google::protobuf::FieldDescriptor*> allFields;
-    if (!proto.ParseFromArray(data.begin(), data.size()))
+
-        error("invalid internal config data");
+    std::function<void(const google::protobuf::Message&, const std::string&)>
-    return proto;
+        recurse = [&](auto& message, const auto& name)
+    {
+        const auto* reflection = message.GetReflection();
+        std::vector<const google::protobuf::FieldDescriptor*> fields;
+        reflection->ListFields(message, &fields);
+
+        for (const auto* f : fields)
+        {
+            auto basename = name;
+            if (!basename.empty())
+                basename += '.';
+            basename += f->name();
+
+            if (f->label() == google::protobuf::FieldDescriptor::LABEL_REPEATED)
+            {
+                for (int i = 0; i < reflection->FieldSize(message, f); i++)
+                {
+                    const auto n = fmt::format("{}[{}]", basename, i);
+                    if (shouldRecurse(f))
+                        recurse(
+                            reflection->GetRepeatedMessage(message, f, i), n);
+                    else
+                        allFields[n] = f;
+                }
+            }
+            else
+            {
+                if (shouldRecurse(f))
+                    recurse(reflection->GetMessage(message, f), basename);
+                else
+                    allFields[basename] = f;
+            }
+        }
+    };
+
+    recurse(message, "");
+    return allFields;
 }

lib/config/proto.h

@@ -37,9 +37,19 @@ extern std::set<unsigned> iterate(const RangeProto& range);
 extern std::set<unsigned> iterate(unsigned start, unsigned count);
 
 extern std::map<std::string, const google::protobuf::FieldDescriptor*>
-findAllProtoFields(google::protobuf::Message* message);
+findAllPossibleProtoFields(const google::protobuf::Descriptor* descriptor);
 
-extern ConfigProto parseConfigBytes(const std::string_view& bytes);
+extern std::map<std::string, const google::protobuf::FieldDescriptor*>
+findAllProtoFields(const google::protobuf::Message& message);
+
+template <class T>
+static inline const T parseProtoBytes(const std::string_view& bytes)
+{
+    T proto;
+    if (!proto.ParseFromArray(bytes.begin(), bytes.size()))
+        error("invalid internal proto data");
+    return proto;
+}
 
 extern const std::map<std::string, const ConfigProto*> formats;
 

lib/core/globals.h

@@ -23,6 +23,9 @@
 #define mkdir(A, B) _mkdir(A)
 #endif
 
+#define STRINGIFY(a) XSTRINGIFY(a)
+#define XSTRINGIFY(a) #a
+
 template <class T>
 static inline std::vector<T> vector_of(T item)
 {

scripts/build.py

@@ -5,13 +5,14 @@ encoders = {}
 
 
 @Rule
-def protoencode_single(self, name, srcs: Targets, proto, symbol):
+def protoencode_single(self, name, srcs: Targets, proto, include, symbol):
     if proto not in encoders:
         r = cxxprogram(
             name="protoencode_" + proto,
             srcs=["scripts/protoencode.cc"],
-            cflags=["-DPROTO=" + proto],
+            cflags=["-DPROTO=" + proto, "-DINCLUDE="+include],
             deps=[
+                "lib/core",
                 "lib/config+proto_lib",
                 "lib/fluxsource+proto_lib",
                 "lib/fluxsink+proto_lib",
@@ -41,12 +42,13 @@ def protoencode_single(self, name, srcs: Targets, proto, symbol):
 
 
 @Rule
-def protoencode(self, name, proto, srcs: TargetsMap, symbol):
+def protoencode(self, name, proto, include,srcs: TargetsMap, symbol):
     encoded = [
         protoencode_single(
             name=f"{k}_cc",
             srcs=[v],
             proto=proto,
+            include=include,
             symbol=f"{symbol}_{k}_pb",
         )
         for k, v in srcs.items()

scripts/protoencode.cc

@@ -3,12 +3,13 @@
 #include <google/protobuf/io/zero_copy_stream_impl.h>
 #include <fstream>
 #include "fmt/format.h"
+#include "lib/core/globals.h"
 #include "tests/testproto.pb.h"
 #include "lib/config/config.pb.h"
 #include <sstream>
 #include <locale>
 
-#define STRINGIFY(s) #s
+const std::string protoname = STRINGIFY(PROTO);
 
 static uint32_t readu8(std::string::iterator& it, std::string::iterator end)
 {
@@ -125,6 +126,7 @@ int main(int argc, const char* argv[])
 
     output << "#include \"lib/core/globals.h\"\n"
            << "#include \"lib/config/proto.h\"\n"
+           << "#include \"" STRINGIFY(INCLUDE) "\"\n"
            << "#include <string_view>\n"
            << "static const uint8_t " << name << "_rawData[] = {";
 
@@ -143,11 +145,11 @@ int main(int argc, const char* argv[])
     output << "\n};\n";
     output << "extern const std::string_view " << name << "_data;\n";
     output << "const std::string_view " << name
-           << "_data = std::string_view((const char*)" << name << "_rawData, " << data.size()
+           << "_data = std::string_view((const char*)" << name << "_rawData, "
-           << ");\n";
+           << data.size() << ");\n";
-    output << "extern const ConfigProto " << name << ";\n";
+    output << "extern const " << protoname << " " << name << ";\n";
-    output << "const ConfigProto " << name << " = parseConfigBytes("
+    output << "const " << protoname << " " << name << " = parseProtoBytes<"
-           << argv[3] << "_data);\n";
+           << protoname << ">(" << argv[3] << "_data);\n";
 
     return 0;
 }

tests/build.py

@@ -39,44 +39,38 @@ tests = [
     "vfs",
 ]
 
-export(
-    name="tests",
-    deps=[
-        test(
-            name="proto_test",
-            command=cxxprogram(
-                name="proto_test_exe",
-                srcs=[
-                    "./proto.cc",
 protoencode_single(
     name="testproto_cc",
     srcs=["./testproto.textpb"],
     proto="TestProto",
+    include="tests/testproto.pb.h",
     symbol="testproto_pb",
-                    ),
+)
+
+export(
+    name="tests",
+    deps=[
+        test(
+            name=f"{n}_test",
+            command=cxxprogram(
+                name=f"{n}_test_exe",
+                srcs=[
+                    f"./{n}.cc",
+                    ".+testproto_cc",
                 ],
                 deps=[
-                    "lib/external+fl2_proto_lib",
                     "+fmt_lib",
                     "+protobuf_lib",
                     "+protocol",
-                    "+z_lib",
                     ".+test_proto_lib",
-                    "dep/adflib",
-                    "dep/agg",
-                    "dep/fatfs",
-                    "dep/hfsutils",
-                    "dep/libusbp",
                     "dep/snowhouse",
-                    "dep/stb",
                     "lib/config",
                     "lib/core",
-                    "lib/data",
                     "lib/fluxsource+proto_lib",
-                    "src/formats",
                 ],
             ),
-        ),
+        )
+        for n in ["proto"]
     ]
     + [
         test(

tests/proto.cc

@@ -28,8 +28,9 @@ static void test_setting(void)
     setProtoByString(&config, "d", "5.5");
     setProtoByString(&config, "f", "6.7");
     setProtoByString(&config, "m.s", "string");
-    setProtoByString(&config, "r.s", "val1");
+    setProtoByString(&config, "r[0].s", "val1");
-    setProtoByString(&config, "r.s", "val2");
+    setProtoByString(&config, "r[0].s", "val2");
+    setProtoByString(&config, "r[1].s", "val3");
     setProtoByString(&config, "firstoption.s", "1");
     setProtoByString(&config, "secondoption.s", "2");
     setProtoByString(&config, "range", "1-3x2");
@@ -49,6 +50,9 @@ static void test_setting(void)
 		r {
 			s: "val2"
 		}
+		r {
+			s: "val3"
+		}
 		secondoption {
 			s: "2"
 		}
@@ -76,6 +80,9 @@ static void test_getting(void)
 		r {
 			s: "val2"
 		}
+		r {
+			s: "val3"
+		}
 		secondoption {
 			s: "2"
 		}
@@ -97,7 +104,8 @@ static void test_getting(void)
     AssertThat(getProtoByString(&tp, "d"), Equals("5.5"));
     AssertThat(getProtoByString(&tp, "f"), Equals("6.7"));
     AssertThat(getProtoByString(&tp, "m.s"), Equals("string"));
-    AssertThat(getProtoByString(&tp, "r.s"), Equals("val2"));
+    AssertThat(getProtoByString(&tp, "r[0].s"), Equals("val2"));
+    AssertThat(getProtoByString(&tp, "r[1].s"), Equals("val3"));
     AssertThrows(
         ProtoPathNotFoundException, getProtoByString(&tp, "firstoption.s"));
     AssertThat(getProtoByString(&tp, "secondoption.s"), Equals("2"));
@@ -206,8 +214,27 @@ static void test_range(void)
 static void test_fields(void)
 {
     TestProto proto;
-    auto fields = findAllProtoFields(&proto);
+    auto fields = findAllPossibleProtoFields(proto.GetDescriptor());
-    AssertThat(fields.size(), Equals(18));
+    std::vector<std::string> fieldNames;
+    for (const auto& e : fields)
+        fieldNames.push_back(e.first);
+
+    AssertThat(fieldNames,
+        Equals(std::vector<std::string>{"d",
+            "f",
+            "firstoption",
+            "firstoption.s",
+            "i32",
+            "i64",
+            "m",
+            "m.s",
+            "r[]",
+            "r[].s",
+            "range",
+            "secondoption",
+            "secondoption.s",
+            "u32",
+            "u64"}));
 }
 
 static void test_options(void)
@@ -220,6 +247,57 @@ static void test_options(void)
     AssertThat(s, Equals("i64"));
 }
 
+static void test_findallfields(void)
+{
+    std::string s = R"M(
+		i64: -1
+		i32: -2
+		u64: 3
+		u32: 4
+		d: 5.5
+		f: 6.7
+		m {
+			s: "string"
+		}
+		r {
+			s: "val2"
+		}
+		r {
+			s: "val3"
+		}
+		secondoption {
+			s: "2"
+		}
+		range {
+			start: 1
+			step: 2
+			end: 3
+		}
+	)M";
+
+    TestProto proto;
+    if (!google::protobuf::TextFormat::MergeFromString(cleanup(s), &proto))
+        error("couldn't load test proto");
+
+    auto fields = findAllProtoFields(proto);
+    std::vector<std::string> fieldNames;
+    for (const auto& e : fields)
+        fieldNames.push_back(e.first);
+
+    AssertThat(fieldNames,
+        Equals(std::vector<std::string>{"d",
+            "f",
+            "i32",
+            "i64",
+            "m.s",
+            "r[0].s",
+            "r[1].s",
+            "range",
+            "secondoption.s",
+            "u32",
+            "u64"}));
+}
+
 int main(int argc, const char* argv[])
 {
     try
@@ -231,6 +309,7 @@ int main(int argc, const char* argv[])
         test_range();
         test_fields();
         test_options();
+        test_findallfields();
     }
     catch (const ErrorException& e)
     {

tests/testproto.proto

@@ -2,8 +2,10 @@ syntax = "proto2";
 
 import "lib/config/common.proto";
 
-message TestProto {
+message TestProto
-	message SubMessageProto {
+{
+    message SubMessageProto
+    {
         optional string s = 1;
     }
 
@@ -16,11 +18,11 @@ message TestProto {
     optional SubMessageProto m = 6;
     repeated SubMessageProto r = 7;
 
-	oneof alt {
+    oneof alt
+    {
         SubMessageProto firstoption = 8;
         SubMessageProto secondoption = 9;
     }
 
     optional RangeProto range = 10;
 }
-