|
Description
Alexander Golubev (Fat-Zer)
2014-04-10 12:40:44 CDT
Created attachment 2023 [details]
0001-remove-useless-cmake-configuration-options-WITH_XEXT.patch
Created attachment 2024 [details]
0002-Move-the-XRandr-test-to-main-ConfigureChecks.cmake.patch
Created attachment 2025 [details]
0003-twin-compton-tde-transparent-pass-TDE-build-time-con.patch
Created attachment 2026 [details]
0004-Move-the-Xinerama-test-to-main-ConfigureChecks.cmake.patch
Created attachment 2027 [details]
0005-twin-compton-tde-make-XRandR-support-optional-in-com.patch
I'd prefer a patchset rather than one big patch because each one here is a quite logically finished stage.
PS: I think this patch set supposed to be applied before the release, but I don't feel enough power to add it to the meta bug myself, so consider it please.
Created attachment 2041 [details]
buildlog with FTBFS
Patches look good. But when I tried to apply, for me cause FTBFS.
Observation: When I banned CONFIG_XSYNC, building is successful. Note: I tested on Debian 6.0 (squeeze - amd64) As I traced for CONFIG_XSYNC is required fence sync support - see http://lists.x.org/archives/xorg-devel/2010-September/013231.html Instead of CONFIG_XSYNC always on, should be added detection XSyncFence. (In reply to Slávek Banko from comment #8) > As I traced for CONFIG_XSYNC is required fence sync support - see > http://lists.x.org/archives/xorg-devel/2010-September/013231.html > > Instead of CONFIG_XSYNC always on, should be added detection XSyncFence. So, libXext in Debian 6.0 doesn't have XSyncFence at all? Am I understood that correct? Also can you check if it builds well if you comment out "#define CONFIG_XSYNC 1" either in compton_config.h.cmake or compton_config.h ? Created attachment 2042 [details]
0006-twin-compton-make-CONFIG_XSYNC-autoconfigurable-to-f.patch
Proposed patch to fix FTBFS mentioned above.
Created attachment 2043 [details]
0006-twin-compton-make-CONFIG_XSYNC-autoconfigurable-to-f (1)
Nice - almost identical patch I had also prepared :)
Fixed a typo, for which was CONFIG_XSYNC always set, regardless of the test XSyncFence.
Created attachment 2044 [details]
0007-twin-compton-glx-sync.diff
During preparation prior patch, I decided that CONFIG_GLX_XSYNC is a typo, and that has to be properly CONFIG_GLX_SYNC. The attached patch corrects this. And because CONFIG_GLX_SYNC can not be used unless CONFIG_XSYNC is also set, the setting CONFIG_GLX_SYNC adjusted.
(In reply to Slávek Banko from comment #13) > Created attachment 2044 [details] > 0007-twin-compton-glx-sync.diff > > During preparation prior patch, I decided that CONFIG_GLX_XSYNC is a typo, > and that has to be properly CONFIG_GLX_SYNC. The attached patch corrects > this. And because CONFIG_GLX_SYNC can not be used unless CONFIG_XSYNC is > also set, the setting CONFIG_GLX_SYNC adjusted. I've noted in the patch that CONFIG_GLX_XSYNC is not actually used on the code, It seems either haven't implemented yet or already removed. So make it depend over CONFIG_X_XSYNC may be confusing... May be it would be better to completely remove it from cmake? (In reply to Fat-Zer from comment #14) > (In reply to Slávek Banko from comment #13) > > Created attachment 2044 [details] > > 0007-twin-compton-glx-sync.diff > > > > During preparation prior patch, I decided that CONFIG_GLX_XSYNC is a typo, > > and that has to be properly CONFIG_GLX_SYNC. The attached patch corrects > > this. And because CONFIG_GLX_SYNC can not be used unless CONFIG_XSYNC is > > also set, the setting CONFIG_GLX_SYNC adjusted. > > I've noted in the patch that CONFIG_GLX_XSYNC is not actually used on the > code, It seems either haven't implemented yet or already removed. So make it > depend over CONFIG_X_XSYNC may be confusing... > May be it would be better to completely remove it from cmake? Nevermind about this comment... I haven't read your message properly... It seams you're right... Squash the last 2 patches... Created attachment 2045 [details]
0006-twin-compton-make-CONFIG_XSYNC-autoconfigurable-to-f (2)
Merge of patches 0006 and 0007.
There is another problem. On Ubuntu 11.04 (Natty) first, which defines XSyncFence, occurs FTBFS:
In file included from /tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/opengl.h:11:0,
from /tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/opengl.c:12:
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/common.h: In function 'free_fence':
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/common.h:1938:5: warning: implicit declaration of function 'XSyncDestroyFence'
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/common.h: In function 'xr_sync_':
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/common.h:2278:7: warning: implicit declaration of function 'XSyncCreateFence'
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/common.h:2285:7: warning: implicit declaration of function 'XSyncTriggerFence'
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/common.h:2286:7: warning: implicit declaration of function 'XSyncAwaitFence'
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/common.h:2294:7: warning: implicit declaration of function 'XSyncResetFence'
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/opengl.c: In function 'xr_glx_sync':
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/opengl.c:19:39: error: 'GL_SYNC_X11_FENCE_EXT' undeclared (first use in this function)
/tmp/buildd/tdebase-trinity-14.0.0-s~1202/twin/compton-tde/opengl.c:19:39: note: each undeclared identifier is reported only once for each function it appears in
make[3]: *** [twin/compton-tde/CMakeFiles/compton-tde.dir/opengl.c.o] Error 1
make[2]: *** [twin/compton-tde/CMakeFiles/compton-tde.dir/all] Error 2
make[1]: *** [all] Error 2
make: *** [debian/stamp-makefile-build] Error 2
Created attachment 2047 [details] 0006-twin-compton-make-CONFIG_XSYNC-autoconfigurable-to-f (3) FTBFS mentioned in a comment 17 fixed. Instead of checking existence of type XSyncFence is checked existence of function XSyncCreateFence. (In reply to Slávek Banko from comment #18) > Created attachment 2047 [details] > 0006-twin-compton-make-CONFIG_XSYNC-autoconfigurable-to-f (3) > > FTBFS mentioned in a comment 17 fixed. Instead of checking existence of type > XSyncFence is checked existence of function XSyncCreateFence. The patch looks nice... (In reply to Fat-Zer from comment #19) > (In reply to Slávek Banko from comment #18) > > Created attachment 2047 [details] > > 0006-twin-compton-make-CONFIG_XSYNC-autoconfigurable-to-f (3) > > > > FTBFS mentioned in a comment 17 fixed. Instead of checking existence of type > > XSyncFence is checked existence of function XSyncCreateFence. > > The patch looks nice... Just one moment: you are not supposed to set neither CMAKE_EXTRA_INCLUDE_FILES or CMAKE_REQUIRED_INCLUDES for check_function_exists() because it checks only if the function is provided by the library. So, you supposed to either drop those declarations or use check_symbol_exists() here. IMO the second variant is preferable because it calls for more strict check in that case e.g. tests that the headers are ok as well as the library. Created attachment 2048 [details]
0006-twin-compton-make-CONFIG_XSYNC-autoconfigurable-to-f (4)
It is a paradox that check_symbol_exists I had in my initial draft of patch. But I had at that patch something wrong, and you were quicker with your version of the patch...
I hope that the last change - check_function_exists changed to check_symbol_exists.
Comment on attachment 2023 [details]
0001-remove-useless-cmake-configuration-options-WITH_XEXT.patch
Pushed to GIT in hash 3c75381e.
Comment on attachment 2024 [details]
0002-Move-the-XRandr-test-to-main-ConfigureChecks.cmake.patch
Pushed to GIT in hash 63dd66d0.
Comment on attachment 2025 [details]
0003-twin-compton-tde-transparent-pass-TDE-build-time-con.patch
Pushed to GIT in hash 394545c2.
Comment on attachment 2026 [details]
0004-Move-the-Xinerama-test-to-main-ConfigureChecks.cmake.patch
Pushed to GIT in hash 30252262.
Comment on attachment 2027 [details]
0005-twin-compton-tde-make-XRandR-support-optional-in-com.patch
Pushed to GIT in hash 7c55134d.
Comment on attachment 2048 [details]
0006-twin-compton-make-CONFIG_XSYNC-autoconfigurable-to-f (4)
Pushed to GIT in hash d7235b31.
|