maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Wlad!

Mostly ok, please see below

On Nov 03, wlad@xxxxxxxxxxx wrote:
> revision-id: 7931ca4f89ad5cd38ea3dc54f1363b7de2fd3414 (mariadb-10.0.22-5-g7931ca4)
> parent(s): fa1438cbf4307731a54ea4137d5f7d4b744cdfbc
> committer: Vladislav Vaintroub
> timestamp: 2015-11-03 11:32:00 +0100
> message:
> 
> MDEV-9024 Build fails with VS2015
> 
> Fix build failures caused by new C runtime library
> - isnan, snprintf, struct timespec are now defined, attempt to redefine them leads
> - P_tmpdir, tzname are no more defined
> -  lfind() and lsearch() in lf_hash.c had to be renamed,  declaration
> conflicts with some C runtime functions with the same name declared
> in a header included by stdlib.h
> 
>  Also fix couple of annoying warnings :
> - remove #define NOMINMAX from config.h to avoid "redefined" compiler
> warnings(NOMINMAX is already in compile flags)
> 
> - disable incremental linker in Debug as well (feature not used much
> and compiler crashes often)
> 
> 
> Also simplify package building with Wix, require Wix 3.9 or later
> (VS2015 is not compatible with old Wix 3.5/3.6)
> 
> diff --git a/cmake/os/Windows.cmake b/cmake/os/Windows.cmake
> index 4c01bab..fd6e739 100644
> --- a/cmake/os/Windows.cmake
> +++ b/cmake/os/Windows.cmake
> @@ -181,14 +184,13 @@ CHECK_SYMBOL_REPLACEMENT(S_IROTH _S_IREAD sys/stat.h)
>  CHECK_SYMBOL_REPLACEMENT(S_IFIFO _S_IFIFO sys/stat.h)
>  CHECK_SYMBOL_REPLACEMENT(SIGQUIT SIGTERM signal.h)
>  CHECK_SYMBOL_REPLACEMENT(SIGPIPE SIGINT signal.h)
> -CHECK_SYMBOL_REPLACEMENT(isnan _isnan float.h)

How can this be wrong? It checks whetherone should use _isnan instead of
isnan. That seems like the kind of check that works both with old and new VS.

>  CHECK_SYMBOL_REPLACEMENT(finite _finite float.h)
> diff --git a/config.h.cmake b/config.h.cmake
> index 46eed79..a1d505f 100644
> --- a/config.h.cmake
> +++ b/config.h.cmake
>  
> -/* We don't want the min/max macros */
> -#ifdef __WIN__
> -#define NOMINMAX
> -#endif

I'd generally prefer to define symbols in headers, instead of putting
everything into ever-growing compiler command line

Besides, the minimal fix for the redefinition warning is

 -#define NOMINMAX
 +#define NOMINMAX 1

because -DNOMINMAX defines NOMINMAX to be 1, not empty.

>  /*
>    MySQL features
>  */
> diff --git a/mysys/lf_hash.c b/mysys/lf_hash.c
> index aa96ca9..fa92971 100644
> --- a/mysys/lf_hash.c
> +++ b/mysys/lf_hash.c
> @@ -43,7 +43,7 @@ const int LF_HASH_OVERHEAD= sizeof(LF_SLIST);
>  
>  /*
>    a structure to pass the context (pointers two the three successive elements
> -  in a list) from lfind to linsert/ldelete
> +  in a list) from my_lfind to my_linsert/my_ldelete
>  */
>  typedef struct {
>    intptr volatile *prev;
> @@ -70,7 +70,7 @@ typedef struct {
>      cursor is positioned in either case
>      pins[0..2] are used, they are NOT removed on return
>  */
> -static int lfind(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr,
> +static int my_lfind(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr,

er... I don't like that. lfind/lsearch are internal static functions in
lf_hash.c, they shouldn't need or use my_ prefix.

Two alternatives:

1.
  #define lfind remap_lfind_for_win
  #include <my_global.h>
  #include <my_sys.h>
  #undef lfind

2.

 -static int lfind(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr,
 +static int l_find(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr,

>                   const uchar *key, uint keylen, CURSOR *cursor, LF_PINS *pins)
>  {
>    uint32       cur_hashnr;
> diff --git a/win/packaging/CMakeLists.txt b/win/packaging/CMakeLists.txt
> index 3b9b72f..0535a48 100644
> --- a/win/packaging/CMakeLists.txt
> +++ b/win/packaging/CMakeLists.txt
>   SET(CPACK_WIX_UPGRADE_CODE "49EB7A6A-1CEF-4A1E-9E89-B9A4993963E3")
> - SET(CPACK_WIX_PACKAGE_NAME "MariaDB @MAJOR_VERSION@.@MINOR_VERSION@")
> + SET(CPACK_WIX_PACKAGE_NAME "MariaDB ${MAJOR_VERSION}.${MINOR_VERSION}")
>  ELSE()
>   SET(CPACK_WIX_UPGRADE_CODE "2331E7BD-EE58-431B-9E18-B2B918BCEB1B")
> - SET(CPACK_WIX_PACKAGE_NAME "MariaDB @MAJOR_VERSION@.@MINOR_VERSION@ (x64)")
> + SET(CPACK_WIX_PACKAGE_NAME "MariaDB ${MAJOR_VERSION}.${MINOR_VERSION} (x64)")

Weird. I was sure I've fixed this months ago :-[]

Regards,
Sergei