Bug 103003

Summary:	building fails when tests are disabled
Product:	poppler	Reporter:	Pavel Vinogradov <public>
Component:	general	Assignee:	poppler-bugs <poppler-bugs>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium	CC:	r.hieber
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	possible solution CMake: add the custom buildtests target only once

Description Pavel Vinogradov 2017-09-27 02:25:46 UTC

Created attachment 134499 [details] [review]
possible solution

cmake fails when called with these flags: -DENABLE_TESTING=0 -DBUILD_GTK_TESTS=0 -DBUILD_QT4_TESTS=0 -DBUILD_QT5_TESTS=0 -DBUILD_CPP_TESTS=0

Output:
CMake Error at cmake/modules/PopplerMacros.cmake:18 (add_custom_target):
  add_custom_target cannot create target "buildtests" because another target
  with the same name already exists.  The existing target is a custom target
  created in source directory "/usr/src/poppler-git/test".  See documentation
  for policy CMP0002 for more details.
Call Stack (most recent call first):
  cpp/tests/CMakeLists.txt:13 (poppler_add_test)
  cpp/tests/CMakeLists.txt:21 (cpp_add_simpletest)

Solution:
patch is attached.

Comment 1 Albert Astals Cid 2017-09-29 23:04:57 UTC

The patch doesn't make any sense to me, we're already setting CMP0002 to old, no?

./modules/PopplerDefaults.cmake:12:cmake_policy(SET CMP0002 OLD)

Also ideally we should be moving from all those OLD settings and having proper fixes.

Comment 2 Pavel Vinogradov 2017-09-29 23:26:16 UTC

Agree with proper fixes, like moving back to autotools, hehe. But the patch works for me (for now).

(In reply to Albert Astals Cid from comment #1)
> The patch doesn't make any sense to me, we're already setting CMP0002 to
> old, no?
> 
> ./modules/PopplerDefaults.cmake:12:cmake_policy(SET CMP0002 OLD)
> 
> Also ideally we should be moving from all those OLD settings and having
> proper fixes.

Comment 3 Albert Astals Cid 2017-09-30 09:15:36 UTC

(In reply to Pavel Vinogradov from comment #2)
> Agree with proper fixes, like moving back to autotools, hehe. 

Ahhh, the smart-ass answer, you just made me lose any interest i had in helping you. Congratulations.

> But the patch works for me (for now).

That doesn't mean it's correct, you could remove all the tests and the patch would work for you too.

> 
> (In reply to Albert Astals Cid from comment #1)
> > The patch doesn't make any sense to me, we're already setting CMP0002 to
> > old, no?
> > 
> > ./modules/PopplerDefaults.cmake:12:cmake_policy(SET CMP0002 OLD)
> > 
> > Also ideally we should be moving from all those OLD settings and having
> > proper fixes.

Comment 4 Roland Hieber 2017-11-06 14:33:01 UTC

*** Bug 103591 has been marked as a duplicate of this bug. ***

Comment 5 Roland Hieber 2017-11-06 14:45:05 UTC

Created attachment 135259 [details] [review]
CMake: add the custom buildtests target only once

Here is a better patch which makes BUILDTESTS_ADDED a global property, so the `buildtests` target gets added only once. As far as I understood, this seems to fulfil the requirements of CMP0002.

Comment 6 Pavel Vinogradov 2017-11-06 23:33:31 UTC

works for me too. thanks.

(In reply to Roland Hieber from comment #5)
> Created attachment 135259 [details] [review] [review]
> CMake: add the custom buildtests target only once
> 
> Here is a better patch which makes BUILDTESTS_ADDED a global property, so
> the `buildtests` target gets added only once. As far as I understood, this
> seems to fulfil the requirements of CMP0002.

Comment 7 Albert Astals Cid 2017-11-08 07:39:26 UTC

Pushed

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.