maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Vladislav!

On Mar 12, Vladislav Vaintroub wrote:
> >> diff --git a/plugin/aws_key_management/CMakeLists.txt b/plugin/aws_key_management/CMakeLists.txt
> >> new file mode 100644
> >> index 0000000..ec9ee46
> >> --- /dev/null
> >> +++ b/plugin/aws_key_management/CMakeLists.txt
> >> @@ -0,0 +1,144 @@
> >> +
> >> +# Give message why the building this plugin is skipped (only if -DVERBOSE is defined)
> >> +# and and bail out.
> >> +MACRO(SKIP_AWS_PLUGIN msg)
> >> +  IF(DEFINED VERBOSE)
> > VERBOSE - is it used somewhere else? Or you've invented it on the spot
> > for this plugin?
> > I understand that it can be used in other cmake files, but is it?
> It is not used elsewhere. invented on the spot.  It can be used 
> elsewhere, if we decide to make it a common convention.

That was the point, exactly. Let's make it a common convention and use
in the future as necessary.

> >> +SET(CXX11_FLAGS)
> >> +SET(OLD_COMPILER_MSG "AWS SDK requires c++11 -capable compiler (minimal supported versions are g++ 4.7, clang 3.3, VS2103)")
> >> +IF(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
> >> +  EXECUTE_PROCESS(COMMAND ${CMAKE_CXX_COMPILER} -dumpversion OUTPUT_VARIABLE GCC_VERSION)
> > eh? I thought cmake knows compiler versions... let me see
> > This one: CMAKE_<LANG>_COMPILER_VERSION
> I did not change this code, I gave other options a fair try, but no avail.
> 1.  CMAKE_<LANG>_COMPILER_VERSION one exists since CMake 2.8.10, and we 
> use older versions on many machines.  Can't really use that, if we stick 
> to native cmakes that come with the distros.

Hmm. We use CMAKE_C_COMPILER_VERSION in quite a few cmake files.
I wonder how does it work now...

May be in conditions like

   IF(CMAKE_C_COMPILER_VERSION VERSION_LESS "4.6")

it doesn't matter whether CMAKE_C_COMPILER_VERSION is defined? If it's
undefined the condition is false, and it's good enough for us?

> 2.  gcc4.7 does not really work ((I on some reason thought it would , 
> but rechecked), even if --std=c++11  . Level of  C++11 standard in this 
> compiler is still  insufficient (missing 
> std::is_trivially_destructible() at least), and also Amazon is using 
> compile options not available with 4.7 (-Wno-<something>)

yes, that was clear. it was not surprising that AWS SDK needs a
reasonably new gcc (as they also require cmake 3.1.2).

> > 3. Is that safe at all - you pull the latest version from git, they can
> >     start using cmake-3.1.2 features any time.
> Ok, I'm no longer use the latest version - I use the latest and the only 
> one marked with a tag 0.9.6 . Using latest is not a  good idea anyway,  
> if we want a reproducible build.

Indeed. Good idea!

> >> +    printf("%s", statement.c_str());
> > printf? You have declared sql_print_information above, why wouldn't you
> > use it here?
> The statement that is being printed  is already formatted, it contains a 
> longish prefix containing tag (like [INFO], [DEBUG], [WARNING]) a 
> timestamp, a subcomponent name . sql_print_information would add 
> duplicate timestamp, another INFO tag, and not really added value, but 
> it will make it look weirder-

I see. On the other hand, it's also weird when some log lines don't
match the standard log line pattern. But ok, let's keep it your way and
see how the log will look like...

Regards,
Sergei