New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install headers and export cmake package for external projects. #5119

Closed
wants to merge 2 commits into
base: branch-7-0
from

Conversation

Projects
None yet
3 participants
@sebastic
Contributor

sebastic commented Jul 10, 2015

This patch is included in the mapserver Debian package to allow mapcache
to build with mapserver support.

All headers are installed in usr/include/mapserver, and the cmake files
in usr/share/mapserver/cmake.

Install headers and export cmake package for external projects.
This patch is included in the mapserver Debian package to allow mapcache
to build with mapserver support.

All headers are installed in usr/include/mapserver, and the cmake files
in usr/share/mapserver/cmake.
@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Sep 8, 2015

Member

@sebastic OK in principle for integrating the patch, however can you check/fix why the continuous build is failing with it applied?

Member

tbonfort commented Sep 8, 2015

@sebastic OK in principle for integrating the patch, however can you check/fix why the continuous build is failing with it applied?

@sebastic

This comment has been minimized.

Show comment
Hide comment
@sebastic

sebastic Sep 9, 2015

Contributor

I've added the missing CMake configuration files to fix this incomplete PR.

Contributor

sebastic commented Sep 9, 2015

I've added the missing CMake configuration files to fix this incomplete PR.

tbonfort added a commit that referenced this pull request Sep 10, 2015

Install headers and export cmake package for external projects (#5119)
This patch is included in the mapserver Debian package to allow mapcache
to build with mapserver support.

All headers are installed in usr/include/mapserver, and the cmake files
in usr/share/mapserver/cmake.
@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Sep 10, 2015

Member

applied to branch-7-0 in 2549b43 , thanks @sebastic

Member

tbonfort commented Sep 10, 2015

applied to branch-7-0 in 2549b43 , thanks @sebastic

@tbonfort tbonfort closed this Sep 10, 2015

@sebastic sebastic deleted the sebastic:cmake-mapserver-export branch Sep 10, 2015

@rouault

This comment has been minimized.

Show comment
Hide comment
@rouault

rouault Sep 12, 2015

Contributor

@sebastic I've finally managed to enable the AppVeyor buils and launched a build of master and it errors out with :

CMake Error at CMakeLists.txt:977 (INSTALL):
install Library TARGETS given no DESTINATION!

( https://ci.appveyor.com/project/MapServer/mapserver/build/job/h53c6gkmn6v5pktu )

Any idea how to fix that ? (I assume this is linked to the CMake changes of this ticket)

The previous build (one month ago) succeeded : https://ci.appveyor.com/project/MapServer/mapserver/build/1.0.1/job/7vk28s03xxy0qsle

Contributor

rouault commented Sep 12, 2015

@sebastic I've finally managed to enable the AppVeyor buils and launched a build of master and it errors out with :

CMake Error at CMakeLists.txt:977 (INSTALL):
install Library TARGETS given no DESTINATION!

( https://ci.appveyor.com/project/MapServer/mapserver/build/job/h53c6gkmn6v5pktu )

Any idea how to fix that ? (I assume this is linked to the CMake changes of this ticket)

The previous build (one month ago) succeeded : https://ci.appveyor.com/project/MapServer/mapserver/build/1.0.1/job/7vk28s03xxy0qsle

@sebastic

This comment has been minimized.

Show comment
Hide comment
@sebastic

sebastic Sep 12, 2015

Contributor

Looks like Windows needs an ARCHIVE install target too (see: [CMake] Install path problem on MinGW)

Can you try commit f86ccbd from my fork?

Contributor

sebastic commented Sep 12, 2015

Looks like Windows needs an ARCHIVE install target too (see: [CMake] Install path problem on MinGW)

Can you try commit f86ccbd from my fork?

@rouault

This comment has been minimized.

Show comment
Hide comment
@rouault

rouault Sep 12, 2015

Contributor

@sebastic Can you submit it as a PR ? It should trigger AppVeyor

Contributor

rouault commented Sep 12, 2015

@sebastic Can you submit it as a PR ? It should trigger AppVeyor

@sebastic

This comment has been minimized.

Show comment
Hide comment
@sebastic

sebastic Sep 12, 2015

Contributor

I can, and will, after I get the results from the Debian package build in which I've included that change for testing. If installing the archive target in the shlib component has adverse effects on the Linux builds I'll need to change the component first.

Contributor

sebastic commented Sep 12, 2015

I can, and will, after I get the results from the Debian package build in which I've included that change for testing. If installing the archive target in the shlib component has adverse effects on the Linux builds I'll need to change the component first.

@rouault

This comment has been minimized.

Show comment
Hide comment
@rouault

rouault Sep 12, 2015

Contributor

@sebastic Oh I just figured out I can myself create the PR : #5164 (perhaps because I've been given admin rights on mapserver repo, I dunno)

Contributor

rouault commented Sep 12, 2015

@sebastic Oh I just figured out I can myself create the PR : #5164 (perhaps because I've been given admin rights on mapserver repo, I dunno)

@rouault

This comment has been minimized.

Show comment
Hide comment
@sebastic

This comment has been minimized.

Show comment
Hide comment
@sebastic

sebastic Sep 12, 2015

Contributor

Looking good on Linux too, although I noticed that the headers are now installed in /usr/lib too, that's a separate issue to fix.

Contributor

sebastic commented Sep 12, 2015

Looking good on Linux too, although I noticed that the headers are now installed in /usr/lib too, that's a separate issue to fix.

@sebastic

This comment has been minimized.

Show comment
Hide comment
@sebastic

sebastic Sep 12, 2015

Contributor

The fix for the header installation in /usr/lib is available in #5165, it also includes the ARCHIVE target change from #5164.

Contributor

sebastic commented Sep 12, 2015

The fix for the header installation in /usr/lib is available in #5165, it also includes the ARCHIVE target change from #5164.

@rouault

This comment has been minimized.

Show comment
Hide comment
@rouault

rouault Sep 14, 2015

Contributor

@sebastic Do you know which branches this applies ? master only or also 7.0 ?

Contributor

rouault commented Sep 14, 2015

@sebastic Do you know which branches this applies ? master only or also 7.0 ?

@sebastic

This comment has been minimized.

Show comment
Hide comment
@sebastic

sebastic Sep 14, 2015

Contributor

My PR is for master, but the change should be backported to branch-7-0 too.

Both master & branch-7-0 have the same issue (installing the headers in /usr/lib too).

Contributor

sebastic commented Sep 14, 2015

My PR is for master, but the change should be backported to branch-7-0 too.

Both master & branch-7-0 have the same issue (installing the headers in /usr/lib too).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment