A backend is required for kdnssd
AbandonedPublic

Authored by adridg on Jun 4 2018, 11:13 AM.

Details

Reviewers
None
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.

Diff Detail

Repository
R272 KDNSSD
Branch
require-backend
Lint
No Linters Available
Unit
No Unit Test Coverage
adridg created this revision.Jun 4 2018, 11:13 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptJun 4 2018, 11:13 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
adridg requested review of this revision.Jun 4 2018, 11:13 AM
adridg edited the test plan for this revision. (Show Details)Jun 4 2018, 11:14 AM

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)

cgiboudeaux added inline comments.
CMakeLists.txt
67–68

wrong place

adridg added inline comments.Jun 4 2018, 11:48 AM
CMakeLists.txt
67–68

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.

cgiboudeaux added inline comments.Jun 4 2018, 11:50 AM
CMakeLists.txt
67–68

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.

adridg updated this revision to Diff 35524.Jun 4 2018, 1:16 PM

Use the right place (sorry Christophe, to use a Dutch phrase:
I must have been looking with my nose).

aacid added a subscriber: aacid.Jun 4 2018, 9:09 PM

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

adridg abandoned this revision.May 31 2020, 3:28 PM

Yeah, let's not then.