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

Pybind11/fixiling windows link problems #1830

Merged

Conversation

roigcarlo
Copy link
Member

Several compilation problems fixed on win.

Mainly the problem is that additional symbols are needed to be exported in case the separation of the "core" and the "interface" has been made (or is it planed). We try to enforce this in Linux as well in the next weeks so compilation in both systems is as close as possible.

@KratosMultiphysics/team-maintainers I just want to let you know that this may happen to other apps if you are using windows. As it will not be the case for all apps it is better to fix (in my opinion) them one by one as the problems appear instead changing in case is not necessary.

@jcotela
Copy link
Member

jcotela commented Apr 10, 2018

@roigcarlo do you think it would be possible to write a few short lines (on the wiki or otherwise) summarizing the best practices regarding this issue? I'd be happy to help check that applications are properly configured but honestly at this point I don't know what to put in the CMakeLists.txt file and I suspect I won't be the only one...

@roigcarlo
Copy link
Member Author

roigcarlo commented Apr 10, 2018

@jcotela Sure. Sorry for not doing this straight from the begining but we ended up facing problems as they appeared.

The problem
The problem appears on Windows when someone tries to link against an application. With boost, both the application code and application interface where compiled inside the same object, and their symbols exported when needed using the KRATOS_API macro. This ended up generating a dynamic object that was the python module and contained the application itself, altogether in the same package.

Pybind changed how things work a bit while creating modules, and one of the changes made impossible to to have a python module as a dependency of one application, drawing derived apps impossible to compile with the old system (as the symbols were inside the module).

In order to solve this, we agreed to divide the applications that are dependencies into FooCore and FooApplication objects, much as the KratosCore & Kratos objects work.

This has increased the number of symbols that need to be visible from the "interface", all files that with add_xxxxx_to_python, as they are no longer in the same place, and this is what is causing the linker errors.

Solution
Just compile your application on Windows and see if there are linking errors. I identified myself the errors under 4 categories:

Missing exported classes:

class foo {}

to

class KRATOS_API(APPLICATION) foo {}

Variables:

KRATOS_DEFINE_VARIABLE(type, FOO)

to

KRATOS_DEFINE_APPLICATION_VARIABLE(APPLICATION, type, FOO)

Local Flags:

KRATOS_DEFINE_LOCAL_FLAG(FOO)

to

KRATOS_DEFINE_LOCAL_APPLICATION_FLAG(APPLICATION, FOO)

Template instantiation in the interface of non header classes

This is more tricky. If you happen to expose a templated class and such class in non header-only, you must explicitly instantiate that in your "core" application, for example in the foo_application.cpp file.

#foo.h
template<typename Ta, typename Tb> class KRATOS_API(FOO_APPLICATION) MyTemplateClass{
}

#foo.cpp
MyTemplateClass::MyTemplateClass() {
    std::cout << "I am implemented in a cpp file" << std::endl;
}

and

# add_utilities_to_python.cpp
class_<MyTemplateClass<int, int>>(m, "MyTemplateClassInts").def(init<>);
class_<MyTemplateClass<double, double>>(m, "MyTemplateClassDoubles").def(init<>);

This will crash, and you need to add:

# foo_application.cpp
template class MyTemplateClass<int, int>;
template class MyTemplateClass<double, double>;

@jcotela
Copy link
Member

jcotela commented Apr 10, 2018

@roigcarlo Thanks a lot for taking the time to write a full explaination of the issue! I fully understand why this was not done in advance, but having it will be extremely useful for the rest of us to avoid repeating the original mistakes.

I've gone ahead and copied your post to the wiki here so that it won't get lost once we merge this.

@roigcarlo roigcarlo merged commit 69b1eed into switching_to_std_pointer Apr 10, 2018
@roigcarlo roigcarlo deleted the pybind11/fixiling-windows-link-problems branch April 10, 2018 14:16
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.

None yet

3 participants