Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Le samedi 26 décembre 2015, 22:46:05 Markus Koschany a écrit : > The package looks good to me. Please add the missing license of tinyxml > to debian/copyright. Done > After that I will upload the package. Thanks, that would be nice to finally close this ITP from 2011. signature.asc Description: This is a digitally signed message part.
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Am 27.12.2015 um 09:35 schrieb Alexandre Detiste: > Le samedi 26 décembre 2015, 22:46:05 Markus Koschany a écrit : >> The package looks good to me. Please add the missing license of tinyxml >> to debian/copyright. > > Done Uploaded. Thanks for your contribution. Markus signature.asc Description: OpenPGP digital signature
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Hi, I spent some time merging your advices & Simon's ones [1] in a way that don't trigger new lintian false positives. [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=807099#74 > Am 11.12.2015 um 12:35 schrieb Alexandre Detiste: > I would add the following paragraph to debian/copyright to avoid any > confusion. > > License: GPL-3+-with-exception > On Debian systems, the complete text of the GNU General Public License > version 3 can be found in `/usr/share/common-licenses/GPL-3'. > . > As a special exception, > > Bonus points if these files are generated automatically during the build. I'll see for the next release if that's usefull; now this part of the code is not used anyway. I sent a pull request for this lintian advice: "I: corsix-th: spelling-error-in-binary usr/lib/games/corsix-th/CorsixTH occured occurred" I feel this problem isn't enought important to warrant a patch for 0.50. The patch for the segfault I found in Arch packaging is well there. Greets & best wishes, Alexandre signature.asc Description: This is a digitally signed message part.
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Am 26.12.2015 um 10:30 schrieb Alexandre Detiste: > Hi, > > I spent some time merging your advices & Simon's ones [1] > in a way that don't trigger new lintian false positives. The package looks good to me. Please add the missing license of tinyxml to debian/copyright. After that I will upload the package. Cheers, Markus signature.asc Description: OpenPGP digital signature
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Le dimanche 6 décembre 2015, 20:34:42 Markus Koschany a écrit : > Several Expat copyright holder are missing in > CorsixTH/Lua/languages > CorsixTH/Src/jit_opt.h > SpriteEncoder/* > LevelEdit/* > > GPL-3+ > SpriteEncoder/parser.cpp > SpriteEncoder/tokens.h This license has this special exception; does this mean it can be considered Expat-licensed as the whole game ? As a special exception, you may create a larger work that contains part or all of the Bison parser skeleton and distribute that work under terms of your choice > > zlib: > WindowsInstaller/ReplaceInFile.nsh signature.asc Description: This is a digitally signed message part.
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Am 11.12.2015 um 12:35 schrieb Alexandre Detiste: > Le dimanche 6 décembre 2015, 20:34:42 Markus Koschany a écrit : >> Several Expat copyright holder are missing in >> CorsixTH/Lua/languages >> CorsixTH/Src/jit_opt.h >> SpriteEncoder/* >> LevelEdit/* >> >> GPL-3+ >> SpriteEncoder/parser.cpp >> SpriteEncoder/tokens.h > > This license has this special exception; does this mean > it can be considered Expat-licensed as the whole game ? > >As a special exception, you may create a larger work that contains >part or all of the Bison parser skeleton and distribute that work >under terms of your choice I would add the following paragraph to debian/copyright to avoid any confusion. License: GPL-3+-with-exception On Debian systems, the complete text of the GNU General Public License version 3 can be found in `/usr/share/common-licenses/GPL-3'. . As a special exception, you may create a larger work that contains part or all of the Bison parser skeleton and distribute that work under terms of your choice, so long as that work isn't itself a parser generator using the skeleton or a modified version thereof as a parser skeleton. Alternatively, if you modify or redistribute the parser skeleton itself, you may (at your option) remove this special exception, which will cause the skeleton and the resulting Bison output files to be licensed under the GNU General Public License without this special exception. Bonus points if these files are generated automatically during the build. Markus signature.asc Description: OpenPGP digital signature
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Le mardi 8 décembre 2015, 12:32:33 Paul Wise a écrit : > On Mon, Dec 7, 2015 at 8:36 PM, Alexandre Detiste wrote: > > > There's one library missing: "lua-socket", which is solely used > > for a call-home / breach of privacy anti-feature. > > > > I would disable it at build-time with sed. > > (beacause lua-socket might have been pulled > > by an other package) > ... > > print "Checking for CorsixTH updates..." > > local update_body, status, headers = http.request(update_url) > > I don't know that it is fair to call it that, it is only checking for > updates. Obviously doing that is pointless for users of a Debian > package, but it would be helpful for users of platforms without a sane > update system. I didn't wanted to sound harsh but I just didn't thought of any other name for this. The lintian tags are named privacy-breach-* as well.
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Hi, Thanks for having a look at this. > so are you saying that the package is doing some dlopen-hack to use the > libraries? > > because otherwise the package needs to link the libraries, and then shlibs > should > do the subst work. > > (note: I didn't clone the repository, but in my opinion the cleaner way > should be to find, > link and use them, not to find them at runtime if available and dlopen them, > but I might be > wrong, because I don't know the context) The context is that this is game is mainly written in Lua + some C++ glue code and I've no idea how Lua libraries work. So I'll spend some time to learn that properly. This lua-lpeg depedency is there because I was until recently a user of the .deb provided by GetDeb and I noticed after an update that the game was spewing a runtime error about this missing lib; installing lua-lpeg solved the problem I told GetDeb maintainer to add this to the package depedencies, without explicitely stating weither this should go in Build-Dep or Dep. And then I started from this list of depedencies to build my package. > cheers, > > G.
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Hi again! >The context is that this is game is mainly written in Lua + >some C++ glue code and I've no idea how Lua libraries work. > >So I'll spend some time to learn that properly. oh well, lua might not link libraries to binaries :) >This lua-lpeg depedency is there because I was until recently a user >of the .deb provided by GetDeb and I noticed after an update >that the game was spewing a runtime error about this missing lib; >installing lua-lpeg solved the problem > >I told GetDeb maintainer to add this to the package depedencies, >without explicitely stating weither this should go in Build-Dep or Dep. > >And then I started from this list of depedencies to build my package. I guess the libraries are somewhat opened in the lines below "local lfs = require "lfs"" <<- lua-filesystem "local lpeg = require "lpeg"" <<- lua-lpeg but I don't think you are actually opening the sofiles but the .lua version http://www.lua.org/pil/8.1.html (require keyword) also please check the "luaL_openlibs" function in the source code. there are many "require" in the code, I would check all of them :) cheers, G.
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Le lundi 7 décembre 2015, 12:06:55 Gianfranco Costamagna a écrit : > > I guess the libraries are somewhat opened in the lines below > "local lfs = require "lfs"" <<- lua-filesystem > "local lpeg = require "lpeg"" <<- lua-lpeg > > but I don't think you are actually opening the sofiles but the .lua version > http://www.lua.org/pil/8.1.html > (require keyword) > > also please check the "luaL_openlibs" function in the source code. > > there are many "require" in the code, I would check all of them :) I trust what I know: strace. There's one library missing: "lua-socket", which is solely used for a call-home / breach of privacy anti-feature. I would disable it at build-time with sed. (beacause lua-socket might have been pulled by an other package) CorsixTH/Lua/config_finder.lua -- Create config.txt if it doesn't exist local config_defaults = { check_for_updates = true } strace -e open -F corsix-th 2>&1 | grep /usr/lib | cut -f2 -d\" | sort -u | grep lua /usr/lib/lua/5.2/socket.so /usr/lib/x86_64-linux-gnu/liblua5.2.so.0 /usr/lib/x86_64-linux-gnu/lua/5.2/lfs.so /usr/lib/x86_64-linux-gnu/lua/5.2/lpeg.so /usr/lib/x86_64-linux-gnu/lua/5.2/socket.so LANG=C dpkg -S $(strace -e open -F corsix-th 2>&1 | grep /usr/lib | cut -f2 -d\" | sort -u | grep lua) dpkg-query: no path found matching pattern /usr/lib/lua/5.2/socket.so liblua5.2-0:amd64: /usr/lib/x86_64-linux-gnu/liblua5.2.so.0 lua-filesystem:amd64: /usr/lib/x86_64-linux-gnu/lua/5.2/lfs.so lua-lpeg:amd64: /usr/lib/x86_64-linux-gnu/lua/5.2/lpeg.so dpkg-query: no path found matching pattern /usr/lib/x86_64-linux-gnu/lua/5.2/socket.so - CorsixTH/Lua/app.lua local success, socket = pcall(require, "socket") if not success then -- LuaSocket is not available, just return print "Cannot check for updates since LuaSocket is not available." return else self.lua_socket_available = true end local http = require "socket.http" local url = require "socket.url" print "Checking for CorsixTH updates..." local update_body, status, headers = http.request(update_url)
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
On Mon, Dec 7, 2015 at 8:36 PM, Alexandre Detiste wrote: > There's one library missing: "lua-socket", which is solely used > for a call-home / breach of privacy anti-feature. > > I would disable it at build-time with sed. > (beacause lua-socket might have been pulled > by an other package) ... > print "Checking for CorsixTH updates..." > local update_body, status, headers = http.request(update_url) I don't know that it is fair to call it that, it is only checking for updates. Obviously doing that is pointless for users of a Debian package, but it would be helpful for users of platforms without a sane update system. -- bye, pabs https://wiki.debian.org/PaulWise
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Le dimanche 6 décembre 2015 21:28:35, vous avez écrit : > I saw that they provided a desktop file within the DebianPackager > directory. desktop files should definitely be provided with the upstream > sources because they are not Debian specific. Other distributions would > also benefit from this change. A manpage would be usefull too :-) I merged mine with the one writen back then by Chris Butler. I can't even find how the arguments are parsed in the program source code. --- I've copied a segfault-preventing patch from Arch Linux too. > >> Please ask upstream to remove the embedded copy > > That's downstream work. > > No, it is not. Please point them to > https://wiki.debian.org/UpstreamGuide#No_inclusion_of_third_party_code I guess they just don't care about this; better start winning some karma points by providing a man page + .desktop then this will be the next step. Greets, Alexandre signature.asc Description: This is a digitally signed message part.
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Control: owner -1 ! Am 05.12.2015 um 11:55 schrieb Alexandre Detiste: > Package: sponsorship-requests > Severity: wishlist > > Dear mentors, > > I am looking for a sponsor for my package "corsix-th": Hi Alexandre, here is my initial review. I am working with the pkg-games Git repository of corsix-th and I suggest we continue to use it instead of the mentors.debian.net packages. debian/control: You build-depend on wx2.8-headers but this package is obsolete and will be removed soon. Please either use wx3.0-headers instead or remove the build-dependency because I don't see anything in the sources that may need it. Why do you depend on lua-filesystem and lua-lpeg explicitly? If those dependencies are really required, the ${shlibs:Depends} substvar should include them already. Otherwise the build system should be updated to require and incorporate those libraries. desktop file: I have already fixed a few things that bothered me. I believe it's a simulation game and StartupNotify should be set to false. I also added keywords and a comment in German. Please tell me if you can live with the changes or if there is a reason to stick with the old values. Please forward the new desktop file upstream. Please use Debian's system library of tinyxml. The embedded copy of tinyxml in AnimView/ is neither mentioned in upstream's LICENSE.txt file nor in debian/copyright. Please ask upstream to remove the embedded copy or to offer a build system option to use the system library instead. They should also mention the copyright holders and license in LICENSE.txt. You can take a look at my Bullet package how I used Debian's tinyxml library. There are several issues with debian/copyright: Some copyright holders and licenses are missing. BSD-3-clause CMake/CMakeFFmpegLibavMacros.cmake CMake/FindFFmpeg.cmake CMake/FindLibAV.cmake CMake/FindSDL2.cmake CMake/FindSDL2_mixer.cmake public-domain CMake/FindDirectX.cmake CMake/FindLua.cmake CMake/FindPkgMacros.cmake Several Expat copyright holder are missing in CorsixTH/Lua/languages CorsixTH/Src/jit_opt.h SpriteEncoder/* LevelEdit/* GPL-3+ SpriteEncoder/parser.cpp SpriteEncoder/tokens.h zlib: WindowsInstaller/ReplaceInFile.nsh Why don't you build the level editor? Wouldn't this be a useful addition to the game? Lintian error: source-contains-unsafe-symlink I think it is safe to override this error and since upstream already removed the debian directory from Git master it will be fixed with the next release of corsix-th. Another option might be to remove the directory for the current version and to append +ds to the upstream version. Forwarding the Lua patch upstream is a good idea. I would change usr/lib/games/corsix-th/CorsixTH to /usr/lib/corsix-th/CorsixTH. We still use /usr/games and /usr/share/games for historical reasons but I think it is OK to omit the games subdirectory here. debian/scripts/corsix-th Please use the exec command. It replaces the current process without forking a new process. That's it for now. Regards, Markus signature.asc Description: OpenPGP digital signature
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Le dimanche 6 décembre 2015, 20:34:42 Markus Koschany a écrit : > Hi Alexandre, > > here is my initial review. I am working with the pkg-games Git > repository of corsix-th and I suggest we continue to use it instead of > the mentors.debian.net packages. Ok > debian/control: > > You build-depend on wx2.8-headers but this package is obsolete and will > be removed soon. Please either use wx3.0-headers instead or remove the > build-dependency because I don't see anything in the sources that may > need it. find . -type f -exec grep ^#include {} \; | grep wx | sort -u #include #include #include #include #include #include #include #include #include #include #include #include #include #include ... It's used in MapEdit/ (currently disabled) and AnimView/ I need to check if this second direcotry is a depedency of MapEdit only or if it is used somewhere else. > Why do you depend on lua-filesystem and lua-lpeg explicitly? If those > dependencies are really required, the ${shlibs:Depends} substvar should > include them already. Otherwise the build system should be updated to > require and incorporate those libraries. Because they are only needed at runtime... does they really need to get pulled in the build chroot for nothing ? I guess it's cleaner that way. > desktop file: I have already fixed a few things that bothered me. I > believe it's a simulation game and StartupNotify should be set to false. > I also added keywords and a comment in German. Please tell me if you can > live with the changes or if there is a reason to stick with the old > values. Please forward the new desktop file upstream. Upstream doesn't ship a .png icon either and had removed the non-maintained DebianPackager... It's not the kind of thing I really want to bother an upstream with; ut rather more usefull things, like having appropriate configure scripts to avoid the need to patch the build system. > Please use Debian's system library of tinyxml. The embedded copy of > tinyxml in AnimView/ is neither mentioned in upstream's LICENSE.txt file > nor in debian/copyright. As stated for WX, I need to check if AnimView is needed. > Please ask upstream to remove the embedded copy That's downstream work. > or to offer a build system option to use the system library instead. I'll come with a P.R if the situation is still applicable for current git master. > They should also mention the copyright holders and license in > LICENSE.txt. You can take a look at my Bullet package how I used > Debian's tinyxml library. Ok > There are several issues with debian/copyright: > Some copyright holders and licenses are missing. > > BSD-3-clause > CMake/CMakeFFmpegLibavMacros.cmake > CMake/FindFFmpeg.cmake > CMake/FindLibAV.cmake > CMake/FindSDL2.cmake > CMake/FindSDL2_mixer.cmake > > public-domain > CMake/FindDirectX.cmake > CMake/FindLua.cmake > CMake/FindPkgMacros.cmake Oops, I just assumed all these *.cmake files were autogenerated ... > Several Expat copyright holder are missing in > CorsixTH/Lua/languages > CorsixTH/Src/jit_opt.h > SpriteEncoder/* > SpriteEncoder/parser.cpp > SpriteEncoder/tokens.h This is a small external tool not used during the build. Well, the debian/copyright must cover the whole source package. > LevelEdit/* > > zlib: > WindowsInstaller/ReplaceInFile.nsh > Why don't you build the level editor? Wouldn't this be a useful addition > to the game? It is disabled by default in the 0.50 release: From the release notes: "The Map Editor is not included in 0.50 due to incompatibilities with the engine change. We hope to have a new improved map editor available in a future release. In the mean time we suggest that level designers copy their full 0.40 install to another location before installing 0.50." > Lintian error: source-contains-unsafe-symlink > > I think it is safe to override this error and since upstream already > removed the debian directory from Git master it will be fixed with the > next release of corsix-th. Another option might be to remove the > directory for the current version and to append +ds to the upstream version. I prefer avoiding repacking if I don't absolutely have to (example: in case of non DFSG assets), here it's about 1 lintian override + document some non needed files in d/copyright, so I'll keep the original tarball. > Forwarding the Lua patch upstream is a good idea. Well, upstream is already aware of the issue & had planned to fix it in another way; https://github.com/CorsixTH/CorsixTH/pull/899 I've added a "Forwarded: not needed, see https://github.com/CorsixTH/CorsixTH/pull/899; DEP-3 tag to the patch. > I would change usr/lib/games/corsix-th/CorsixTH to > /usr/lib/corsix-th/CorsixTH. We still use /usr/games and > /usr/share/games for historical reasons but I think it is OK to omit the > games subdirectory here. Will have a look > debian/scripts/corsix-th > > Please use the exec command. It replaces the current process without > forking a new process.
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Am 06.12.2015 um 21:13 schrieb Alexandre Detiste: > Le dimanche 6 décembre 2015, 20:34:42 Markus Koschany a écrit : [...] >> You build-depend on wx2.8-headers but this package is obsolete and will >> be removed soon. Please either use wx3.0-headers instead or remove the >> build-dependency because I don't see anything in the sources that may >> need it. > > find . -type f -exec grep ^#include {} \; | grep wx | sort -u > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > ... > > It's used in MapEdit/ (currently disabled) and AnimView/ > I need to check if this second direcotry is a depedency of MapEdit only or > if it is used somewhere else. I have just rebuilt the package without the B-D on wx2.8-headers and it still works. >> Why do you depend on lua-filesystem and lua-lpeg explicitly? If those >> dependencies are really required, the ${shlibs:Depends} substvar should >> include them already. Otherwise the build system should be updated to >> require and incorporate those libraries. > > Because they are only needed at runtime... > does they really need to get pulled in the build chroot for nothing ? > I guess it's cleaner that way. No, if they are only needed at runtime, so be it. >> desktop file: I have already fixed a few things that bothered me. I >> believe it's a simulation game and StartupNotify should be set to false. >> I also added keywords and a comment in German. Please tell me if you can >> live with the changes or if there is a reason to stick with the old >> values. Please forward the new desktop file upstream. > > Upstream doesn't ship a .png icon either and had removed > the non-maintained DebianPackager... > > It's not the kind of thing I really want to bother an upstream with; > ut rather more usefull things, like having appropriate configure > scripts to avoid the need to patch the build system. I saw that they provided a desktop file within the DebianPackager directory. desktop files should definitely be provided with the upstream sources because they are not Debian specific. Other distributions would also benefit from this change. > >> Please use Debian's system library of tinyxml. The embedded copy of >> tinyxml in AnimView/ is neither mentioned in upstream's LICENSE.txt file >> nor in debian/copyright. > > As stated for WX, I need to check if AnimView is needed. > >> Please ask upstream to remove the embedded copy > That's downstream work. No, it is not. Please point them to https://wiki.debian.org/UpstreamGuide#No_inclusion_of_third_party_code Markus signature.asc Description: OpenPGP digital signature
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Hi, quick question: >> Why do you depend on lua-filesystem and lua-lpeg explicitly? If those >> dependencies are really required, the ${shlibs:Depends} substvar should >> include them already. Otherwise the build system should be updated to >> require and incorporate those libraries. > >Because they are only needed at runtime... >does they really need to get pulled in the build chroot for nothing ? >I guess it's cleaner that way. so are you saying that the package is doing some dlopen-hack to use the libraries? because otherwise the package needs to link the libraries, and then shlibs should do the subst work. (note: I didn't clone the repository, but in my opinion the cleaner way should be to find, link and use them, not to find them at runtime if available and dlopen them, but I might be wrong, because I don't know the context) cheers, G.
Bug#807099: RFS: corsix-th/0.50-1 ITP 610087 - A Theme Hospital engine reimplementation.
Package: sponsorship-requests Severity: wishlist Dear mentors, I am looking for a sponsor for my package "corsix-th": * Package name: corsix-th Version : 0.50-1 Upstream Author : Peter "Corsix" Cawley * URL : https://github.com/CorsixTH/CorsixTH/ * License : GPL-3 Section : contrib/games It builds those binary packages: corsix-th - the engine corsix-th-data - platform independant the Lua Scripts To access further information about this package, please visit the following URLs: http://anonscm.debian.org/cgit/pkg-games/corsix-th.git/ I didn't succeeded into creating the source package. The binary package ("debuild -us -uc -b") is fine though and is a 1:1 replacement for the one from GetDeb. (this package _does_ work fine too, even if FrankenDebian is a bad idea; that's why I'm packaging this) dpkg-source: info: using source format '3.0 (quilt)' dpkg-source: info: building corsix-th using existing ./corsix- th_0.50.orig.tar.gz dpkg-source: info: local changes detected, the modified files are: corsix-th/CMakeCache.txt corsix-th/CMakeFiles/3.3.2/CMakeCCompiler.cmake corsix-th/CMakeFiles/3.3.2/CMakeCXXCompiler.cmake corsix-th/CMakeFiles/3.3.2/CMakeSystem.cmake corsix-th/CMakeFiles/3.3.2/CompilerIdC/CMakeCCompilerId.c corsix-th/CMakeFiles/3.3.2/CompilerIdCXX/CMakeCXXCompilerId.cpp corsix-th/CMakeFiles/CMakeDirectoryInformation.cmake Changes since the last upload: - Initial release Regards, Alexandre Detiste -- System Information: Debian Release: stretch/sid APT prefers testing APT policy: (500, 'testing'), (450, 'unstable'), (400, 'experimental') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 4.2.0-1-amd64 (SMP w/6 CPU cores) Locale: LANG=fr_BE.UTF-8, LC_CTYPE=fr_BE.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system)