widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

I Approve of the reasoning and the change. 

> Am 26.04.2014 um 18:10 schrieb Hans Joachim Desserud <launchpad@xxxxxxxxxxxx>:
> 
> Hans Joachim Desserud has proposed merging lp:~hjd/widelands/remove-all-patches into lp:~widelands-dev/widelands/debian.
> 
> Requested reviews:
>  Widelands Developers (widelands-dev)
> 
> For more details, see:
> https://code.launchpad.net/~hjd/widelands/remove-all-patches/+merge/217341
> 
> (Follow-up from https://code.launchpad.net/~hjd/widelands/disabled-s390-patch/+merge/217293)
> 
>> I looked through this and I wondered why you did not also delete the patch file.
> 
> At the moment I wanted to do the minimal thing to get the builds working again. I also wasn't sure whether to remove it right away or to keep it around as a reminder that this was something we might wanted to consider fixing in trunk.
> 
> I also wish to keep the changes between our packaging and upstream's to a minimum, for easier merging/syncing of changes from Debian.
> 
> I've thought a bit more about it, and I don't see a lot of value gained from these patches. Most of them fix issues (ftbfs or otherwise) on obscure architectures which the PPA doesn't even have available as build targets. Furthermore, they are based on build18, and with ongoing source changes probably only a matter of time before they fail to apply cleanly. 
> 
> Simply skipping the whole patch directory would ensure that the build doesn't suddenly break because certain files have changed and patches cannot be applied, and it makes it uncomplicated to merge new versions if we simply remove those files either way. (I do think new patches should be looked at and considered for inclusion in trunk, but I believe this has already been done for the patches attached to the build18 packaging.)
> 
> (Proof things still builds after removing the patches directory https://code.launchpad.net/~hjd/+recipe/widelands-test, built on Quantal and Utopic)
> -- 
> https://code.launchpad.net/~hjd/widelands/remove-all-patches/+merge/217341
> Your team Widelands Developers is requested to review the proposed merge of lp:~hjd/widelands/remove-all-patches into lp:~widelands-dev/widelands/debian.
> === removed directory 'debian/patches' === removed file 'debian/patches/dbg_symbols' --- debian/patches/dbg_symbols	2014-03-11 20:57:31 +0000 +++ debian/patches/dbg_symbols	1970-01-01 00:00:00 +0000 @@ -1,20 +0,0 @@ -Description: compile with -g so that the dbg package contains something -Forwarded-Upstream: n/a (upstream strips dbg symbols on purpose from releases) - ---- - CMakeLists.txt | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -Index: b/CMakeLists.txt -=================================================================== ---- a/CMakeLists.txt -+++ b/CMakeLists.txt -@@ -273,7 +273,7 @@ - ENDIF (WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.1" OR WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.2") - ENDIF (CMAKE_COMPILER_IS_GNUCXX) - --set (CMAKE_CXX_FLAGS_RELEASE "${WL_COMPILERFLAG_CXXSTD} ${WL_COMPILERFLAG_OPTIMIZATIONS} -DNDEBUG${WL_COMPILERFLAG_OLDSTYLECAST}${WL_COMPILERFLAG_GENERICWARNINGS}${WL_COMPILERFLAG_EXTRAWARNINGS}${WL_COMPILERFLAG_GCCWARNINGS}${WL_COMPILERFLAG_STRICT}" CACHE STRING "Set by widelands CMakeLists.txt" FORCE) -+set (CMAKE_CXX_FLAGS_RELEASE "${WL_COMPILERFLAG_CXXSTD} -g ${WL_COMPILERFLAG_OPTIMIZATIONS} -DNDEBUG${WL_COMPILERFLAG_OLDSTYLECAST}${WL_COMPILERFLAG_GENERICWARNINGS}${WL_COMPILERFLAG_EXTRAWARNINGS}${WL_COMPILERFLAG_GCCWARNINGS}${WL_COMPILERFLAG_STRICT}" CACHE STRING "Set by widelands CMakeLists.txt" FORCE) - - #If building with MSVC, then check for 3rdparty libs - if (DEFINED MSVC) === removed file 'debian/patches/hurd_PATH_MAX_missing' --- debian/patches/hurd_PATH_MAX_missing	2014-03-11 20:57:31 +0000 +++ debian/patches/hurd_PATH_MAX_missing	1970-01-01 00:00:00 +0000 @@ -1,39 +0,0 @@ -Author: Enrico Tassi -Description: define PATH_MAX if not defined (i.e. on hurd) -Forwarded-Upstream: should be - ---- - src/io/filesystem/filesystem.cc | 4 ++++ - src/wlapplication.cc | 4 ++++ - 2 files changed, 8 insertions(+) - -Index: b/src/io/filesystem/filesystem.cc -=================================================================== ---- a/src/io/filesystem/filesystem.cc -+++ b/src/io/filesystem/filesystem.cc -@@ -60,6 +60,10 @@ - #define PATH_MAX MAX_PATH - #endif - -+#ifndef PATH_MAX /* This happens, for example on the Hurd architecture */ -+ #define PATH_MAX 1024 -+#endif -+ - FileSystem::FileSystem() - { - m_root = ""; -Index: b/src/wlapplication.cc -=================================================================== ---- a/src/wlapplication.cc -+++ b/src/wlapplication.cc -@@ -92,6 +92,10 @@ - #endif - #endif - -+#ifndef PATH_MAX /* This happens, for example on the Hurd architecture */ -+ #define PATH_MAX 1024 -+#endif -+ - #define MINIMUM_DISK_SPACE 250000000lu - #define SCREENSHOT_DIR "screenshots" -   === removed file 'debian/patches/mips_gcc_ICE_with-03' --- debian/patches/mips_gcc_ICE_with-03	2014-03-11 20:57:31 +0000 +++ debian/patches/mips_gcc_ICE_with-03	1970-01-01 00:00:00 +0000 @@ -1,31 +0,0 @@ -Description: Set compilation level to -02 on mips (it's -03 by default upstream) - . - Optimizing further drives gcc nuts and produce ICE. - . - https://buildd.debian.org/status/fetch.php?pkg=widelands&arch=mips&ver=1%3A18-1&stamp=1393243179 -Author: Martin Quinson -Forwarded-Upstream: should be - ---- - CMakeLists.txt | 6 ++++-- - 1 file changed, 4 insertions(+), 2 deletions(-) - -Index: b/CMakeLists.txt -=================================================================== ---- a/CMakeLists.txt -+++ b/CMakeLists.txt -@@ -264,10 +264,12 @@ - OUTPUT_VARIABLE WLBUILD_COMPILERVERSION - ) -   STRING(REGEX REPLACE ".*(4)\\.(5)\\.([0-9]).*" "\\1.\\2.\\3" WLBUILD_COMPILERVERSION_REP ${WLBUILD_COMPILERVERSION}) -- IF (WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.1" OR WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.2") -- message("Detected gcc ${WLBUILD_COMPILERVERSION_REP}") -+ IF (WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.1" OR WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.2" OR PROCESSOR_ARCHITECTURE STREQUAL "mips64" OR PROCESSOR_ARCHITECTURE STREQUAL "mips") -+ message("Detected gcc ${WLBUILD_COMPILERVERSION_REP} on ${PROCESSOR_ARCHITECTURE}") - message("Suffering from gcc bug, disabling -O3") - set (WL_COMPILERFLAG_OPTIMIZATIONS "-O2") -+ else (WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.1" OR WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.2" OR PROCESSOR_ARCHITECTURE STREQUAL "mips64" OR PROCESSOR_ARCHITECTURE STREQUAL "mips") -+ message("Detected architecture: ${PROCESSOR_ARCHITECTURE}") - ENDIF (WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.1" OR WLBUILD_COMPILERVERSION_REP STREQUAL "4.5.2") - ENDIF (CMAKE_COMPILER_IS_GNUCXX) - === removed file 'debian/patches/s390_new_architecture' --- debian/patches/s390_new_architecture	2014-03-11 20:57:31 +0000 +++ debian/patches/s390_new_architecture	1970-01-01 00:00:00 +0000 @@ -1,52 +0,0 @@ -Description: Port the game to the s390 architecture -Forwarded-Upstream: should be - ---- - src/logic/widelands.h | 20 ++++++++++++++++++++ - 1 file changed, 20 insertions(+) - -Index: b/src/logic/widelands.h -=================================================================== ---- a/src/logic/widelands.h -+++ b/src/logic/widelands.h -@@ -78,6 +78,13 @@ - { - assert(I < std::numeric_limits::max()); - } -+#if (defined(__s390__) && !defined(__s390x__)) -+	explicit _Index(uintptr_t const I) -+	 : i(static_cast(I)) -+	{ -+	 assert(I < std::numeric_limits::max()); -+	} -+#endif - - /// For compatibility with old code that use int32_t for building index - /// and use -1 to indicate invalidity. -@@ -116,6 +123,17 @@ - value_t i; - }; - -+#if (defined(__s390__) && !defined(__s390x__)) -+#define DEFINE_INDEX(NAME) \ -+ struct NAME : public _Index { \ -+ NAME(NAME const & other = Null()) : _Index(other) {} \ -+ explicit NAME(value_t const I) : _Index(I) {} \ -+ explicit NAME(size_t const I) : _Index(I) {} \ -+ explicit NAME(int32_t const I) __attribute__((deprecated)); \ -+ explicit NAME(uintptr_t const I) : _Index(I) {} \ -+ }; \ -+ -+#else - #define DEFINE_INDEX(NAME) \ - struct NAME : public Index_ { \ - NAME(const NAME & other = Null()) : Index_(other) {} \ -@@ -124,6 +142,8 @@ - explicit NAME(int32_t const I) __attribute__((deprecated)); \ - }; \ - -+#endif /* s390 architecture */ -+ - DEFINE_INDEX(Building_Index) - DEFINE_INDEX(Ware_Index) - === removed file 'debian/patches/series' --- debian/patches/series	2014-04-25 17:15:44 +0000 +++ debian/patches/series	1970-01-01 00:00:00 +0000 @@ -1,3 +0,0 @@ -mips_gcc_ICE_with-03 -hurd_PATH_MAX_missing -dbg_symbols
> _______________________________________________
> Mailing list: https://launchpad.net/~widelands-dev
> Post to     : widelands-dev@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~widelands-dev
> More help   : https://help.launchpad.net/ListHelp

-- 
https://code.launchpad.net/~hjd/widelands/remove-all-patches/+merge/217341
Your team Widelands Developers is requested to review the proposed merge of lp:~hjd/widelands/remove-all-patches into lp:~widelands-dev/widelands/debian.