0003538: The preferred form of IP2C database for modifications (CSV) is not currently distributed

Notes
(0019936) WubTheCaptain (reporter) 2018-10-06 11:53	0003512:0019932: Quote from Zalewa We could avoid putting 13 useless Megabytes into the repository by packaging the MaxMind's CSV files together with our IpToCountry.dat file in an archive and putting that archive on our home page and stating in compilation instructions that to prepare the proper deployment package you need to grab this package from the server. The current CMake INSTALL() dependency on IpToCountry.dat will have to be made optional. Negative consequences of this are that a) the build process gets more complicated, and b) the risk of preparing the package incorrectly rises. So, this is still something that I don't like. Finally, we reach upon the solution where we put the 13 Megabytes of CSV together with the IpToCountry.dat prepared from that exact CSV into the repo in a singular commit. This should satisfy the requirements of "preferable edit format" and also keep in check the exact license that the currently preinstalled database is on (MaxMind's archive contains the license file).

(0019939) WubTheCaptain (reporter) 2018-10-06 12:19	Here's my opinions: Quote from Zalewa I don't like an approach where the database would be downloaded from MaxMind I don't like this. Quote from Zalewa converted on the fly during the package process I like this (for Debian GNU/Linux), especially because it takes only few seconds now since commit 536f21e02e30fb552fcdb8e832b400064bc56fb9. Quote from Zalewa Not only this adds a mandatory dependency on Python I don't like this much. Ok if the this is a seperate (source) package, e.g. doomseeker-ip2c. Quote from Zalewa Moreover, it will mean that we will either have to get rid of the "IP2C Update" feature from the program Unnecessary, if there's one specific version of the CSV database in each version? Though personally, I believe in less features being better. Quote from Zalewa or automatize update of the IpToCountry.dat available from our home page See above. Quote from Zalewa or otherwise it may turn out that the preinstalled database is more up-to-date than the supposedly up-to-date database on the home page. This may theoretically happen at any time, anyway. Quote from Zalewa We could avoid putting 13 useless Megabytes into the repository by packaging the MaxMind's CSV files together with our IpToCountry.dat file in an archive and putting that archive on our home page and stating in compilation instructions that to prepare the proper deployment package you need to grab this package from the server. I'm liking this. If IpToCountry.dat is included with GeoLite2-Country-CSV.zip (or its contents), there will be a DFSG-removal of IpToCountry.dat in Debian but alright. Quote from Zalewa The current CMake INSTALL() dependency on IpToCountry.dat will have to be made optional. I'm possibly liking this. Quote from Zalewa Negative consequences of this are that a) the build process gets more complicated, and b) the risk of preparing the package incorrectly rises. So, this is still something that I don't like. As far as I've understood this is not a concern, just like Doomseeker and Wadseeker are seperate packages and one depends on the other. Quote from Zalewa Finally, we reach upon the solution where we put the 13 Megabytes of CSV together with the IpToCountry.dat prepared from that exact CSV into the repo in a singular commit. This should satisfy the requirements of "preferable edit format" and also keep in check the exact license that the currently preinstalled database is on (MaxMind's archive contains the license file). Yes, but create a new repository for this for CSV / IP2C updates only.

(0019940) WubTheCaptain (reporter) 2018-10-06 12:20	Quote from Zalewa Not only this adds a mandatory dependency on Python Oh and well, I guess there's always an option to rewrite the tool in C or C++.

(0019941) WubTheCaptain (reporter) 2018-10-06 12:24	And my point here is: You can keep building Doomseeker from source during development, but make IP2C truly optional. This would mean a new repository for IP2C data and source, and this may be updated less frequently (or autoupdated with IP2C Update). The only concern here I see is setting a stable date/revision for the IP2C database.

(0019945) Zalewa (developer) 2018-10-06 13:47 edited on: 2018-10-06 13:56	Quote from WubTheCaptain I believe in less features being better. But you do not believe in complicating the building process being worse? * Increasing amount of text in COMPILE.Windows.txt and COMPILE.txt by another section is complicating the build as someone now needs to read more to be ready to do work on the project. * Moving IP2C data to a separate repository is complicating the build. I loathe projects that feel the need to split everything into tiny repositories, especially when it turns out you need to alter something and there are interdependencies between the repositories and you need to simultaneously make changes and push commits to both. * Optional INSTALL() statement is complicating the build on the basis that it allows the person who prepares the package to make a mistake and not include the item that was meant to be included. Programmers create software to remove the possibility of human error, not enable it, and Doomseeker should really always be redistributed with the database. * Even though we can safely assume that every Linux distro will have a Python interpreter available, Windows needs to have it installed separately. I have Python on all my Windows machines because it is useful to have it, but not providing a premade IpToCountry.dat for any other potential contributor who would also want to use Windows is complicating the build and reducing this contributor's desire to engage with the project. In regards to the "IP2C Update" feature I'm however beginning to agree with you that this feature has become unnecessary - or rather, since Linux is covered by package managers and Mac & Windows are covered by Mendeley's Updater we can remove the separate "IP2C Update" feature altogether and rely on the aforementioned methods of update. Quote from WubTheCaptain Doomseeker and Wadseeker are seperate packages True for Linux, false for Windows. The default package for Windows is monolithic and includes the whole distribution with all plugins, Qt, MSVC redist and docs. The only other Windows package in existence I know of is the one that's bundled with Zandronum and it's pretty much the same other than lacking non-Zandronum plugins. Quote from WubTheCaptain Quote from Zalewa Negative consequences of this are that a) the build process gets more complicated, and b) the risk of preparing the package incorrectly rises. As far as I've understood this is not a concern Complicating a build is pretty much always a concern. Look how simple building Doomseeker on Linux is right now - you get the repo, satisfy the dependencies through the package manager and it will build without any fuss. If the problem we're having here was caused by a technical (software engineering) issue or solving it was needed to implement some new feature I would relent and gladly complicate the build as the end-user would benefit from it. But let's face it: we're spending lots of resources to solve a problem that average user doesn't give a damn about, we've been in violation of the license for a long time now and no one cared and we're introducing contrivances to solve issues that exist only on paper and need to be explained by even more paper. Unless someone has less contrived idea, the only solution that I dislike the least is putting the CSVs into the current repo. By this I also wish to state that I do not like any of the solutions here, but since we want to comply with licensing as best as we can, I'm willing to choose the lesser of several evils to solve the problem.

(0019948) WubTheCaptain (reporter) 2018-10-06 14:41 edited on: 2018-10-06 14:41	Quote from Zalewa But you do not believe in complicating the building process being worse? It's a packaging issue, not a software one. Windows "does not" have a good package manager available. Of course it's worse. Heck, I'd prefer if there was no CMake (or any GNU autohell) involved in building Doomseeker; it'd be quite unrealistic to expect of a Qt software not UNIX-style or POSIX-like working with standard input/output. Quote from Zalewa Increasing amount of text in COMPILE.Windows.txt and COMPILE.txt by another section is complicating the build as someone now needs to read more to be ready to do work on the project. This should be (or needs to be) done anyway, for someone wanting to build Doomseeker truly from scratch using the source, minus the "ready to do work" part. But the IP2C is mainly optional for a source hacker testing changes... Quote from Zalewa I loathe projects that feel the need to split everything into tiny repositories, especially when it turns out you need to alter something and there are interdependencies between the repositories and you need to simultaneously make changes and push commits to both. Agreed. But why would you need to push anything but upstream CSV data and/or built IpToCountry.dat there? This happens quite infrequently, anyway. Quote from Zalewa I have Python on all my Windows machines because it is useful to have it, but not providing a premade IpToCountry.dat for any other potential contributor who would also want to use Windows is complicating the build and reducing this contributor's desire to engage with the project. The problem is not with including a premade IpToCountry.dat. You can include it, ship .exe along with it and offer/convey the source code (to Doomseeker, not necessarily IpToCountry.dat). Only Debian Free Software Guidelines (DFSG) see it as an issue. There can be no precompiled files in Debian's source tarball (derived from upstream tarball, and named a DFSG-source if modified by the package maintainer with those generated files removed). If no source is available at all, that will most likely block creating a package to the free repositories (thus, the 13MB CSV argument). Quote from Zalewa Optional INSTALL() statement is complicating the build on the basis that it allows the person who prepares the package to make a mistake and not include the item that was meant to be included. Make it (loudly) state during cmake if internal or system database will be used? This is what we already with other dependencies. Oh, and we have a proposal to make the game/engine plugins optional too. Quote from Zalewa Even though we can safely assume that every Linux distro will have a Python interpreter available Fyi: OpenBSD – although not GNU/Linux – does not have Python (but there's a port available). Anyway... Quote from Zalewa In regards to the "IP2C Update" feature I'm however beginning to agree with you that this feature has become unnecessary - or rather, since Linux is covered by package managers and Mac & Windows are covered by Mendeley's Updater we can remove the separate "IP2C Update" feature altogether and rely on the aforementioned methods of update. New issue 0003541. Quote from Zalewa True for Linux, false for Windows. 0003292 looks ever so much more inviting to resolve, anyway we're not making progress with this ticket. Quote from Zalewa the only solution that I dislike the least is putting the CSVs into the current repo As long as there is a tool to convert from this file without downloads from the Internet, it should be okay for Debian. I agree I don't like the solution either, but it's also the one I dislike the least too.

(0019949) WubTheCaptain (reporter) 2018-10-06 14:41	So, is the solution here to include a revision of the CSV files in Doomseeker/doomseeker repository?

(0019957) Zalewa (developer) 2018-10-06 17:35	Quote from WubTheCaptain So, is the solution here to include a revision of the CSV files in Doomseeker/doomseeker repository? I'd say yes. Blzut?

(0019970) Blzut3 (administrator) 2018-10-06 21:28	I think the fact being missed here is that presumably the reason that we baked an IP2C database into the executable in the first place is to smooth out the start up experience. In other words we want users to immediately see flags instead of having to wait for a download to complete. To that end, my assumption the whole time here is that our IP2C updater would provide the canonical source of that database whether a database is included with the program (our distributions) or not (Debian). If we really wanted to adding a timestamp to the database would be an easy way to solve the "local is newer" problem (this timestamp could be put in the HTTP header when checking for a newer database), but I personally don't think it's required to be solved. Since having the initial database is not a requirement, there's no additional hard dependency on Python. Just an optional one for those creating release packages. Those just building for themselves won't care since they'll just download the database when the app starts. Another thing to keep in mind is that if we continue to include a database in the source, how long until Wub opens a ticket every week to tell us to update the vendored database. :P As for build reproducibility: This is a fair point, but as an optional component that contains highly volatile data I'm not sure it's particularly important to ensure that the exact state of the database at release time is reproducible. Keep in mind also that the usefulness of an old version of Doomseeker also falls apart when protocols change, so this project is kind of temporal in nature. Complicating the packaging process: I already have a script for Mac (makemacdmg.sh/mergemacdmg.sh), Linux (cpackpostprocess.sh), and sources packages (makesourcepackages.sh). Inserting a database into the zip file could easily be captured here. You might even be able to use "cmake -E tar --format=zip" to do this without having to assume the user has a zip tool installed. (Although I wouldn't be surprised if Powershell just has this built in.) In short I think I'd rather continue to update IP2C databases the way we are now, especially in light of having to disable the Mac program auto updates. If including a database in the repo makes you sleep better at night then do it. I just think it's a pointless tens of MB. Perhaps reconsider the idea of a separate repo for the database if your primary concern is having the databases frozen in time (again it's optional so no inter-dependencies), but I'm not going to lose sleep if you insist on keeping it in the main repo.

(0019985) Zalewa (developer) 2018-10-07 09:38	In spite of my concerns I have finally decided to move the IP2C data to the "blobs" repository which can be used in the future to insert there more big binary stuff (though with much, much caution and reluctance, please). CSVs were put into this repository in this particular commit:'https://bitbucket.org/Doomseeker/doomseeker-blobs/commits/c33890cb9a0f39a17a0a0369a1f28f202d2b79e4 [^]' I have not included the IpToCountry.dat blob because it can be quickly generated using the geolite2_conv.py script, which also now resides in the blobs repo. To save myself from remembering the command line, I also allowed myself to add a make_ip2c.bat file that will prepare the files as we need them. geolite2_conv.py has also been modified to create 'md5' and 'geolite2.gz' files with hardcoded names that can be directly uploaded through FTP to our website for "IP2C Update" purpose (0003541). As IpToCountry.dat has been removed from Doomseeker's main repo it has become necessary to introduce DOOMSEEKER_IP2C_DAT CMake option:'https://bitbucket.org/Doomseeker/doomseeker/commits/ad48cade47f154f56970ecf117a8306748c08199 [^]' And, as a final consequence, since IpToCountry.dat inclusion in the distribution doesn't "just work" anymore, it became necessary to explain how to include it in the compilation instructions:'https://bitbucket.org/Doomseeker/doomseeker/commits/13bf2040a055181d3403a575a489701c03f214bc [^]' Finally, the excessive granularity of these tickets makes it difficult to attach the correct ticket number to the commit message and forces me to repeat myself in these notes.

(0019992) WubTheCaptain (reporter) 2018-10-07 11:03	We'll see how this goes for packaging in the future, and open issues later. Quote from Zalewa Finally, the excessive granularity of these tickets makes it difficult to attach the correct ticket number to the commit message and forces me to repeat myself in these notes. Sorry, I believe this was a necessary evil to divulge into some topics more in-depth without making the notes look noisy and off-topic to the issue mentioned in OP (and assumed resolution). In case of doubt, the correct ticket number for commit messages is usually the parent ticket.

This issue is already marked as resolved. If you feel that is not the case, please reopen it and explain why.
Supporters:	No one explicitly supports this issue yet.
Opponents:	No one explicitly opposes this issue yet.

Issue History
Date Modified	Username	Field	Change
2018-10-06 11:49	WubTheCaptain	New Issue
2018-10-06 11:49	WubTheCaptain	Summary	IP2C database is downloaded from MaxMind and converted on the fly during the package process => IP2C database "is" downloaded from MaxMind and converted on the fly during the package process
2018-10-06 11:50	WubTheCaptain	Relationship added	child of 0003246
2018-10-06 11:50	WubTheCaptain	Relationship added	related to 0003512
2018-10-06 11:51	WubTheCaptain	Target Version	=> 1.2
2018-10-06 11:51	WubTheCaptain	Description Updated	View Revisions
2018-10-06 11:51	WubTheCaptain	Relationship added	related to 0003255
2018-10-06 11:53	WubTheCaptain	Note Added: 0019936
2018-10-06 11:58	WubTheCaptain	Product Version	=> 1.1
2018-10-06 12:00	WubTheCaptain	Relationship added	related to 0003537
2018-10-06 12:01	WubTheCaptain	Relationship deleted	related to 0003512
2018-10-06 12:03	WubTheCaptain	Status	new => confirmed
2018-10-06 12:19	WubTheCaptain	Note Added: 0019939
2018-10-06 12:20	WubTheCaptain	Note Added: 0019940
2018-10-06 12:20	WubTheCaptain	Summary	IP2C database "is" downloaded from MaxMind and converted on the fly during the package process => IP2C database would need to be downloaded from MaxMind and converted on the fly during the package process
2018-10-06 12:24	WubTheCaptain	Note Added: 0019941
2018-10-06 12:30	WubTheCaptain	Description Updated	View Revisions
2018-10-06 12:31	WubTheCaptain	Description Updated	View Revisions
2018-10-06 12:36	WubTheCaptain	Description Updated	View Revisions
2018-10-06 12:37	WubTheCaptain	Summary	IP2C database would need to be downloaded from MaxMind and converted on the fly during the package process => The preferred form of IP2C database for modifications (CSV) is not currently distributed
2018-10-06 12:37	WubTheCaptain	Description Updated	View Revisions
2018-10-06 12:37	WubTheCaptain	Additional Information Updated	View Revisions
2018-10-06 12:37	WubTheCaptain	Relationship added	child of 0003512
2018-10-06 12:38	WubTheCaptain	Relationship replaced	related to 0003512
2018-10-06 12:38	WubTheCaptain	Relationship added	related to 0003237
2018-10-06 12:40	WubTheCaptain	Description Updated	View Revisions
2018-10-06 12:40	WubTheCaptain	Additional Information Updated	View Revisions
2018-10-06 12:41	WubTheCaptain	Relationship deleted	related to 0003237
2018-10-06 12:42	WubTheCaptain	Relationship replaced	child of 0003512
2018-10-06 13:47	Zalewa	Note Added: 0019945
2018-10-06 13:56	Zalewa	Note Edited: 0019945	View Revisions
2018-10-06 14:41	WubTheCaptain	Note Added: 0019948
2018-10-06 14:41	WubTheCaptain	Note Edited: 0019948	View Revisions
2018-10-06 14:41	WubTheCaptain	Note Added: 0019949
2018-10-06 17:35	Zalewa	Note Added: 0019957
2018-10-06 18:06	WubTheCaptain	Assigned To	=> WubTheCaptain
2018-10-06 18:06	WubTheCaptain	Status	confirmed => feedback
2018-10-06 18:06	WubTheCaptain	Assigned To	WubTheCaptain =>
2018-10-06 21:28	Blzut3	Note Added: 0019970
2018-10-07 06:17	Zalewa	Assigned To	=> Zalewa
2018-10-07 06:17	Zalewa	Status	feedback => assigned
2018-10-07 09:38	Zalewa	Note Added: 0019985
2018-10-07 09:38	Zalewa	Status	assigned => needs review
2018-10-07 11:03	WubTheCaptain	Note Added: 0019992
2018-10-07 11:03	WubTheCaptain	Status	needs review => resolved
2018-10-07 11:03	WubTheCaptain	Fixed in Version	=> 1.2
2018-10-07 11:03	WubTheCaptain	Resolution	open => fixed
2018-10-27 22:54	WubTheCaptain	Status	resolved => closed
2018-12-17 03:34	WubTheCaptain	Category	Bug => IP2C