Closed pemensik closed 1 year ago
It seems a bad idea to me to require other software do support version 3 only or version 2 only, but without possibility to support both versions without significant effort.
The main problem is that v3 represents a huge rewrite of the whole library with some changes to the API too in order to drastically reduce compilation times. It's not that easy to write a migration layer unfortunately.
Is any notes documented, how should old code be converted to new library format?
I have made surprisingly lot of conversions by crude sed script:
s/uvw::TimerHandle/uvw::timer_handle/g
s/uvw::TimerEvent/uvw::timer_event/g
s/uvw::timer_handle::Time/uvw::timer_handle::time/g
s/uvw::TCPHandle/uvw::tcp_handle/g
s/uvw::UDPHandle/uvw::udp_handle/g
s/uvw::UDPDataEvent/uvw::udp_data_event/g
s/uvw::Loop/uvw::loop/g
s/uvw::loop::Time/uvw::loop::time/g
s/uvw::ErrorEvent/uvw::error_event/g
s/uvw::ConnectEvent/uvw::connect_event/g
s/uvw::CloseEvent/uvw::close_event/g
s/uvw::ShutdownEvent/uvw::shutdown_event/g
s/uvw::EndEvent/uvw::end_event/g
s/uvw::DataEvent/uvw::data_event/g
s/uvw::WriteEvent/uvw::write_event/g
s/uvw::IPv6/uvw::ipv6/g
s/uvw::IPv4/uvw::ipv4/g
s/uvw::udp_handle::Bind/uvw::udp_handle::udp_flags/
s/uvw::tcp_handle::Bind/uvw::tcp_handle::tcp_flags/
It helped in lot of cases. Remaining cases usually were using bind
I have made surprisingly lot of conversions by crude sed script
Fair enough. Makes sense.
Is there possible way to use address in std::string and still be able specify family, whether that should use IPv4 or IPv6? Do I have to use sockaddr if I want it specified manually?
Yeah, I don't remember all the internals by heart but we mimic the libuv api 90% of the time, so I'd tell you that if you can't do it there then you can't do it in uvw and vice versa.
I looked into util.cpp, details::ip_addr() in new version. It seems suspicious to me. Does it work correctly for IPV6 addresses? struct sockaddr_in6
has size of 28, but normal struct sockaddr
has size just of 16. There is special struct sockaddr_storage
, which has size of 128, that should be able to contain any address. Since the function returns just structure of size 16, does it contain full IPv6 address when used?
DNS software I have seen often uses union { sockaddr sa, sockaddr_in in, sockaddr_in6 in6 }
to store multiple families in single compact structure. But those use just C language. Is there any C++ standard behaviour, which ensures the current construction will always work? Even for different compilers like MSCV, gcc or clang? I haven't tested it yet, but it seems to me there is no obvious guarantee that should work reliably.
Fair enough. Makes sense.
It seems to me backward compatible namespace uvw { typedef UDPHandle udp_handle; };
might provide backward compatibility to types, which just changed type name. That won't work for bind() calls, which have changed from template to normal function.
Yeah, I don't remember all the internals by heart but we mimic the libuv api 90% of the time, so I'd tell you that if you can't do it there then you can't do it in uvw and vice versa.
It seems to me not in this case. I think the problem is with returning sockaddr by value, not by pointer or reference. Conversion note from libuv 0.10 mentions example how to do that. Since libuv call change just the pointer, it should be safe. But because uvw returns copy of sockaddr, I think that copy would be incomplete for sockaddr_in6. Change of pointer type is safe if it points to data long enough. It seems to me this is not the case in current implementation, where it returns the reference. But since the function returns just sockaddr, I doubt that will work correctly. Returning sockaddr_storage would be safe, just initializing structure by pointer or reference as well.
It seems it also hides possible format error in address text, it initializes just zeroed structure. Which might not be the same. Which is not done by original libuv.
It would be nice if any header had some defines with major (and optionally minor version). That would allow to support somehow also older version from single code, with help of preprocessor's #if UVW_VERSION_MAJOR >= 3
. It does not seem possible now, which makes backward compatibility almost undoable.
The main problem is probably that it's not available in the older versions. You'd have to check if it exists, then what version is it. Otherwise, it's going to fail.
Sure, you may define it for older versions too:
#ifndef UVW_VERSION_MAJOR
#define UVW_VERSION_MAJOR 2
#endif
Of course it would be better to have it right from the start, but adding it for next major version would be better than nothing at all. Otherwise just some autoconf hacks might be used to detect proper version.
Sure, you may define it for older versions too:
Yeah, I mean, I can't really update ie version 2.0.0 and therefore I'd have to create version 2.0.1 for that. However, it doesn't really solve the problem this way. So, it seems pointless to me.
Otherwise just some autoconf hacks might be used to detect proper version.
I think one can easily get it with a cmake machinery. You've both the libuv version and the uwv version when generating the project. So, in theory, extracting the version should be possible. Have you tried it already?
The previous define could be used by the project using uvw, in my case flamethrower.
No, I haven't tried cmake compat yet. But to make conditional change to the code, even that cmake would have to define some variable or define, on which conditional preprocessor could choose old or the new format. Wouldn't it be easier if the uvw defined such values itself? CMake might then provide compat definition for previous versions.
Yeah, it would be easier if uvw
exported a version file to use in your own software. The cmake solution can be a working one for older versions though.
The problem is that I can add this number to version ie 2.12.0 but I can't change its commit. Therefore, the best that I can do is to create version 2.12.1 with this information. To me, it looks even worse then. You've the version if using 2.x.y only if y isn't 0, otherwise I'm sorry but the version isn't available. It sounds really odd, doesn't it?
So, from my point of view, the cleanest approach is to add a version file upstream and that's is. For older versions, one can use the cmake trick or any other trick because we can't really fix it without breaking something else in the meantime (like, the version commit).
Am I missing something?
Closing this due to starvation.
I were preparing update from 2.12 to 3.0 and tried to compile flamethrower with it. It does not compile anymore because a lot of renamed classes. Is there any backward compatible layer, which would allow supporting old code without changes? Or some way, which might allow modified code to support both 3.0 and 2.12 version?