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

Could oatpp support Windows ? #93

Closed
portsip opened this issue Jul 7, 2019 · 14 comments
Closed

Could oatpp support Windows ? #93

portsip opened this issue Jul 7, 2019 · 14 comments
Labels
Enhancement New feature or request Question Further information is requested

Comments

@portsip
Copy link

portsip commented Jul 7, 2019

We are plan use oatpp to replace the cpp-netlib in our project, we consider below libraries:

Oatpp
crow - https://github.com/ipkn/crow
pistache - https://github.com/oktal/pistache

But the crow is no longer maintained, the pistache and oatpp both not support Windows, since our project must support Windows, so may I know does Oatpp has any roadmap to support windows ?

Thanks

@lganzzzo lganzzzo added Enhancement New feature or request Question Further information is requested labels Jul 7, 2019
@lganzzzo
Copy link
Member

lganzzzo commented Jul 7, 2019

Hello @portsip ,

This question was previously raised here (#2) but it did not get much of a traction since.

In short:

Can oatpp support Windows ?
Yes, potentially Windows support can be added comparably easy.

Is Windows support on the road map?
At the point - no - due to other priorities.

Of Course Windows support is very important and personally I want it to be implemented. But at the moment it is hard to allocate time for this.

If one is ready to take this challenge, I will provide all necessary support.

Best Regards,
Leonid

@bamkrs
Copy link
Member

bamkrs commented Jul 30, 2019

Hi @lganzzzo,

we are currently investigating using oatpp for our project. First POC on debian worked like a charm, great framework!
Next we tried to port oatpp to Windows and managed to achieve some progress (cmake compiles up to 58%).
However we are currently stuck at src/oatpp/core/async/Coroutine.hpp:402:

oatpp\src\oatpp/core/async/Coroutine.hpp(402): error C2440: "reinterpret_cast": "ReturnType (__thiscall oatpp::async::AbstractCoroutine::* )(void)" could not be converted to "ReturnType (__thiscall oatpp::async::AbstractCoroutine::MemberCaller::* )(std::shared_ptr<oatpp::network::ConnectionProvider::IOStream>)"
        with
        [
            ReturnType=oatpp::async::AbstractCoroutine::Action
        ]

The visual studio compiler hits this error at src/oatpp/network/virtual_/client/ConnectionProvider.cpp:91.
I understand what it does and how it does it, but not to the depth that I am able to fix the issue visual studio has. Since this compiles fine on clang/llvm/gcc I assume there might be a quick solution to that. Can you provide any help where to start looking?

Cheers!
Benedikt

@lganzzzo
Copy link
Member

lganzzzo commented Jul 30, 2019

Hello @bhorn !

Thank you for trying to port oatpp to Windows! - This is a really wanted feature.

The only thing this hack MemberCaller is needed is here:
https://github.com/oatpp/oatpp/blob/master/src/oatpp/core/async/Coroutine.hpp#L803

So that child coroutine (the one which was called by the 'parent' coroutine) could callback the parent with correct set of parameters by the base class - AbstractCoroutine.

The trick here is to keep correct memory alignment during the call, so that all members of the class are interpreted correctly.

I guess that it can be refactored easily to something more clear and readable.
If you manage to do this - it will be great. Just don't forget to submit a PR :)
I'll try to look at this on my side too.

Please keep me updated on the progress!
There are few more challenges there on the way to oatpp-Windows port. - I'll be happy assist!

Best Regards,
Leonid

@bamkrs
Copy link
Member

bamkrs commented Jul 31, 2019

Hey Leonid,

it turned out to be one of these "visual studio" things where Microsoft tries to optimize stuff where it should not.
There are two ways fixing it, either an compiler-switch or an #pragma. (https://docs.microsoft.com/en-us/cpp/preprocessor/pointers-to-members?view=vs-2019)
I decided to use the compiler-switch to keep the code impact as small as possible. However this could be changed to that pragma if you are no fan of compiler-switches.
However, we record great progress and I'll keep you up to date.

Best Regards,
Benedikt

@lganzzzo
Copy link
Member

Hey @bhorn ,

Thanks for the update!

I think compiler-switch is a good choice. Especially that the code impact is minimal.
Later we can think of a some work-around specifically for this case.

You may also opt to join https://gitter.im/oatpp-framework/Lobby for updates and tech discussions.

Regards,
Leonid

@bamkrs
Copy link
Member

bamkrs commented Jul 31, 2019

Hi @lganzzzo ,

thank you for the invitation, i'll join soon.
The library itself does compile now (yay!) and we are trying to get the tests working.

But there we found the next big bummer. Something in the DTO-Macros and ENDPOINT-Macros does not expand correctly in MSVC.

OATPP_MACRO_HAS_ARGS always evaluates to 1 in MSVC. Thus OATPP_MACRO_DTO_FIELD_1 is invoked which ultimately expands DTO_FIELD(String, _string); to

oatpp::data::mapping::type::Type::Property* Z__CLASS_FIELD__string = Z__CLASS_GET_FIELD__string(static_cast<oatpp::base::Countable*>(this), (oatpp::data::mapping::type::AbstractObjectWrapper*)(&_string)); static oatpp::data::mapping::type::Type::Property* Z__CLASS_GET_FIELD__string(oatpp::base::Countable* _this, oatpp::data::mapping::type::AbstractObjectWrapper* _reg) { static oatpp::data::mapping::type::Type::Property* field = new oatpp::data::mapping::type::Type::Property(Z__CLASS_GET_FIELDS_MAP(), (v_int64) _reg - (v_int64) _this, , String::Class::getType()); return field; } String _string;

instead of

oatpp::data::mapping::type::Type::Property* Z__CLASS_FIELD__string = Z__CLASS_GET_FIELD__string(static_cast<oatpp::base::Countable*>(this), (oatpp::data::mapping::type::AbstractObjectWrapper*)(&_string)); static oatpp::data::mapping::type::Type::Property* Z__CLASS_GET_FIELD__string(oatpp::base::Countable* _this, oatpp::data::mapping::type::AbstractObjectWrapper* _reg) { static oatpp::data::mapping::type::Type::Property* field = new oatpp::data::mapping::type::Type::Property(Z__CLASS_GET_FIELDS_MAP(), (v_int64) _reg - (v_int64) _this, "_string", String::Class::getType()); return field; } String _string;

Watch for the missing "_string" in the vs expanded macro. When following the discussion in https://gustedt.wordpress.com/2010/06/08/detect-empty-macro-arguments/ where you got the snippet from, this seems to be an MSVC-"Bug" we need to find a workaround for. Someone found a workaround using Boost for this but it is somewhat huge when refactoring the Boost code.

Best Regards,
Benedikt

@lganzzzo
Copy link
Member

Hey @bhorn,

Thanks for the update!
I'll be able to take a look at this issue this evening.

Meanwhile you may add name qualifiers manually in the following macros - in order to proceed and see if your build is working.

  • DTO_FIELD(String, _string, "_string")
  • Same in PATH, HEADER, QUERY
  • In the ENDPOINT just inject the Request object in order to make it non - empty.

But for sure this is something that has to be fixed.

Regards,
Leonid

@bamkrs
Copy link
Member

bamkrs commented Jul 31, 2019

Hey @lganzzzo,

I found another compiler switch (see https://github.com/bhorn/oatpp/blob/438fafd49fdd09cd3a18eebfcfe30f87f276b631/test/CMakeLists.txt#L2) that forces MSVC to comply to C99 macro parsing which renders DTO_FIELD and ENDPOINT useable again. However this is rather hacky and may introduces some drawbacks later on if one wants to use those macros and the Windows-API in the same file.

However, I had to change the tests a bit since you used field names like _int32 which seems to be an MSVC specific type already. They are called f_int32 now, did the trick.

I'm at the point where everything compiles fine:
https://github.com/bhorn/oatpp

The first commit (bamkrs@3aa2145) enables oatpp library to compile with MSVC and the Windows SDK.
The second commit (bamkrs@438fafd) addresses all MSVC quirks and other problems that MSVC fails to do compared to clang/llvm/gcc.

The only show-stopper is the IOEventWorker. Windows doesn't have epoll or something with a similar API. So I quickly tried to hack something with wepoll. It compiles fine but doesn't link. I and none of my colleques could figure out why because no one here excels with the MSVC linker.

Yes, I commited and pushed this unfinished hack. Since we are at the end of our knowledge, we hope someone might see the problem and can finish this work. I have to step back here for the moment since the allocated time for this POC-Study is used up.

Best Regards,
Benedikt

@lganzzzo
Copy link
Member

Hey @bhorn ,

Thank you for the great work!
This is a very good starting point!

I'll try to substitute a simple IOWorker instead of an Event one so that we could at least run the tests.

Once I have it, I'll submit a PR to your repo.

Thank you for your time and your interest in the project!

Best Regards,
Leonid

@lganzzzo lganzzzo mentioned this issue Aug 6, 2019
@lganzzzo
Copy link
Member

lganzzzo commented Aug 6, 2019

Hey All, @portsip,

The first draft of Windows support is currently available in the branch - https://github.com/oatpp/oatpp/tree/windows_support
Also see the PR - #104.

Best Regards,
Leonid

@portsip
Copy link
Author

portsip commented Aug 8, 2019

@lganzzzo Thanks, we will check.

@lganzzzo
Copy link
Member

Code for Linux/Unix/Windows platforms is fully-compatible now!
All MSVC specific compiler switches where removed.

See PR - #112

@lganzzzo lganzzzo added this to In progress in oat++ project Aug 22, 2019
@lganzzzo
Copy link
Member

lganzzzo commented Nov 6, 2019

Windows support is added since Oat++ version 0.19.8

@lganzzzo lganzzzo closed this as completed Nov 6, 2019
oat++ project automation moved this from In progress to Done Nov 6, 2019
@kenkopelson
Copy link

May I ask which IOWorker module is used for the Windows platform? Was someone able to get "wepoll" working, or was something else done? Thank you the info in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Question Further information is requested
Projects
oat++ project
  
Done
Development

No branches or pull requests

4 participants