Merge lp:~xnox/unity8/cross-compile into lp:unity8

Proposed by Dimitri John Ledkov on 2013-12-13
Status: Merged
Approved by: Michał Sawicz on 2013-12-13
Approved revision: 591
Merged at revision: 595
Proposed branch: lp:~xnox/unity8/cross-compile
Merge into: lp:unity8
Diff against target: 42 lines (+6/-4)
4 files modified
tests/mocks/LightDM/demo/CMakeLists.txt (+0/-1)
tests/mocks/Unity/Application/CMakeLists.txt (+0/-1)
tests/mocks/Unity/Launcher/CMakeLists.txt (+0/-1)
tests/plugins/AccountsService/CMakeLists.txt (+6/-1)
To merge this branch: bzr merge lp:~xnox/unity8/cross-compile
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve on 2013-12-13
Michał Sawicz 2013-12-13 Approve on 2013-12-13
Review via email: mp+198854@code.launchpad.net

Commit message

Fix cross-compilation

Description of the change

Fix cross-compilation

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:591
http://jenkins.qa.ubuntu.com/job/unity8-ci/1886/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1679
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1613
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/658
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/409
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/410
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/410/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/409
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1488
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1679
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1679/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1613
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1613/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4125
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2308

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1886/rebuild

review: Needs Fixing (continuous-integration)
Albert Astals Cid (aacid) wrote :

Makes sense to me, but otoh Michał was saying we should simply not run qdbusxml2cpp on compile time but on commit time and just have the file in there so I'll leave him to comment if we want this or not.

Dimitri John Ledkov (xnox) wrote :

On 13 December 2013 08:48, Albert Astals Cid
<email address hidden> wrote:
> Makes sense to me, but otoh Michał was saying we should simply not run qdbusxml2cpp on compile time but on commit time and just have the file in there so I'll leave him to comment if we want this or not.

Please do not commit auto-generated code to VCS. All other upstreams /
packages in the archive, generate dbus code at compile time,
regardless if it's native or cross. Why commit a static version, when
it can always be generated afresh?

--
Regards,

Dimitri.

Michał Sawicz (saviq) wrote :

On 13.12.2013 10:12, Dimitri John Ledkov wrote:
> Please do not commit auto-generated code to VCS. All other upstreams /
> packages in the archive, generate dbus code at compile time,
> regardless if it's native or cross. Why commit a static version, when
> it can always be generated afresh?

http://qt-project.org/doc/qt-5.0/qtdbus/qdbusxml2cpp.html

"The qdbusxml2cpp tool is not meant to be run every time you compile
your application. Instead, it's meant to be used when developing the
code or when the interface changes."

I'm leaning towards that just because it makes sense, that generated
file will barely ever change (as long as the source file doesn't
change). OTOH building against different versions of Qt, in this case,
might yield different results for the generated files...

Dimitri John Ledkov (xnox) wrote :

On 13 December 2013 09:45, Michał Sawicz <email address hidden> wrote:
> On 13.12.2013 10:12, Dimitri John Ledkov wrote:
>> Please do not commit auto-generated code to VCS. All other upstreams /
>> packages in the archive, generate dbus code at compile time,
>> regardless if it's native or cross. Why commit a static version, when
>> it can always be generated afresh?
>
> http://qt-project.org/doc/qt-5.0/qtdbus/qdbusxml2cpp.html
>
> "The qdbusxml2cpp tool is not meant to be run every time you compile
> your application. Instead, it's meant to be used when developing the
> code or when the interface changes."
>

it does make sense to ship pre-generated dbus code in, e.g. release
tarball, but not in the version control system. And that dbus tool, is
not that different from all the other dbus implementations - e.g.
upstream (libdbus), gnome/gio dbus, libnih-dbus, etc. Also as per
debian policy, generated source code must be re-generated at
build-time which is applied consistently for all dbus, cython, vala,
etc. generators.

> I'm leaning towards that just because it makes sense, that generated
> file will barely ever change (as long as the source file doesn't
> change). OTOH building against different versions of Qt, in this case,
> might yield different results for the generated files...
>

For QA and CI it's a slippery slope to forget regenerating static
copies. Or not catch that one cannot regenerate it anymore.

--
Regards,

Dimitri.

Michał Sawicz (saviq) wrote :

On 13.12.2013 10:54, Dimitri John Ledkov wrote:
> it does make sense to ship pre-generated dbus code in, e.g. release
> tarball, but not in the version control system. And that dbus tool, is
> not that different from all the other dbus implementations - e.g.
> upstream (libdbus), gnome/gio dbus, libnih-dbus, etc. Also as per
> debian policy, generated source code must be re-generated at
> build-time which is applied consistently for all dbus, cython, vala,
> etc. generators.
>
>> >I'm leaning towards that just because it makes sense, that generated
>> >file will barely ever change (as long as the source file doesn't
>> >change). OTOH building against different versions of Qt, in this case,
>> >might yield different results for the generated files...
>> >
> For QA and CI it's a slippery slope to forget regenerating static
> copies. Or not catch that one cannot regenerate it anymore.

Yeah, ok. You convinced me.

Michał Sawicz (saviq) :
review: Approve
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/mocks/LightDM/demo/CMakeLists.txt'
2--- tests/mocks/LightDM/demo/CMakeLists.txt 2013-06-14 19:35:25 +0000
3+++ tests/mocks/LightDM/demo/CMakeLists.txt 2013-12-13 01:03:42 +0000
4@@ -1,4 +1,3 @@
5-include(FindPkgConfig)
6 pkg_check_modules(LIBUSERMETRICSOUTPUT REQUIRED libusermetricsoutput-1)
7
8 set(LibLightDM_SOURCES
9
10=== modified file 'tests/mocks/Unity/Application/CMakeLists.txt'
11--- tests/mocks/Unity/Application/CMakeLists.txt 2013-11-25 17:26:15 +0000
12+++ tests/mocks/Unity/Application/CMakeLists.txt 2013-12-13 01:03:42 +0000
13@@ -1,4 +1,3 @@
14-include(FindPkgConfig)
15 pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application)
16
17 set(FakeUnityApplicationQml_SOURCES
18
19=== modified file 'tests/mocks/Unity/Launcher/CMakeLists.txt'
20--- tests/mocks/Unity/Launcher/CMakeLists.txt 2013-09-12 07:05:56 +0000
21+++ tests/mocks/Unity/Launcher/CMakeLists.txt 2013-12-13 01:03:42 +0000
22@@ -1,4 +1,3 @@
23-include(FindPkgConfig)
24 pkg_check_modules(LAUNCHER_API REQUIRED unity-shell-launcher=3)
25
26 add_definitions(-DTOP_SRCDIR="${CMAKE_SOURCE_DIR}")
27
28=== modified file 'tests/plugins/AccountsService/CMakeLists.txt'
29--- tests/plugins/AccountsService/CMakeLists.txt 2013-09-05 18:09:41 +0000
30+++ tests/plugins/AccountsService/CMakeLists.txt 2013-12-13 01:03:42 +0000
31@@ -1,6 +1,11 @@
32+if(DEFINED ENV{DEB_BUILD_MULTIARCH})
33+ set(QDBUSXML2CPP_EXECUTABLE "/usr/lib/$ENV{DEB_BUILD_MULTIARCH}/qt5/bin/qdbusxml2cpp")
34+else()
35+ FIND_PROGRAM(QDBUSXML2CPP_EXECUTABLE qdbusxml2cpp)
36+endif()
37 macro(make_dbus_class NAME INTERFACE)
38 if(${CMAKE_CURRENT_SOURCE_DIR}/interfaces.xml IS_NEWER_THAN ${CMAKE_CURRENT_BINARY_DIR}/${NAME}Adaptor.h)
39- execute_process(COMMAND qdbusxml2cpp -c ${NAME}Adaptor -a ${CMAKE_CURRENT_BINARY_DIR}/${NAME}Adaptor ${CMAKE_CURRENT_SOURCE_DIR}/interfaces.xml ${INTERFACE})
40+ execute_process(COMMAND ${QDBUSXML2CPP_EXECUTABLE} -c ${NAME}Adaptor -a ${CMAKE_CURRENT_BINARY_DIR}/${NAME}Adaptor ${CMAKE_CURRENT_SOURCE_DIR}/interfaces.xml ${INTERFACE})
41 endif()
42 endmacro(make_dbus_class)
43

Subscribers

People subscribed via source and target branches