Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-3584

Port Qserv to OS X/Clang

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: Qserv
    • Labels:
      None
    • Team:
      Architecture

      Description

      Now that the Qserv dependencies work on OS X (DM-1662) this ticket details the issues that arise from attempting to build Qserv on OS X using the clang compiler.

      A preliminary investigation finds:

      • site_scons/state.py enables the -rpath-link linker option which is not supported on OS X.
      • -Wno-unused-local-typedefs is not supported by clang (see DM-869).
      • -pthread link option is ignored.
      • libqserv_css links against log4cxx but that library is not listed explicitly in core/modules/SConscript.
      • The python/swig bindings _cssLib.so do not link properly.

      Also clang issues a fatal compiler error from within core/modules/mysql/MySqlConnection.h:

      In file included from build/mysql/MySqlConnection.cc:29:
      build/mysql/MySqlConnection.h:73:23: error: implicit instantiation of undefined template 'std::__1::basic_string<char, std::__1::char_traits<char>,
            std::__1::allocator<char> >'
          const std::string getError() const { assert(_mysql); return std::string(mysql_error(_mysql)); }
                            ^
      /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/iosfwd:188:33: note: template is declared
            here
          class _LIBCPP_TYPE_VIS_ONLY basic_string;
                                      ^
      In file included from build/mysql/MySqlConnection.cc:29:
      build/mysql/MySqlConnection.h:73:65: error: implicit instantiation of undefined template 'std::__1::basic_string<char, std::__1::char_traits<char>,
            std::__1::allocator<char> >'
          const std::string getError() const { assert(_mysql); return std::string(mysql_error(_mysql)); }
                                                                      ^
      /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/iosfwd:188:33: note: template is declared
            here
          class _LIBCPP_TYPE_VIS_ONLY basic_string;
                                      ^
      

      Compiler warnings

      In file included from build/sql/statement.cc:32:
      build/sql/Schema.h:74:1: warning: 'Schema' defined as a struct here but previously declared as a class [-Wmismatched-tags]
      struct Schema {
      ^
      build/sql/statement.h:35:1: note: did you mean struct here?
      class Schema; // Forward
      ^~~~~
      

      build/mysql/MySqlConnection.cc:147:41: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int]
          std::string killSql = "KILL QUERY " + boost::lexical_cast<int>(threadId);
                                ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      build/mysql/MySqlConnection.cc:147:41: note: use array indexing to silence this warning
          std::string killSql = "KILL QUERY " + boost::lexical_cast<int>(threadId);
                                              ^
                                &             [                                   ]
      

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            Fritz Mueller this was the error:

            g++ -o build/proto/ProtoHeaderWrap.os -c -std=c++11 -pedantic -Wall -Wno-long-long -Wno-variadic-macros -g -fPIC -D_FILE_OFFSET_BITS=64 -I/Users/timj/work/lsstsw/stack/DarwinX86/mysql/5.1.65.lsst2+da39a3ee5e/include -I/Users/timj/work/lsstsw/stack/DarwinX86/boost/1.55.0.1.lsst2+fbf04ba888/include -I/Users/timj/work/lsstsw/stack/DarwinX86/antlr/2.7.7.lsst1+da39a3ee5e/include -I/Users/timj/work/lsstsw/stack/DarwinX86/xrootd/u.timj.DM-3584-ge22410fa7f+da39a3ee5e/include/xrootd -I/Users/timj/work/lsstsw/stack/DarwinX86/log/1.1.2-5-gf327f39+2686ea5520/include -I/Users/timj/work/lsstsw/stack/DarwinX86/log4cxx/0.10.0.lsst4+7d4e39ddbb/include -I/Users/timj/work/lsstsw/stack/DarwinX86/protobuf/2.6.1+fbf04ba888/include -I/Users/timj/work/lsstsw/stack/DarwinX86/zookeeper/3.4.6.lsst2+da39a3ee5e/c-binding/include -Ibuild build/proto/ProtoHeaderWrap.cc
            In file included from build/proto/ProtoHeaderWrap.cc:30:
            In file included from build/proto/ProtoHeaderWrap.h:42:
            In file included from build/proto/WorkerResponse.h:28:
            In file included from build/proto/worker.pb.h:26:
            /Users/timj/work/lsstsw/stack/DarwinX86/protobuf/2.6.1+fbf04ba888/include/google/protobuf/unknown_field_set.h:214:13: warning: anonymous types
                  declared in an anonymous union are an extension [-Wnested-anon-types]
                mutable union {
                        ^
            build/proto/ProtoHeaderWrap.cc:47:24: error: non-constant-expression cannot be narrowed from type 'unsigned char' to 'char' in initializer list
                  [-Wc++11-narrowing]
                std::string msgBuf{phSize};
                                   ^~~~~~
            build/proto/ProtoHeaderWrap.cc:47:24: note: insert an explicit cast to silence this issue
                std::string msgBuf{phSize};
                                   ^~~~~~
                                   static_cast<char>( )
            1 warning and 1 error generated.
            

            I thought it was a bit silly to cast the thing we just created that's only used once. I'm happy to change it though.

            Regarding the comment on boost::format, I had seen other places in the code that used .cstr() so assumed that was idiomatic. I'll sort that out as well.

            Show
            tjenness Tim Jenness added a comment - Fritz Mueller this was the error: g++ -o build/proto/ProtoHeaderWrap.os -c -std=c++11 -pedantic -Wall -Wno-long-long -Wno-variadic-macros -g -fPIC -D_FILE_OFFSET_BITS=64 -I/Users/timj/work/lsstsw/stack/DarwinX86/mysql/5.1.65.lsst2+da39a3ee5e/include -I/Users/timj/work/lsstsw/stack/DarwinX86/boost/1.55.0.1.lsst2+fbf04ba888/include -I/Users/timj/work/lsstsw/stack/DarwinX86/antlr/2.7.7.lsst1+da39a3ee5e/include -I/Users/timj/work/lsstsw/stack/DarwinX86/xrootd/u.timj.DM-3584-ge22410fa7f+da39a3ee5e/include/xrootd -I/Users/timj/work/lsstsw/stack/DarwinX86/log/1.1.2-5-gf327f39+2686ea5520/include -I/Users/timj/work/lsstsw/stack/DarwinX86/log4cxx/0.10.0.lsst4+7d4e39ddbb/include -I/Users/timj/work/lsstsw/stack/DarwinX86/protobuf/2.6.1+fbf04ba888/include -I/Users/timj/work/lsstsw/stack/DarwinX86/zookeeper/3.4.6.lsst2+da39a3ee5e/c-binding/include -Ibuild build/proto/ProtoHeaderWrap.cc In file included from build/proto/ProtoHeaderWrap.cc:30: In file included from build/proto/ProtoHeaderWrap.h:42: In file included from build/proto/WorkerResponse.h:28: In file included from build/proto/worker.pb.h:26: /Users/timj/work/lsstsw/stack/DarwinX86/protobuf/2.6.1+fbf04ba888/include/google/protobuf/unknown_field_set.h:214:13: warning: anonymous types declared in an anonymous union are an extension [-Wnested-anon-types] mutable union { ^ build/proto/ProtoHeaderWrap.cc:47:24: error: non-constant-expression cannot be narrowed from type 'unsigned char' to 'char' in initializer list [-Wc++11-narrowing] std::string msgBuf{phSize}; ^~~~~~ build/proto/ProtoHeaderWrap.cc:47:24: note: insert an explicit cast to silence this issue std::string msgBuf{phSize}; ^~~~~~ static_cast<char>( ) 1 warning and 1 error generated. I thought it was a bit silly to cast the thing we just created that's only used once. I'm happy to change it though. Regarding the comment on boost::format , I had seen other places in the code that used .cstr() so assumed that was idiomatic. I'll sort that out as well.
            Hide
            fritzm Fritz Mueller added a comment - - edited

            Ah, I missed that the error was coming from the {} init on the next line! Yes, narrowing conversions in {} inits are explicitly verboten... Your change as written is fine (or consider inlining the cast directly within the {} init, and losing the phSize local, if you like?)

            boost::str vs. boost::format::str() is also fine either way – if there are .cstr() littered about and you feel it is more consistent to use .str() then go with that.

            Moving to "reviewed" – thanks!

            Show
            fritzm Fritz Mueller added a comment - - edited Ah, I missed that the error was coming from the {} init on the next line! Yes, narrowing conversions in {} inits are explicitly verboten... Your change as written is fine (or consider inlining the cast directly within the {} init, and losing the phSize local, if you like?) boost::str vs. boost::format::str() is also fine either way – if there are .cstr() littered about and you feel it is more consistent to use .str() then go with that. Moving to "reviewed" – thanks!
            Hide
            tjenness Tim Jenness added a comment -

            I've fixed all the problems from the review and pushed a new version of the branch. Do you want to run it through an integration test or shall I just merge it?

            Show
            tjenness Tim Jenness added a comment - I've fixed all the problems from the review and pushed a new version of the branch. Do you want to run it through an integration test or shall I just merge it?
            Hide
            fritzm Fritz Mueller added a comment -

            Go for it!

            Show
            fritzm Fritz Mueller added a comment - Go for it!
            Hide
            tjenness Tim Jenness added a comment -

            Merged as requested.

            Show
            tjenness Tim Jenness added a comment - Merged as requested.

              People

              • Assignee:
                tjenness Tim Jenness
                Reporter:
                tjenness Tim Jenness
                Reviewers:
                Fritz Mueller
                Watchers:
                Andy Salnikov, Fritz Mueller, Jacek Becla, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel