Skip to content

Bugfix. Inconsistent behaviour in runtime#293

Closed
drmateo wants to merge 3 commits intoplasmodic:masterfrom
drmateo:plasmodic
Closed

Bugfix. Inconsistent behaviour in runtime#293
drmateo wants to merge 3 commits intoplasmodic:masterfrom
drmateo:plasmodic

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