Sysadmin fails to build under GCC 7.x

[mtp401]

 59%] Building CXX object decibel-cpp/src/CMakeFiles/decibel.dir/decibel/Protobuf.cpp.o
In file included from /home/mpelland/work/oss-sysadmin/decibel-cpp/src/decibel/Protobuf.cpp:1:0:
/home/mpelland/work/oss-sysadmin/decibel-cpp/include/decibel/Protobuf.h:79:12: error: redefinition of ‘struct decibel::ProtobufSetter<long int>’
     struct ProtobufSetter<TYPE>                                                \
            ^~~~~~~~~~~~~~~~~~~~
/home/mpelland/work/oss-sysadmin/decibel-cpp/include/decibel/Protobuf.h:103:1: note: in expansion of macro ‘DEFINE_PROTOBUF_FIELD_MAPPING’
 DEFINE_PROTOBUF_FIELD_MAPPING(long, Int64);
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mpelland/work/oss-sysadmin/decibel-cpp/include/decibel/Protobuf.h:79:12: note: previous definition of ‘struct decibel::ProtobufSetter<long int>’
     struct ProtobufSetter<TYPE>                                                \
            ^~~~~~~~~~~~~~~~~~~~
/home/mpelland/work/oss-sysadmin/decibel-cpp/include/decibel/Protobuf.h:102:1: note: in expansion of macro ‘DEFINE_PROTOBUF_FIELD_MAPPING’
 DEFINE_PROTOBUF_FIELD_MAPPING(int64_t, Int64);
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mpelland/work/oss-sysadmin/decibel-cpp/include/decibel/Protobuf.h:79:12: error: redefinition of ‘struct decibel::ProtobufSetter<long unsigned int>’
     struct ProtobufSetter<TYPE>                                                \
            ^~~~~~~~~~~~~~~~~~~~
/home/mpelland/work/oss-sysadmin/decibel-cpp/include/decibel/Protobuf.h:106:1: note: in expansion of macro ‘DEFINE_PROTOBUF_FIELD_MAPPING’
 DEFINE_PROTOBUF_FIELD_MAPPING(unsigned long, UInt64);
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mpelland/work/oss-sysadmin/decibel-cpp/include/decibel/Protobuf.h:79:12: note: previous definition of ‘struct decibel::ProtobufSetter<long unsigned int>’
     struct ProtobufSetter<TYPE>                                                \
            ^~~~~~~~~~~~~~~~~~~~
/home/mpelland/work/oss-sysadmin/decibel-cpp/include/decibel/Protobuf.h:105:1: note: in expansion of macro ‘DEFINE_PROTOBUF_FIELD_MAPPING’
 DEFINE_PROTOBUF_FIELD_MAPPING(uint64_t, UInt64);
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [decibel-cpp/src/CMakeFiles/decibel.dir/build.make:591: decibel-cpp/src/CMakeFiles/decibel.dir/decibel/Protobuf.cpp.o] Error 1

[edbrown23]

Arguably it's decibel-cpp that doesn't build, but I think I know what this issue is. I blame OSX

[mckaymatt]

I was able to run the build, but not the tests, just by removing the changes to decibel-cpp/include/decibel/Protobuf.h from the OSX compatibility commit. I'm not sure GCC7 is to blame because it failed under 5 and 6 as well.
685ccd9#diff-2cf25f626211d3a68ade9300aa0619f3R103

[edbrown23]

Now that you mention it, there's a fix on this branch I made for setting up CI: 322fdd3

[mckaymatt]

with those changes I'm only seeing a single error on tests.

[ RUN      ] HookInitFixture.HookFulfillingOptionalHooks
/home/user/sysadmin/test/test_hook_initializer.cpp:111: Failure
      Expected: "requiredhook"
To be equal to: pairs[1].ToString()
      Which is: "optionalhook"
[  FAILED  ] HookInitFixture.HookFulfillingOptionalHooks (0 ms)

https://github.com/StarryInternet/sysadmin/blob/master/test/test_hook_initializer.cpp#L111
Lines 111 and 112 are:

    ASSERT_EQ("requiredhook", pairs[1].ToString());
    ASSERT_EQ("optionalhook", pairs[0].ToString());

but it looks like they should be:

    ASSERT_EQ("requiredhook", pairs[0].ToString());
    ASSERT_EQ("optionalhook", pairs[1].ToString());

Making those changes fixes the tests. Can I commit that change or does the existing test reflect the logic we want? @edbrown23

[edbrown23]

In theory you can make that change, I just hesitated to do it because it feels like a regression, and one that I don't remember what would have caused it. I'd like to do some bisecting to figure out what the source of that change is, though it might be totally innocuous/idiotic