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

Add compatibility to vcpkg buildsystem #8317

Merged
merged 11 commits into from
Jun 10, 2019

Conversation

adrido
Copy link
Contributor

@adrido adrido commented Mar 5, 2019

Vcpkg is a C++ Library Manager for Windows, Linux, and MacOS. https://github.com/Microsoft/vcpkg

It is really helpful because it makes it easy to manage Minetest's load of (optional) dependencies. Especially on Windows.

This PR changes all required things to make cmake+vcpkg work out of the box.
It also updates the documentation and adds instructions how to build on windows.

Here a checklist what's currently possible:

Triplets:
✔ x64-windows
❌ x64-windows-static (link error with irrlicht, openAL, curl and leveldb)
✔ x86-windows
❓ x86-windows-static (needs testing)

Required Features:
✔ build client
✔ build server
✔ irrlicht
✔ sqlite3
✔ zlib

Optional Features:
✔ curl
❓ curses (have no idea what's that)
✔ freetype
❓ gettext (supported by vcpkg but not tested)
❓ gles (probably not possible)
✔ leveldb
✔ luajit
✔ postgresql (changes to Minetest required)
❌ redis (Minetest depends on 'hiredis' which is Unix exclusive)
✔ sound
❓ spatial (there is a libary called libspatiallite)
❓ system-gmp (not supported by vcpkg)
❓ system-jsoncpp (supported by vcpkg but not tested)

It would be nice if someone could follow the documentation and find out if its complete and understandable.

That missing optional features can also added later, so I would consider this PR as ready for review.
This PR already includes #8295
Fixes #7642

Edit 14.03.2019: replaced the task list with "emotes" as it was not really a task list

@SmallJoker SmallJoker added @ Build CMake, build scripts, official builds, compiler and linker errors @ Documentation labels Mar 10, 2019
@SmallJoker
Copy link
Member

Should the old outdated windows build instructions kept or removed?

I think you can remove those. If someone still wants to compile the annoying way, then there's still the same guide on the wiki (maybe add a link to it?).

or schould it stay like it is (lua51.lib) this would make it impossible use it under Linux and OS-X

Guard it with ifs, to keep compatibility as much as possible.

@adrido
Copy link
Contributor Author

adrido commented Mar 10, 2019

Done. Regarding PostgreSQL:

minetest/src/CMakeLists.txt

Lines 161 to 186 in 77961aa

if(ENABLE_POSTGRESQL)
find_program(POSTGRESQL_CONFIG_EXECUTABLE pg_config DOC "pg_config")
find_library(POSTGRESQL_LIBRARY pq)
if(POSTGRESQL_CONFIG_EXECUTABLE)
execute_process(COMMAND ${POSTGRESQL_CONFIG_EXECUTABLE} --includedir-server
OUTPUT_VARIABLE POSTGRESQL_SERVER_INCLUDE_DIRS
OUTPUT_STRIP_TRAILING_WHITESPACE)
execute_process(COMMAND ${POSTGRESQL_CONFIG_EXECUTABLE}
OUTPUT_VARIABLE POSTGRESQL_CLIENT_INCLUDE_DIRS
OUTPUT_STRIP_TRAILING_WHITESPACE)
# This variable is case sensitive for the cmake PostgreSQL module
set(PostgreSQL_ADDITIONAL_SEARCH_PATHS ${POSTGRESQL_SERVER_INCLUDE_DIRS} ${POSTGRESQL_CLIENT_INCLUDE_DIRS})
endif()
find_package("PostgreSQL")
if(POSTGRESQL_FOUND)
set(USE_POSTGRESQL TRUE)
message(STATUS "PostgreSQL backend enabled")
# This variable is case sensitive, don't try to change it to POSTGRESQL_INCLUDE_DIR
message(STATUS "PostgreSQL includes: ${PostgreSQL_INCLUDE_DIR}")
include_directories(${PostgreSQL_INCLUDE_DIR})
else()
message(STATUS "PostgreSQL not found!")
endif()
endif(ENABLE_POSTGRESQL)

BTW is it intentional that
first find_library(POSTGRESQL_LIBRARY pq) is called
and then find_package("PostgreSQL") but at the end linking to POSTGRESQL_LIBRARY??? https://github.com/minetest/minetest/blob/master/src/CMakeLists.txt#L596

in case of vcpkg it looks like this:
find_library(POSTGRESQL_LIBRARY pq) --> POSTGRESQL_LIBRARY-NOTFOUND (its named libpq)
find_package("PostgreSQL") --> found --> sets PostgreSQL_LIBRARY to libpq.lib
target_link_libraries(${PROJECT_NAME} ${POSTGRESQL_LIBRARY}) --> ERROR

@adrido
Copy link
Contributor Author

adrido commented Mar 31, 2019

Rebased.

@adrido
Copy link
Contributor Author

adrido commented Apr 19, 2019

Rebased. As #8435 is merged, PostgreSQL is now working as well.

Copy link
Contributor

@ClobberXD ClobberXD left a comment

Choose a reason for hiding this comment

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

Some minor grammar and spelling corrections in README :)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Compiling on Windows sucks, so this is a good step forward. I tested this PR on Linux, to see whether the build system is still fine. It is, and Travis also confirms that.
👍

@rubenwardy
Copy link
Member

This needs testing to make sure the buildbot script still works, and other ways of compiling on Windows too

@rubenwardy rubenwardy merged commit bd6f1cc into minetest:master Jun 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Build CMake, build scripts, official builds, compiler and linker errors @ Documentation Testing needed >= Two approvals ✅ ✅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce vcpkg for easier development on Windows
4 participants