Merge lp:~robert-ancell/mir/server-headers into lp:~mir-team/mir/trunk

[Robert Ancell]

Correctly substitute into mirserver.pc and install server header files

Install the server headers so that Mir compositors can be developed separately from lp:mir

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:498
http://jenkins.qa.ubuntu.com/job/mir-ci/77/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/78/console
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/78/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/77/rebuild

review: Approve (continuous-integration)

[Thomas Voß (thomas-voss)]

Looks good, a minor niggle: Could you rename SERVER_PUBLIC_HEADERS to MIR_SERVER_PUBLIC_HEADERS to match the naming pattern?

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Is this a full or partial resolution for bug 1136938?

[Daniel van Vugt (vanvugt)]

P.S. I think Thomas is wrong. SERVER_PUBLIC_HEADERS is a local variable. It does not need a namespace "MIR_" prefix.

[Daniel van Vugt (vanvugt)]

I used to distinguish locals by making them lower case. We could start doing that...

[Robert Ancell (robert-ancell)]

Yes, it does fix bug 1136938.

[Daniel van Vugt (vanvugt)]

It works well. Stylistically I have no immediate complaint. And bug 1136938 is indeed fixed.

However the list of "server headers" is very long. Some headers such as:
    ${CMAKE_SOURCE_DIR}/include/server/mir/graphics/gl_renderer.h
should not be installed as part of the server (that one is for example code and tests only). There will probably be other such headers that should not be in that list so please re-evaluate the minimum set of headers you really need right now.

review: Needs Fixing

[Robert Ancell (robert-ancell)]

> Looks good, a minor niggle: Could you rename SERVER_PUBLIC_HEADERS to
> MIR_SERVER_PUBLIC_HEADERS to match the naming pattern?

This matches CLIENT_PUBLIC_HEADERS in src/client/CMakeLists.txt so I'll leave that as consistent. If the naming is wrong it should be updated for both in a separate patch.

[Alan Griffiths (alan-griffiths)]

I don't like this being a list of individual files - there's too much chance of it getting out of step.

I would expect everything under either include/common/mir or include/server/mir to be part of this.

IMO if there are things there that shouldn't be exposed then that is a separate issue.

review: Needs Fixing

[Robert Carr (robertcarr)]

I agree, it should use install(DIRECTORY,

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:499
http://jenkins.qa.ubuntu.com/job/mir-ci/104/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/105/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/104/rebuild

review: Needs Fixing (continuous-integration)

[Robert Ancell (robert-ancell)]

Updated, I think this addresses the concerns listed:

Thomas - variable no longer exists
Alan, Robert - whole directory now installed
Daniel - still probably installs headers that are not required, however I agree with Alan here that these headers should not be in include/server if that is the case.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:500
http://jenkins.qa.ubuntu.com/job/mir-ci/105/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/106/console
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/106/artifact/work/output/*zip*/output.zip
    None: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/834/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/105/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

I would expect everything under either include/common/mir or include/server/mir to be part of this.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:500
http://jenkins.qa.ubuntu.com/job/mir-ci/117/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/17/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/118/console
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/118/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/117/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:502
http://jenkins.qa.ubuntu.com/job/mir-ci/135/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/46/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/136/console
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/136/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/135/rebuild

review: Approve (continuous-integration)

=== modified file 'debian/libmirserver-dev.install'
--- debian/libmirserver-dev.install	2013-02-01 11:08:00 +0000
+++ debian/libmirserver-dev.install	2013-03-20 01:29:23 +0000
@@ -1,2 +1,3 @@
+usr/include/mir/server
 usr/lib/pkgconfig/mirserver.pc
 usr/lib/libmirserver.so

=== modified file 'src/server/CMakeLists.txt'
--- src/server/CMakeLists.txt	2013-03-11 17:42:55 +0000
+++ src/server/CMakeLists.txt	2013-03-20 01:29:23 +0000
@@ -9,6 +9,11 @@
 add_subdirectory(frontend/)
 add_subdirectory(shell/)
 
+set(PREFIX "${CMAKE_INSTALL_PREFIX}")
+set(EXEC_PREFIX "${CMAKE_INSTALL_PREFIX}")
+set(LIBDIR "${CMAKE_INSTALL_PREFIX}/lib")
+set(INCLUDEDIR "${CMAKE_INSTALL_PREFIX}/include/mir/server")
+
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/mirserver.pc.in
   ${CMAKE_CURRENT_BINARY_DIR}/mirserver.pc
@@ -62,6 +67,9 @@
   install(TARGETS mirserver
     LIBRARY DESTINATION lib
   )
+  install(DIRECTORY
+    ${CMAKE_SOURCE_DIR}/include/server DESTINATION "include/mir"
+  )
 else()
   target_link_libraries(mirserver
     LINK_PUBLIC