I Approve of the reasoning and the change. > Am 26.04.2014 um 18:10 schrieb Hans Joachim Desserud <[email protected]>: > > 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_STR ICT}" 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 furth er 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_COMPILE RVERSION_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()) : I ndex_(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 : [email protected] > 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. _______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
