-
Notifications
You must be signed in to change notification settings - Fork 23
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
Fix build failure and automake support #11
base: master
Are you sure you want to change the base?
Conversation
@@ -71,7 +71,7 @@ | |||
|
|||
#include <algorithm> | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linking of the test cases and memcached files was running into the error below. I had renamed a couple methods that contained the same signature causing multiple definition errors.
CXXLD tests/libmemcached-1.0/testapp tests/libmemcached-1.0/tests_libmemcached_1_0_testapp-dynamic_mode_test.o: In function
get_world':
/usr/include/c++/4.8/sstream:129: multiple definition of get_world' tests/libmemcached-1.0/tests_libmemcached_1_0_testapp-all_tests.o:/home/ubuntu/new/aws-elasticache-cluster-client-libmemcached/BUILD/../tests/libmemcached-1.0/all_tests.cc:75: first defined here tests/libmemcached-1.0/tests_libmemcached_1_0_testapp-dynamic_mode_test.o:(.data.rel+0x0): multiple definition of
collection'
tests/libmemcached-1.0/tests_libmemcached_1_0_testapp-all_tests.o:(.data.rel+0x0): first defined here
/usr/bin/ld: Warning: size of symbol collection' changed from 2272 in tests/libmemcached-1.0/tests_libmemcached_1_0_testapp-all_tests.o to 64 in tests/libmemcached-1.0/tests_libmemcached_1_0_testapp-dynamic_mode_test.o collect2: error: ld returned 1 exit status
@@ -9,6 +9,5 @@ nobase_include_HEADERS+= libmemcached-1.0/struct/memcached.h | |||
nobase_include_HEADERS+= libmemcached-1.0/struct/result.h | |||
nobase_include_HEADERS+= libmemcached-1.0/struct/sasl.h | |||
nobase_include_HEADERS+= libmemcached-1.0/struct/server.h | |||
nobase_include_HEADERS+= libmemcached-1.0/struct/configuration_server.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dead code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified the automake include files to the memcached dependencies required for the test cases during compilation/linking
@@ -24,8 +24,7 @@ noinst_HEADERS+= libmemcached/continuum.hpp | |||
noinst_HEADERS+= libmemcached/do.hpp | |||
noinst_HEADERS+= libmemcached/encoding_key.h | |||
noinst_HEADERS+= libmemcached/error.hpp | |||
noinst_HEADERS+= libmemcached/flag.hpp | |||
noinst_HEADERS+= libmemcached/host.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dead code
@@ -192,6 +193,7 @@ $(TMP_DIR): | |||
@$(mkdir_p) $(TMP_DIR) | |||
|
|||
libtest_unittest_LDADD+= libtest/libtest.la | |||
libtest_unittest_LDADD+= $(libmemcached_libmemcached_la_OBJECTS) | |||
libtest_unittest_SOURCES= libtest/unittest.cc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified the automake include files to fix compiling/linking errors when referencing memcached functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed files that will be regenerated during the autoreconf process. For example "configure" & "Makefile.in"
Hi Kosty, |
These are not needed
These are not needed
I'm trying to use your PR, to create a docker image to run in ECR.
The configure gives error:
Checking the build-aux dir (created by autoreconf):
|
@raphaelquati you can symlink shtool into the dir you extracted the tar |
#9 #8 #7
@kmaleyev I created a new pull request to more clearly define the changes and incorporate your feedback. I have tested the build on Ubuntu 14 and Amazon Linux 2017.03
I organized this PR more cleanly and discarded some changes after your clarification and feedback. I tried squashing PR #10 but it got really ugly and wanted to be clear on the changes.
Let me know your thoughts, thanks in advance.