Merge lp:~widelands-dev/widelands/remove_duplicate_code into lp:widelands

[Tino]

Remove duplicate code for revision detection task (code already in BzrRevision.cmake)

[GunChleoc (gunchleoc)]

LGTM :)

@bunnybot merge

review: Approve

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4039. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/432862269.
Appveyor build 3835. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_remove_duplicate_code-3835.

[Tino (tino79)]

OK, the merge was a bit premature:
Revision detection is not broken, but does not happen any longer...

[GunChleoc (gunchleoc)]

I tested this on Linux and got a correct version detection - maybe it's a Windows issue?

[Benedikt Straub (nordfriese)]

Um, the compile script no longer works for me on linux in trunk-8854?

[…]
-- Not using AddressSanitizer.
-- Version of Widelands Build is (Release)

CMake Warning (dev) at /usr/share/cmake-3.10/Modules/CTest.cmake:234 (if):
  Policy CMP0054 is not set: Only interpret if() arguments as variables or
  keywords when unquoted. Run "cmake --help-policy CMP0054" for policy
  details. Use the cmake_policy command to set the policy and suppress this
  warning.

  Quoted variables like "" will no longer be dereferenced when the policy is
  set to NEW. Since the policy is not set the OLD behavior will be used.
Call Stack (most recent call first):
  CMakeLists.txt:254 (include)
This warning is for project developers. Use -Wno-dev to suppress it.

-- Configuring done
-- Generating done
-- Build files have been written to: /home/benedikt/widelands-debug/widelands/build
[67/68] Linking CXX executable src/scripting/test/WL_VERSIONtest_scripting
src/third_party/libthird_party_eris.a(loslib.c.o): In Funktion »os_tmpname«:
loslib.c:(.text+0x467): Warnung: the use of `tmpnam' is dangerous, better use `mkstemp'
[68/68] cd /home/benedikt/widelands-debug/widelands/build && /usr/bin/ctest --output-on-failure
Test project /home/benedikt/widelands-debug/widelands/build
    Start 1: test_economy
1/4 Test #1: test_economy ..................... Passed 0.06 sec
    Start 2: test_io_filesystem
2/4 Test #2: test_io_filesystem ............... Passed 0.03 sec
    Start 3: notifications_test
3/4 Test #3: notifications_test ............... Passed 0.01 sec
    Start 4: test_scripting
4/4 Test #4: test_scripting ................... Passed 0.01 sec

100% tests passed, 0 tests failed out of 4

Total Test time (real) = 0.12 sec
mv: Aufruf von stat für 'src/widelands' nicht möglich: Datei oder Verzeichnis nicht gefunden

[Toni Förster (stonerl)]

The cmake warning is caused by Tino's "Quickfix" at revision 8853 in trunk.

These lines in BzrRevision.cmake trigger it the policy:

string(REGEX REPLACE "\n|\r$" "" WL_VERSION "${WL_VERSION}")
string(STRIP WL_VERSION "${WL_VERSION}")
file (WRITE ${CMAKE_CURRENT_BINARY_DIR}/VERSION "${WL_VERSION}")

See here:

https://cmake.org/cmake/help/v3.10/policy/CMP0054.html

adding this line to CMakeLists.txt solved the error:

cmake_policy(SET CMP0054 NEW)

[Toni Förster (stonerl)]

The version isn't shown on macOS either:

The "RESULT_VARIABLE" says: No such file or directory

So the command does not seem to be executed and therefore WL_Version is empty.

[Toni Förster (stonerl)]

Sorry for the spam. This line needs to be removed:

WORKING_DIRECTORY ${MAKE_CURRENT_SOURCE_DIR}

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2018-08-21 08:14:38 +0000
+++ CMakeLists.txt	2018-09-25 07:48:30 +0000
@@ -246,17 +246,6 @@
     BzrRevision ALL
     COMMAND ${CMAKE_COMMAND} -DWL_INSTALL_BASEDIR=${WL_INSTALL_BASEDIR} -DWL_INSTALL_DATADIR=${WL_INSTALL_DATADIR} -DPYTHON_EXECUTABLE=${PYTHON_EXECUTABLE} -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DSOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR} -DBINARY_DIR=${CMAKE_CURRENT_BINARY_DIR} -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/BzrRevision.cmake
   )
-
-  # Detect version now
-  execute_process (
-    COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/utils/detect_revision.py
-    OUTPUT_VARIABLE WL_VERSION
-    WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
-    OUTPUT_STRIP_TRAILING_WHITESPACE
-  )
-  file (WRITE ${CMAKE_CURRENT_BINARY_DIR}/VERSION "${WL_VERSION}")
-  configure_file (${CMAKE_CURRENT_SOURCE_DIR}/src/build_info.cc.cmake ${CMAKE_CURRENT_BINARY_DIR}/src/build_info.cc)
-  message (STATUS "Version of Widelands Build is ${WL_VERSION}(${CMAKE_BUILD_TYPE})")
 else (NOT DEFINED WL_VERSION)
   add_custom_target (
     InputRevision ALL