Skip to content
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

CMake: changes needed to build on Linux #452

Merged
merged 1 commit into from
Jun 16, 2016
Merged

CMake: changes needed to build on Linux #452

merged 1 commit into from
Jun 16, 2016

Conversation

ideasman42
Copy link
Collaborator

@ideasman42 ideasman42 commented Jun 15, 2016

Note that these are the main changes from the linux port.
Not a fully working linux port.

Merging this will make updating from master less hassle.

set(TIFF_LIB ${TIFF_LIBRARY})
# the libraries have not .pc nor preferred Find*.cmake. use custom modules.
# find_package(PNG REQUIRED)
set(PNG_LIB ${SDKROOT}/libpng-1.6.21/.libs/libpng16.so)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many distros ship current versions of libpng, can this be changed into a fallback only when system libpng is not found?

Copy link

@J5lx J5lx Jun 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If anything, there should be a static switch for using system libpng IMO. Autodetecting stuff like that and building a software differently based on the result is bad for packaging that software reproducibly (i.e. the packaging process will result in different packages depending on whether libpng is installed on the build system or not. So if some package maintainer is unaware of the autodetection he might create a package without the libpng dependency and someone with system libpng available will end up with a package with incomplete dependency information).

Copy link
Collaborator Author

@ideasman42 ideasman42 Jun 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated it to use systems libpng.

@J5lx, CMake lists libraries used, package maintainers can pass them explicitly to CMake if they don't want to use auto-detection. (CMake is often used this way)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even so I believe it's better to be explicit about dependencies. But I guess it's a somewhat controversial topic after all (though far less controversial than code style or editors 😉) so let's not make mountains out of molehills. I'll just try to find that list of libraries in the future.

Copy link
Collaborator Author

@ideasman42 ideasman42 Jun 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being explicit about dependencies within the CMake files can make bisecting older versions of the software difficult. (unless your project keeps an image of the platform used to build each version - some do).
In my experience its best to store exact configurations in a separate CMake configuration file or build script.

Note that these are the main changes from the linux port.
Not a fully working linux port.
@skitaoka
Copy link
Member

Jenkins, test this.

@skitaoka
Copy link
Member

LGTM

@skitaoka skitaoka merged commit 22fd3b0 into opentoonz:master Jun 16, 2016
@janisozaur janisozaur mentioned this pull request Jun 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants