Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-16663

options requested in top-level SConstruct to support non-default libraries and includes

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major - P3
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0-rc6
    • Component/s: Build
    • Labels:
      None
    • Backwards Compatibility:
      Fully Compatible
    • Sprint:
      BUILD 1

      Description

      I have a small diff but I am waiting for legal to let me know whether I can share that so I will describe the change. I am building MongoDB with a compiler and library toolchain that works "in production" so I don't want to use the default locations for libraries and include files. I ended up needing two options in the top-level SConstruct to make this work:

      1) --dont-add-local-paths - this changes MongoDB to not add local paths. For example, I want to keep this from being added for 64-bit linux:
      env.Append( EXTRALIBPATH=["/usr/lib64" , "/lib64" ] )

      2) --linkflags - values here get appended to LINKFLAGS. I used this to add extra flags and libraries to link with jemalloc. I ended up using ":" as the option separator because options already use "," and the following is an example of a value I used:
      --linkflags="-Wl,--whole-archive /path/to/libjemalloc.a:-Wl,--no-whole-archive"

      I can also use this to set rpath options, although I don't need to do that yet we do that for other binaries like mysqld...

      --linkflags="--enable-new-dtags:-Wl,-rpath=/path/to/production/lib"

        Issue Links

          Activity

          Hide
          acm Andrew Morrow added a comment - - edited

          Hi Mark -

          Thanks for the bug report. A few comments.

          • RE local paths: There is suspicion, but not yet proof, that those path entries are vestigial at best. They can be actively harmful in some cases, as you point out. If you are using a properly configured toolchain, the correct default library search paths should be baked into the toolchain, and the SConstruct should not be in the business of guessing about the relationship between current platform and library or header search paths. Adding the flag you propose would be useful as we could then easily test whether builds with --dont-add-local-paths worked correctly on our build systems of record. If so, the flag could just be removed along with the now-known-to-be-useless path adornments.
          • RE extra link flags: This has been a frequent internal request. Ideally this would be done not only for link flags but also for all of CFLAGS, CCFLAGS, CXXFLAGS, LINKFLAGS, and the "SH" prefixed version of those. One subtlety to consider is whether the user flags should come before flags added in the SConstruct itself, in which case there is a risk that the user specified flags are overridden, or after, which might subvert the aim of setting them (e.g. can't pass configure checks without the flags, but we don't add them until later). As a result I suspect that we actually need a user to be able to set both CXXFLAGS (set at Environment construction time) and EXTRA_CXXFLAGS (set after Configure), for instance. So, this becomes a lot of flags to add. As you note, there are also some complications regarding delimiters, quoting, spaces in paths, etc., for which there is not yet a clearly optimal solution. Finally, SCons offers a 'Variables' facility, which would make it possible to specify user flag customizations in a potentially more natural format: "scons CCFLAGS=-fno-omit-frame-pointer" for instance. But this mechanism has its own issues with how to specify multiple flags, spaces in names, etc.

          Overall, there is agreement that improvements along these lines would be beneficial.

          Show
          acm Andrew Morrow added a comment - - edited Hi Mark - Thanks for the bug report. A few comments. RE local paths: There is suspicion, but not yet proof, that those path entries are vestigial at best. They can be actively harmful in some cases, as you point out. If you are using a properly configured toolchain, the correct default library search paths should be baked into the toolchain, and the SConstruct should not be in the business of guessing about the relationship between current platform and library or header search paths. Adding the flag you propose would be useful as we could then easily test whether builds with --dont-add-local-paths worked correctly on our build systems of record. If so, the flag could just be removed along with the now-known-to-be-useless path adornments. RE extra link flags: This has been a frequent internal request. Ideally this would be done not only for link flags but also for all of CFLAGS, CCFLAGS, CXXFLAGS, LINKFLAGS, and the "SH" prefixed version of those. One subtlety to consider is whether the user flags should come before flags added in the SConstruct itself, in which case there is a risk that the user specified flags are overridden, or after, which might subvert the aim of setting them (e.g. can't pass configure checks without the flags, but we don't add them until later). As a result I suspect that we actually need a user to be able to set both CXXFLAGS (set at Environment construction time) and EXTRA_CXXFLAGS (set after Configure), for instance. So, this becomes a lot of flags to add. As you note, there are also some complications regarding delimiters, quoting, spaces in paths, etc., for which there is not yet a clearly optimal solution. Finally, SCons offers a 'Variables' facility, which would make it possible to specify user flag customizations in a potentially more natural format: "scons CCFLAGS=-fno-omit-frame-pointer" for instance. But this mechanism has its own issues with how to specify multiple flags, spaces in names, etc. Overall, there is agreement that improvements along these lines would be beneficial.
          Hide
          mdcallag Mark Callaghan added a comment -

          I had another problem and my workaround is to change --extrapath so that it only sets paths for libraries and then added --extraincpath to do the same for include paths.

          I use gcc 4.8.1 and with non-standard library paths have been using --extrapath to find them, but that also added include paths and there was a compile error courtesy of some (questionable) code in src/mongo/crypto/tom/tomcrypt_custom.h that declares functions that libc provides leading to a compile error when the declarations did not match.

          <noformat>
          /* macros for various libc functions you can change for embedded targets */
          #ifndef XMALLOC
          #ifdef malloc
          #define LTC_NO_PROTOTYPES
          #endif
          #define XMALLOC malloc
          #endif
          <noformat>

          Which resulted in ...
          <noformat>

          1. 1 "src/mongo/crypto/tom/tomcrypt_cfg.h" 1
          2. 41 "src/mongo/crypto/tom/tomcrypt_cfg.h"
            void * malloc(size_t n);
            void * realloc(void *p, size_t n);
            void * calloc(size_t n, size_t s);
            void free(void *p);

          void qsor(void *base, size_t nmemb, size_t size, int(*compar)(const void *, const void *));

          clock_t clock(void);

          void * memcpy(void *dest, const void *src, size_t n);
          int memcmp(const void *s1, const void *s2, size_t n);
          void * memse(void *s, int c, size_t n);

          int strcmp(const char *s1, const char *s2);
          <noformat>

          Show
          mdcallag Mark Callaghan added a comment - I had another problem and my workaround is to change --extrapath so that it only sets paths for libraries and then added --extraincpath to do the same for include paths. I use gcc 4.8.1 and with non-standard library paths have been using --extrapath to find them, but that also added include paths and there was a compile error courtesy of some (questionable) code in src/mongo/crypto/tom/tomcrypt_custom.h that declares functions that libc provides leading to a compile error when the declarations did not match. <noformat> /* macros for various libc functions you can change for embedded targets */ #ifndef XMALLOC #ifdef malloc #define LTC_NO_PROTOTYPES #endif #define XMALLOC malloc #endif <noformat> Which resulted in ... <noformat> 1 "src/mongo/crypto/tom/tomcrypt_cfg.h" 1 41 "src/mongo/crypto/tom/tomcrypt_cfg.h" void * malloc(size_t n); void * realloc(void *p, size_t n); void * calloc(size_t n, size_t s); void free(void *p); void qsor(void *base, size_t nmemb, size_t size, int(*compar)(const void *, const void *)); clock_t clock(void); void * memcpy(void *dest, const void *src, size_t n); int memcmp(const void *s1, const void *s2, size_t n); void * memse(void *s, int c, size_t n); int strcmp(const char *s1, const char *s2); <noformat>
          Hide
          acm Andrew Morrow added a comment -

          Hi Mark -

          If we move to allowing command line overrides of things like CFLAGS, SHLINKFLAGS, etc., then I think it would be natural to extend that syntax to allow setting CPPPATH and LIBPATH as well, and probably deprecate or remove the cpppath, libpath, and extrapath options, which have less than ideal semantics at present.

          Thanks,
          Andrew

          Show
          acm Andrew Morrow added a comment - Hi Mark - If we move to allowing command line overrides of things like CFLAGS, SHLINKFLAGS, etc., then I think it would be natural to extend that syntax to allow setting CPPPATH and LIBPATH as well, and probably deprecate or remove the cpppath, libpath, and extrapath options, which have less than ideal semantics at present. Thanks, Andrew
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'jbreams', u'name': u'Jonathan Reams', u'email': u'jbreams@mongodb.com'}

          Message: SERVER-16663 Add variables to set CFLAGS et all on command-line
          Branch: master
          https://github.com/mongodb/mongo/commit/1b72a40d26b43b64a6d08717e2a01a31e6e861a8

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'jbreams', u'name': u'Jonathan Reams', u'email': u'jbreams@mongodb.com'} Message: SERVER-16663 Add variables to set CFLAGS et all on command-line Branch: master https://github.com/mongodb/mongo/commit/1b72a40d26b43b64a6d08717e2a01a31e6e861a8
          Hide
          jonathan.reams Jonathan Reams added a comment -

          We added scons variables to set the initial values of ARFLAGS, CCFLAGS, CFLAGS, CPPDEFINES CPPPATH, CXXFLAGS, LIBPATH, LIBS, LINKFLAGS, SHCCFLAGS, SHCFLAGS, SHCXXFLAGS, and SHLINKFLAGS on the command line. Different flags are separated by a space, as though you were setting the corresponding shell environment variable.

          scons CCFLAGS="-fno-omit-frame-pointer mongod -O4" all

          Show
          jonathan.reams Jonathan Reams added a comment - We added scons variables to set the initial values of ARFLAGS, CCFLAGS, CFLAGS, CPPDEFINES CPPPATH, CXXFLAGS, LIBPATH, LIBS, LINKFLAGS, SHCCFLAGS, SHCFLAGS, SHCXXFLAGS, and SHLINKFLAGS on the command line. Different flags are separated by a space, as though you were setting the corresponding shell environment variable. scons CCFLAGS="-fno-omit-frame-pointer mongod -O4" all
          Hide
          mdcallag Mark Callaghan added a comment -

          Can you add RPATH too?

          CPPPATH worked for me but I get errors from LINKFLAGS for:
          scons LINKFLAGS="-enable-new-dtags -Wl,-whole-archive /foobar/libjemalloc.a" ...

          Warning: the -W option is not yet implemented
          ESC[?1034hscons: Reading SConscript files ...
          Mkdir("build/scons")
          scons version: 2.3.0
          python version: 2 6 6 'final' 0

          scons: *** Error converting option: LINKFLAGS
          No closing quotation

          In MySQL 5.6 with cmake we do stuff like:
          MYSQLD_LDFLAGS+=" Wl,-no-whole-archive $LIB/libunwind.a"
          LDFLAGS+=" --enable-new-dtags -Wl,-rpath=/foobar/$BUILD_ENV/lib"

          $

          {CMAKE}

          $

          {CMAKE_FLAGS}

          -DWITH_MYSQLD_LDFLAGS="$MYSQLD_LDFLAGS"

          Show
          mdcallag Mark Callaghan added a comment - Can you add RPATH too? CPPPATH worked for me but I get errors from LINKFLAGS for: scons LINKFLAGS="- enable-new-dtags -Wl, -whole-archive /foobar/libjemalloc.a" ... Warning: the -W option is not yet implemented ESC[?1034hscons: Reading SConscript files ... Mkdir("build/scons") scons version: 2.3.0 python version: 2 6 6 'final' 0 scons: *** Error converting option: LINKFLAGS No closing quotation — In MySQL 5.6 with cmake we do stuff like: MYSQLD_LDFLAGS+=" Wl, -no-whole-archive $LIB/libunwind.a" LDFLAGS+=" --enable-new-dtags -Wl,-rpath=/foobar/$BUILD_ENV/lib" $ {CMAKE} $ {CMAKE_FLAGS} -DWITH_MYSQLD_LDFLAGS="$MYSQLD_LDFLAGS"
          Hide
          mdcallag Mark Callaghan added a comment -

          It works, but I still would like for SConstruct to support RPATH.

          I need to read up on shell quoting. I have a shell script that calls scons and using this in the shell script does work...
          scons LINKFLAGS="--enable-new-dtags -Wl,-whole-archive /foobar/libjemalloc.a ..."
          scons LINKFLAGS='--enable-new-dtags -Wl,-whole-archive /foobar/libjemalloc.a ...'

          But this works in the shell script
          scons LINKFLAGS="\"--enable-new-dtags -Wl,-whole-archive /foobar/libjemalloc.a ...\""

          Show
          mdcallag Mark Callaghan added a comment - It works, but I still would like for SConstruct to support RPATH. I need to read up on shell quoting. I have a shell script that calls scons and using this in the shell script does work... scons LINKFLAGS="--enable-new-dtags -Wl,-whole-archive /foobar/libjemalloc.a ..." scons LINKFLAGS='--enable-new-dtags -Wl,-whole-archive /foobar/libjemalloc.a ...' But this works in the shell script scons LINKFLAGS="\"--enable-new-dtags -Wl,-whole-archive /foobar/libjemalloc.a ...\""
          Hide
          acm Andrew Morrow added a comment -

          Hi Mark Callaghan -

          Re the quoting, the single quotes worked fine for me for passing multiple arguments in via LINKFLAGS, both from the command line and from a shell script, at least on OS X.

          For the SCons RPATH variable: I agree we should have this. I just found a need for it myself and had been planning to add it. Would you mind filing a new ticket though? I think it is likely we would want to backport to 3.0 and the legacy C++ driver, and it would be easier to track that workflow under a new ticket.

          Show
          acm Andrew Morrow added a comment - Hi Mark Callaghan - Re the quoting, the single quotes worked fine for me for passing multiple arguments in via LINKFLAGS, both from the command line and from a shell script, at least on OS X. For the SCons RPATH variable: I agree we should have this. I just found a need for it myself and had been planning to add it. Would you mind filing a new ticket though? I think it is likely we would want to backport to 3.0 and the legacy C++ driver, and it would be easier to track that workflow under a new ticket.
          Hide
          mdcallag Mark Callaghan added a comment -

          For RPATH https://jira.mongodb.org/browse/SERVER-17694

          I still haven't figured out the right way to quote it. If my shell script echoes the command line, I can copy/paste it to run scons. But if my shell script invokes scons directly, then it isn't working yet. My shell quoting knowledge is weak so I will ask a local expert.

          Show
          mdcallag Mark Callaghan added a comment - For RPATH https://jira.mongodb.org/browse/SERVER-17694 I still haven't figured out the right way to quote it. If my shell script echoes the command line, I can copy/paste it to run scons. But if my shell script invokes scons directly, then it isn't working yet. My shell quoting knowledge is weak so I will ask a local expert.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                  Agile