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

V1.0.3 10.1 #3

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

V1.0.3 10.1 #3

wants to merge 10 commits into from

Conversation

davidl71
Copy link

@davidl71 davidl71 commented May 5, 2020

Allow db2iengine to compile on IBMiV7R2 using gcc 6.3 from IBM yum repository against mariadb 10.1. This pull request should be used for reference and preferably not merged into master but onto a separate branch.

@ThePrez
Copy link
Collaborator

ThePrez commented Jun 22, 2020

@abmusse , can you please look these over?

Copy link
Collaborator

@abmusse abmusse left a comment

Choose a reason for hiding this comment

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

@ThePrez ATM I don't see much value added from merging these changes.

@ThePrez ThePrez self-requested a review June 29, 2020 21:29
Copy link
Collaborator

@ThePrez ThePrez left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! Please respond to the few questions I have in the review, and we'll make sure to pull in the valuable pieces with the moving master branch.

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -malign-power -malign-natural")
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -malign-power -malign-natural")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DMARIA_PLUGIN_INTERFACE_VERSION=536875296 -malign-power -malign-natural -DDBUG_OFF -mlong-double-128")
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DMARIA_PLUGIN_INTERFACE_VERSION=536875296 -malign-power -malign-natural -DDBUG_OFF -mlong-double-128")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@davidl71, what's the magic you used to determine -DMARIA_PLUGIN_INTERFACE_VERSION=536875296?

I believe the other changes to this line are no longer needed due to other source code changes in the master branch

Fixup for gcc 6.3 from IBM yum repo
cd storage/ibmdb2i/special
cp /usr/include/unistd.h
cp /QIBM/include/qmyse.h
Copy link
Collaborator

Choose a reason for hiding this comment

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

@abmusse , do you know why these steps aren't required for your builds you've done?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks there is qmyse.h in special/include already

https://github.com/zendtech/db2iengine/blob/master/special/include/qmyse.h

We prepend the special/include to our include directories here:

https://github.com/zendtech/db2iengine/blob/master/CMakeLists.txt#L3

https://cmake.org/cmake/help/v3.7/command/include_directories.html#command:include_directories

I'm not sure why /usr/include/unistd.h is needed though. @davidl-zend Can you chime in on this?

@@ -3859,10 +3842,10 @@ maria_declare_plugin(ibmdb2i)
PLUGIN_LICENSE_GPL,
ibmdb2i_init_func,
ibmdb2i_done_func,
0x0100,
0x0102,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@davidl71, please explain this change. We might need this!

@@ -61,6 +61,24 @@ OF SUCH DAMAGE.
#include "db2i_errors.h"
#include "db2i_sqlStatementStream.h"

//David Lowes - Temporarily exclude mysql/plugin.h and manually define used constants
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any idea why you needed to do this?
IBM has been building against 10.3, so curious if something changed between mariadb versions

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

Successfully merging this pull request may close these issues.

3 participants