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

Fix #7047 #7075

Merged
merged 1 commit into from Feb 25, 2018
Merged

Fix #7047 #7075

merged 1 commit into from Feb 25, 2018

Conversation

nOOb3167
Copy link
Contributor

Fix #7047 by specifying absolute paths for source files within src/CMakeLists.txt (more detail as to why this works in the #7047 comments).
That file was the only place where relative paths were used for source files, therefore after this commit it is now consistent throughout the codebase.

Looking at the source lists, two files were found specified twice (first in common_SRCS, then again in client_SRCS. (client_SRCS includes common_SRCS)): convert_json.cpp and hud.cpp.
These two files are now specified just in common_SRCS.

Alternatively the required CMake version could be bumped past the version exhibiting this bug (3.8.1).

@sfan5
Copy link
Member

sfan5 commented Feb 24, 2018

this looks like lots of unneeded changes, I'd prefer just restricting the source_groups feature to cmake > 3.8.1

@sfan5 sfan5 added @ Build CMake, build scripts, official builds, compiler and linker errors Bugfix 🐛 PRs that fix a bug labels Feb 24, 2018
@nOOb3167
Copy link
Contributor Author

This PR originally contained a more complicated fix (see https://github.com/nOOb3167/minetest/tree/cmakehdr3_old).
It has been updated to the alternative method (source_groups only for CMake past version 3.8.1).

@@ -530,7 +530,7 @@ set(server_SRCS
)
list(SORT server_SRCS)

if ((CMAKE_VERSION VERSION_GREATER 3.8) OR (CMAKE_VERSION VERSION_EQUAL 3.8))
if (CMAKE_VERSION VERSION_GREATER 3.8.1)
Copy link
Member

Choose a reason for hiding this comment

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

maybe add a comment here stating why it's 3.8.1 and not something else

@nOOb3167
Copy link
Contributor Author

The comment now added to the change is formatted after a preexisting case in src/CMakeLists.txt about blacklisting locales (short summary line followed by issue numbers):

# Blacklisted locales that don't work.
# see issue #4638

@SmallJoker SmallJoker merged commit 88a7160 into minetest:master Feb 25, 2018
minduser00 pushed a commit to minduser00/minetest that referenced this pull request Mar 18, 2018
minduser00 pushed a commit to minduser00/minetest that referenced this pull request Mar 18, 2018
@SmallJoker SmallJoker mentioned this pull request Jun 11, 2018
osjc pushed a commit to osjc/minetest that referenced this pull request Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix 🐛 PRs that fix a bug @ Build CMake, build scripts, official builds, compiler and linker errors One approval ✅ ◻️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants