D13328: A backend is required for kdnssd
adridg abandoned this revision. adridg added a comment. Yeah, let's not then. REPOSITORY R272 KDNSSD REVISION DETAIL https://phabricator.kde.org/D13328 To: adridg Cc: aacid, cgiboudeaux, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D13328: A backend is required for kdnssd
adridg created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. adridg requested review of this revision. REVISION SUMMARY Refuse to build if there are no backends for DNSSD at all. The messages for the OPTIONAL backeds mention that one is required, but it wasn't enforced. KDE CI has been building with no backend configured for some time. TEST PLAN Configure without as backend: cmake -DCMAKE_DISABLE_FIND_PACKAGE_Avahi=1 \ -DCMAKE_DISABLE_FIND_PACKAGE_DNSSD=1 note that the backend requirements are optional, but one is required. REPOSITORY R272 KDNSSD BRANCH require-backend REVISION DETAIL https://phabricator.kde.org/D13328 AFFECTED FILES CMakeLists.txt To: adridg Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D13328: A backend is required for kdnssd
adridg added a comment. Something to consider, though, is that it might perhaps be useful to have kdnssd build -- without a backend -- for environments where there is no real dnssd and any service discovery returns "not found". Requiring a backend makes that impossible. (So something else to do is to install avahi on the openSUSE and other Linux KDE CI machines) REPOSITORY R272 KDNSSD REVISION DETAIL https://phabricator.kde.org/D13328 To: adridg Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D13328: A backend is required for kdnssd
cgiboudeaux added inline comments. INLINE COMMENTS > CMakeLists.txt:65-66 > ecm_install_po_files_as_qm(po) > +else() > +message(FATAL_ERROR "Either Avahi or DNSSD is required for KDE > applications to make use of multicast DNS/DNS-SD service discovery") > endif() wrong place REPOSITORY R272 KDNSSD REVISION DETAIL https://phabricator.kde.org/D13328 To: adridg Cc: cgiboudeaux, kde-frameworks-devel, michaelh, ngraham, bruns
D13328: A backend is required for kdnssd
adridg added inline comments. INLINE COMMENTS > cgiboudeaux wrote in CMakeLists.txt:65-66 > wrong place Hi. If you're posting a review, please provide constructive input. If there are policies or guidelines about places, please link to them. If there aren't, but you would prefer a change, please describe the (outline of the) change. If you mean "there should be an additional space in the else() statement to match the text formatting from line 57", then write that. REPOSITORY R272 KDNSSD REVISION DETAIL https://phabricator.kde.org/D13328 To: adridg Cc: cgiboudeaux, kde-frameworks-devel, michaelh, ngraham, bruns
D13328: A backend is required for kdnssd
cgiboudeaux added inline comments.
INLINE COMMENTS
> adridg wrote in CMakeLists.txt:65-66
> Hi. If you're posting a review, please provide constructive input. If there
> are policies or guidelines about places, please link to them. If there
> aren't, but you would prefer a change, please describe the (outline of the)
> change. If you mean "there should be an additional space in the else()
> statement to match the text formatting from line 57", then write that.
your else() follows if (IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/po"), I
thought the comment was clear.
Your current patch means: if there's no "po" subdir, throw a fatal error with
an unrelated message.
REPOSITORY
R272 KDNSSD
REVISION DETAIL
https://phabricator.kde.org/D13328
To: adridg
Cc: cgiboudeaux, kde-frameworks-devel, michaelh, ngraham, bruns
D13328: A backend is required for kdnssd
adridg updated this revision to Diff 35524. adridg added a comment. Use the right place (sorry Christophe, to use a Dutch phrase: I must have been looking with my nose). REPOSITORY R272 KDNSSD CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13328?vs=35514&id=35524 BRANCH require-backend REVISION DETAIL https://phabricator.kde.org/D13328 AFFECTED FILES CMakeLists.txt To: adridg Cc: cgiboudeaux, kde-frameworks-devel, michaelh, ngraham, bruns
D13328: A backend is required for kdnssd
aacid added a comment. I'm with you that having a possibility to just compile kdnssd without any backend may be of value so i would either not commit this or improve the cmake so there's a way to say I_KNOW_WHAT_I_AM_DOING_KEEP_GOING REPOSITORY R272 KDNSSD REVISION DETAIL https://phabricator.kde.org/D13328 To: adridg Cc: aacid, cgiboudeaux, kde-frameworks-devel, michaelh, ngraham, bruns
D13328: A backend is required for kdnssd
adridg edited the test plan for this revision. REPOSITORY R272 KDNSSD REVISION DETAIL https://phabricator.kde.org/D13328 To: adridg Cc: kde-frameworks-devel, michaelh, ngraham, bruns
