Subject: Re: [PATCH 02/20] gnu: minetest: Search for mods in MINETEST_MOD_PATH.

From: Leo Prikler <[email-masked]>
Date: Mon, 02 Aug 2021 19:28:58 +0200

Hi Maxime,

Am Montag, den 02.08.2021, 17:50 +0200 schrieb Maxime Devos:
> * gnu/packages/patches/Add-environment-variable-
> MINETEST_MOD_PATH.patch:
>   New file.
> * gnu/packages/games.scm
>   (minetest)[source]{patches}: Add it.
>   (minetest)[native-search-paths]: Add "MINETEST_MOD_PATH".
> * gnu/local.mk (dist_patch_DATA): Add the patch.
> ---
>  gnu/local.mk                                  |   1 +
>  gnu/packages/games.scm                        |   7 +-
>  ...vironment-variable-MINETEST_MOD_PATH.patch | 115
> ++++++++++++++++++
>  3 files changed, 122 insertions(+), 1 deletion(-)
>  create mode 100644 gnu/packages/patches/Add-environment-variable-
> MINETEST_MOD_PATH.patch
> 
> diff --git a/gnu/local.mk b/gnu/local.mk
> index c80a9af78c..d96d4e3dbc 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -801,6 +801,7 @@ dist_patch_DATA =					
> 	\
>    %D%/packages/patches/abseil-cpp-fix-gtest.patch		\
>    %D%/packages/patches/abseil-cpp-fix-strerror_test.patch	\
>    %D%/packages/patches/adb-add-libraries.patch			
> \
> +  %D%/packages/patches/Add-environment-variable-
> MINETEST_MOD_PATH.patch	\
>    %D%/packages/patches/aegis-constness-error.patch         	\
>    %D%/packages/patches/aegis-perl-tempdir1.patch           	\
>    %D%/packages/patches/aegis-perl-tempdir2.patch           	\
> diff --git a/gnu/packages/games.scm b/gnu/packages/games.scm
> index 3e7086b398..2f3285c6ea 100644
> --- a/gnu/packages/games.scm
> +++ b/gnu/packages/games.scm
> @@ -3553,6 +3553,7 @@ match, cannon keep, and grave-itation pit.")
>                 (base32
>                  "062ilb7s377q3hwfhl8q06vvcw2raydz5ljzlzwy2dmyzmdcndb
> 8"))
>                (modules '((guix build utils)))
> +              (patches (search-patches "Add-environment-variable-
> MINETEST_MOD_PATH.patch"))
>                (snippet
>                 '(begin
>                    ;; Delete bundled libraries.
> @@ -3599,7 +3600,11 @@ match, cannon keep, and grave-itation pit.")
>      (native-search-paths
>       (list (search-path-specification
>              (variable "MINETEST_SUBGAME_PATH")
> -            (files '("share/minetest/games")))))
> +            (files '("share/minetest/games")))
> +           (search-path-specification
> +            (variable "MINETEST_MOD_PATH")
> +            (files '("share/minetest/mods"))
> +            (separator #f))))
>      (native-inputs
>       `(("pkg-config" ,pkg-config)))
>      (inputs
> diff --git a/gnu/packages/patches/Add-environment-variable-
> MINETEST_MOD_PATH.patch b/gnu/packages/patches/Add-environment-
> variable-MINETEST_MOD_PATH.patch
> new file mode 100644
> index 0000000000..8478a7bf72
> --- /dev/null
> +++ b/gnu/packages/patches/Add-environment-variable-
> MINETEST_MOD_PATH.patch
> @@ -0,0 +1,115 @@
> +From 6eb753c5bf67764890856cf23a67c0bf65973c16 Mon Sep 17 00:00:00
> 2001
> +From: Maxime Devos <[email-masked]>
> +Date: Thu, 29 Jul 2021 22:24:50 +0200
> +Subject: [PATCH] Add environment variable MINETEST_MOD_PATH
> +
> +This adds an environment variable MINETEST_MOD_PATH.
> +When it exists, Minetest will look there for mods
> +in addition to ~/.minetest/mods/.
> +
> +This patch as-is is not yet ready for upstream, because:
> +
> +  * the GUI will only display mods in MINETEST_MOD_PATH
> +    or mods in ~/.minetest/mods, it won't combine the two
> +
> +  * the GUI for installing mods from ContentDB is disabled
> +    when MINETEST_MOD_PATH is set, because otherwise Minetest
> +    would try to install mods in the store.
These two are fine for a "Guix-only" patch, although I do think we
should still read ~/.minetest/mods for backwards compatibility.

> +  * MINETEST_MOD_PATH can only have a single component
This one seems kinda arbitrary, though, and does not fit well with
MINETEST_SUBGAME_PATH.

> +---
> + builtin/mainmenu/dlg_contentstore.lua |  7 +++++++
> + src/content/subgames.cpp              |  3 +++
> + src/script/lua_api/l_mainmenu.cpp     | 14 +++++++++++++-
> + src/script/lua_api/l_mainmenu.h       |  2 ++
> + 4 files changed, 25 insertions(+), 1 deletion(-)
> +
> +diff --git a/builtin/mainmenu/dlg_contentstore.lua
> b/builtin/mainmenu/dlg_contentstore.lua
> +index 7096c9187..c4a2cbd18 100644
> +--- a/builtin/mainmenu/dlg_contentstore.lua
> ++++ b/builtin/mainmenu/dlg_contentstore.lua
> +@@ -22,6 +22,13 @@ if not core.get_http_api then
> + 	end
> + 	return
> + end
> ++if core.mod_path_set() then
> ++	function create_store_dlg()
> ++		return messagebox("store",
> ++				fgettext("Mods from ContentDB cannot be
> installed when mods from Guix are also installed"))
> ++	end
> ++	return
> ++end
> + 
> + -- Unordered preserves the original order of the ContentDB API,
> + -- before the package list is ordered based on installed state.
> +diff --git a/src/content/subgames.cpp b/src/content/subgames.cpp
> +index e9dc609b0..1809f189e 100644
> +--- a/src/content/subgames.cpp
> ++++ b/src/content/subgames.cpp
> +@@ -110,6 +110,9 @@ SubgameSpec findSubgame(const std::string &id)
> + 	std::set<std::string> mods_paths;
> + 	if (!user_game)
> + 		mods_paths.insert(share + DIR_DELIM + "mods");
> ++	const char *env_mod_path = getenv("MINETEST_MOD_PATH");
> ++	if (env_mod_path)
> ++		mods_paths.insert(std::string(env_mod_path));
Here, I would instead use an std::istringstream together with
std::getline(<>, <>, ':') to get the components of MINETEST_MOD_PATH
and insert each of them.  Either that or copy whatever is used for
MINETEST_SUBGAME_PATH.
> + 	if (user != share || user_game)
> + 		mods_paths.insert(user + DIR_DELIM + "mods");
> + 
> +diff --git a/src/script/lua_api/l_mainmenu.cpp
> b/src/script/lua_api/l_mainmenu.cpp
> +index ad00de1c4..737550c42 100644
> +--- a/src/script/lua_api/l_mainmenu.cpp
> ++++ b/src/script/lua_api/l_mainmenu.cpp
> +@@ -495,9 +495,19 @@ int ModApiMainMenu::l_get_user_path(lua_State
> *L)
> +
> /********************************************************************
> **********/
> + int ModApiMainMenu::l_get_modpath(lua_State *L)
> + {
> ++	const char *c_modpath = getenv("MINETEST_MOD_PATH");
> + 	std::string modpath = fs::RemoveRelativePathComponents(
> + 		porting::path_user + DIR_DELIM + "mods" + DIR_DELIM);
> +-	lua_pushstring(L, modpath.c_str());
> ++	if (c_modpath == NULL)
> ++		c_modpath = modpath.c_str();
> ++	lua_pushstring(L, c_modpath);
> ++	return 1;
> ++}
> ++
> ++/******************************************************************
> ************/
> ++int ModApiMainMenu::l_mod_path_set(lua_State *L)
> ++{
> ++	lua_pushboolean(L, NULL != getenv("MINETEST_MOD_PATH"));
> + 	return 1;
> + }
> + 
> +@@ -855,6 +865,7 @@ void ModApiMainMenu::Initialize(lua_State *L,
> int top)
> + 	API_FCT(get_mapgen_names);
> + 	API_FCT(get_user_path);
> + 	API_FCT(get_modpath);
> ++	API_FCT(mod_path_set);
> + 	API_FCT(get_clientmodpath);
> + 	API_FCT(get_gamepath);
> + 	API_FCT(get_texturepath);
> +@@ -888,6 +899,7 @@ void ModApiMainMenu::InitializeAsync(lua_State
> *L, int top)
> + 	API_FCT(get_mapgen_names);
> + 	API_FCT(get_user_path);
> + 	API_FCT(get_modpath);
> ++	API_FCT(mod_path_set);
> + 	API_FCT(get_clientmodpath);
> + 	API_FCT(get_gamepath);
> + 	API_FCT(get_texturepath);
> +diff --git a/src/script/lua_api/l_mainmenu.h
> b/src/script/lua_api/l_mainmenu.h
> +index ec2d20da2..719c26077 100644
> +--- a/src/script/lua_api/l_mainmenu.h
> ++++ b/src/script/lua_api/l_mainmenu.h
> +@@ -112,6 +112,8 @@ class ModApiMainMenu: public ModApiBase
> + 
> + 	static int l_get_modpath(lua_State *L);
> + 
> ++	static int l_mod_path_set(lua_State *L);
> ++
> + 	static int l_get_clientmodpath(lua_State *L);
> + 
> + 	static int l_get_gamepath(lua_State *L);
> +-- 
> +2.32.0
What are these modpaths used for?  For mod installation or for querying
mod existence?  If it's the former, you could leave them as-is, similar
to how elpa stays enabled in Emacs.

Regards,