-
Notifications
You must be signed in to change notification settings - Fork 36
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
Failsafe when Servus implementation can't created and fallback to dummy #80
Conversation
ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for the failsafe, I remember building an app on my laptop for 30 min and have it crash on startup because Servus had no implementation and this was not handled in ZeroEQ, or something similar.
If it were for me, I would not allow Servus to configure without a backend at all. But to do that we need to remove the Serializable, URI and friends from it first...
CMakeLists.txt
Outdated
set(CPACK_DEBIAN_PACKAGE_DEPENDS "libavahi-client3, avahi-daemon") | ||
set(SERVUS_DEBIAN_PACKAGE_DEPENDS) | ||
if (avahi-client_FOUND) | ||
list(APPEND SERVUS_DEBIAN_PACKAGE_DEPENDS "libavahi-client3, avahi-daemon") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The syntax is now: NAME_PACKAGE_DEB_DEPENDS, and it can be a CMake list:
https://github.com/Eyescale/CMake/blob/master/CommonCPack.cmake#L17
so this line should be:
list(APPEND SERVUS_PACKAGE_DEB_DEPENDS libavahi-client3 avahi-daemon)
doc/Changelog.md
Outdated
@@ -3,6 +3,8 @@ | |||
|
|||
# git master | |||
|
|||
* [80](https://github.com/HBPVis/Servus/pull/80): | |||
Failsafe when Servus implementation can't created and fallback to dummy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't be created
Failsafe in ZeroEQ is another story because the dummy Servus implementation throws if you try to announce a service. This commit is intented to not make it not throw when you create a Servus object intended for browsing only. |
hmm yes, I think we'll have to revisit this API at some point. The constructor was not documented as throwing so this is a bugfix, but now that it does not throw the user can't know if he created a valid object because isAvailable() returns a compile-time constant... |
set(SERVUS_PACKAGE_DEB_DEPENDS) | ||
if (avahi-client_FOUND) | ||
list(APPEND SERVUS_PACKAGE_DEB_DEPENDS libavahi-client3 avahi-daemon) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should be unconditional, otherwise catch-22
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get your point. This variable is set for packaging after configuration. If avahi was not present at configuration time you don't want to add it a as package dependency.
This is not to be confused with SERVUS_DEB_DEPENDS, which lists these packages unconditionally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad. I confused it with DEB_PACKAGES.
This is needed for the nix build, which is complied with avahi support, but when run somewhere where the avahi daemon is not running it crashes whatever application uses Servus.