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

Bugfix. Inconsistent behaviour in runtime #293

Closed
wants to merge 3 commits into from

Conversation

drmateo
Copy link
Contributor

@drmateo drmateo commented Dec 23, 2016

This pull request refers to previous attempts #291 and #292.

@stonier
Copy link
Contributor

stonier commented Dec 25, 2016

+1 for bugfixing the return code on process(). What's the logic for dropping the plasm_loader dependency?

@drmateo
Copy link
Contributor Author

drmateo commented Dec 25, 2016

Remove the plasm_loader dependency is not actually necessary because that change was just an attempt to resolve a problem in compilation time when you try to build the library tests using boost 1.62.

But I've detected the problem is situated in the TEST(SerialTest, Plasm) in the file named test/cpp/serialization.cpp .

My workaround solution (now I don't have time to find another better solution) is just avoid this test to be able to build and run the remaining tests. And as it is not necessary remove plasm_loader dependency, I do a rollback in the file test/scripts/CMakeLists.txt.

  • The error message of the problem throw in compilation time is:
    Undefined symbols for architecture x86_64: "void ecto::plasm::load<boost::archive::binary_iarchive>(boost::archive::binary_iarchive&, unsigned int)", referenced from: void boost::serialization::access::member_load<boost::archive::binary_iarchive, ecto::plasm>(boost::archive::binary_iarchive&, ecto::plasm&, unsigned int) in serialization.cpp.o "void ecto::plasm::save<boost::archive::binary_oarchive>(boost::archive::binary_oarchive&, unsigned int) const", referenced from: void boost::serialization::access::member_save<boost::archive::binary_oarchive, ecto::plasm const>(boost::archive::binary_oarchive&, ecto::plasm const&, unsigned int) in serialization.cpp.o

@stonier
Copy link
Contributor

stonier commented Dec 26, 2016

Two separate things here then. Each PR should cover a single problem or related set of problems.

The missing return bugfix gets a +1 from me to go in immediately. Can you make a PR covering that one exactly?

The latter workaround I'm not comfortable in just dropping from the build just because it doesn't compile on an as yet unsupported dependency. In my experience, these things never come back to be checked later if just hidden. I've fired up an issue in #294 where we can talk about it. @vrabaud - thoughts?

@drmateo
Copy link
Contributor Author

drmateo commented Dec 26, 2016

The missing return bugfix gets a +1 from me to go in immediately. Can you make a PR covering that one exactly?

+1. I'm going to create a new PR just covering that problem

The latter workaround I'm not comfortable in just dropping from the build just because it doesn't compile on an as yet unsupported dependency. In my experience, these things never come back to be checked later if just hidden. I've fired up an issue in #294 where we can talk about it. @vrabaud - thoughts?

I'll continue handle this test problem in the issue #294 .

@drmateo
Copy link
Contributor Author

drmateo commented Dec 26, 2016

The issue number related with the new PR (covering just the missing return problem) is #295

@stonier
Copy link
Contributor

stonier commented Dec 27, 2016

Closing this here then - @vrabaud if you tune in, we are moving discussion to issue #294.

@stonier stonier closed this Dec 27, 2016
@stonier stonier self-assigned this Dec 27, 2016
@stonier stonier added the bug label Dec 27, 2016
@drmateo drmateo deleted the plasmodic branch December 31, 2016 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants