Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CONC-430] Compilation on AIX #124

Merged
merged 5 commits into from Aug 13, 2020
Merged

[CONC-430] Compilation on AIX #124

merged 5 commits into from Aug 13, 2020

Conversation

EGuesnet
Copy link
Contributor

This PR improves the behavior of mariadb-conector-c on AIX. Without, it does not compile.

Mariadb-connector-c was tested with mariadb-server (patch not submitted yet); 20 fail on 3640 tests on debug mode, 5 fail on 3339 tests on release mode.

Code compiled with GCC 8.3 (both 32 and 64 bits) on AIX 6.1 and with CMake 3.16.0.

Issue associated: https://jira.mariadb.org/browse/CONC-430.


Comment about commit:

Some specific code are no more needed. It breaks the compilation.
Some typedef are not consistant. They are defined in another way on /usr/include/sys/inttypes.h. With "signed", typedef is consistant with system and with comments.
Definition of MSG_DONTWAIT is made on libmariadb/mariadb_async.c for AIX. It is not enought, I add also it on plugins/pvio/pvio_socket.c file.

Thanks.

@EGuesnet EGuesnet changed the title Compilation on AIX [CONC-430] Compilation on AIX Jan 10, 2020
@EGuesnet
Copy link
Contributor Author

With this PR, mariadb-connector-c will compile and work on AIX. It is only AIX-specific code, and a correction of compatibility typedef.

Can someone have a look?

@grooverdan
Copy link
Contributor

Looks good too me. All either AIX contained or explicitly signing char is essential as the C standard doesn't define it (previous similar fix). signed short seems the default and if its already in the header its a good fix to be consistent.

@EGuesnet
Copy link
Contributor Author

Hi,
Anything new here? Does something block this PR?

@grooverdan
Copy link
Contributor

@9E0R9 are you ok with this?

@9EOR9 9EOR9 closed this Aug 13, 2020
@9EOR9 9EOR9 reopened this Aug 13, 2020
@9EOR9 9EOR9 merged commit 49be7b2 into mariadb-corporation:3.1 Aug 13, 2020
@@ -52,6 +52,10 @@ CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/libmariadb.pc.in

ADD_EXECUTABLE(mariadb_config ${CMAKE_CURRENT_BINARY_DIR}/mariadb_config.c)

IF(CMAKE_SYSTEM_NAME MATCHES AIX)
TARGET_LINK_LIBRARIES(mariadb_config compat-getopt)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EGuesnet what provides the compat-getopt library?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getopt_long() function is not available on AIX by default. Opensource teams on AIX have created compatibility libraries to provide some function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is it available?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lib, and devel package. They are not available yet on AIX Toolbox for Linux Applications.

ADD_DEFINITIONS(-DLIBICONV_PLUG)
IF(NOT CMAKE_SYSTEM_NAME MATCHES AIX)
ADD_DEFINITIONS(-DLIBICONV_PLUG)
ENDIF()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iconv related, but not this change:

I got link errors using the 3.1 branch as updated in libmariadb under mariadb-server

Scanning dependencies of target libmariadb
[ 12%] Linking C shared library libmariadb
ld: 0711-317 ERROR: Undefined symbol: .libiconv_open
ld: 0711-317 ERROR: Undefined symbol: .libiconv
ld: 0711-317 ERROR: Undefined symbol: .libiconv_close
ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.

It turns out /usr/lib/libiconv.a doesn't define these symbols, /usr/linux/lib/libiconv.a does.

I used the ugly hack of

--- a/cmake/FindIconv.cmake
+++ b/cmake/FindIconv.cmake
@@ -28,6 +28,9 @@ ELSEIF(APPLE)
                /usr/lib/
                NO_CMAKE_SYSTEM_PATH)
     SET(ICONV_EXTERNAL TRUE)
+ELSEIF(CMAKE_SYSTEM_NAME MATCHES "AIX")
+  SET(ICONV_LIBRARIES -liconv)
+  SET(ICONV_EXTERNAL TRUE)
 ELSE()
   find_library(ICONV_LIBRARIES NAMES iconv libiconv libiconv-2)
   IF(ICONV_LIBRARIES)

To work around this. Do you have any ideas on making this more robust?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually use libraries built by opensource teams (in /usr/linux/lib or /opt/freeware/lib), even if they are available on base system (except ssl and crypto). So, I have not faced this issue. Your hack seems to be compatible with both version of libiconv. I do not have idea to make it more robust.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Highlighting it has made a push to remove it as a dependency anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants