diff mbox series

[bug#49828,02/20] gnu: minetest: Search for mods in MINETEST_MOD_PATH.

Message ID 20210802155019.6122-2-maximedevos@telenet.be
State Accepted
Headers show
Series Add minetest mods | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

M Aug. 2, 2021, 3:50 p.m. UTC
* 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

Comments

Leo Prikler Aug. 2, 2021, 5:28 p.m. UTC | #1
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 <maximedevos@telenet.be>
> +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,
M Aug. 2, 2021, 5:53 p.m. UTC | #2
> > +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/mods is still read when MINETEST_MOD_PATH is unset.
MINETEST_MOD_PATH is only set when some mod is actually installed.
So backwards compatibility should be ok.

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

Yes, I know.  I didn't know how to adjust pkgmgr.lua and dlg_contentstore.lua
to support multiple components, though I have an idea now to try.

> > + -- 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.

Minetest has a class 'Strfnd' supporting iteration.  Using that,
it should be easy to allow MINETEST_MOD_PATH to contain ":" seperators.
However, the GUI client code (pkgmgr.lua)
uses some other logic for determining which mods exists, and currently,
that logic does not support ":" separators.

> > +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.

It is only used by the GUI.  The GUI looks in the directory returned by
"get_modpath" for two things:

(1) to determine which mods exist (and to find their descriptions, their
    dependency list, screenshots ...).  Only the mods that exist there
    can be enabled.

(2) to determine where mods must be installed when using Minetest's
    built-in installer.

Greetings,
Maxime.
Leo Prikler Aug. 2, 2021, 6:47 p.m. UTC | #3
Am Montag, den 02.08.2021, 19:53 +0200 schrieb Maxime Devos:
> > > +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/mods is still read when MINETEST_MOD_PATH is unset.
> MINETEST_MOD_PATH is only set when some mod is actually installed.
> So backwards compatibility should be ok.
I mean in the sense of "have some mods in ~/.minetest/mods, that aren't
yet packaged in Guix and have the rest sit in the profile where they
belong".  Not everyone will like the the import to manifest approach
that is needed while you're waiting for review.
> > > +  * MINETEST_MOD_PATH can only have a single component
> > This one seems kinda arbitrary, though, and does not fit well with
> > MINETEST_SUBGAME_PATH.
> 
> Yes, I know.  I didn't know how to adjust pkgmgr.lua and
> dlg_contentstore.lua
> to support multiple components, though I have an idea now to try.
> 
> > > + -- 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.
> 
> Minetest has a class 'Strfnd' supporting iteration.  Using that,
> it should be easy to allow MINETEST_MOD_PATH to contain ":"
> seperators.
> However, the GUI client code (pkgmgr.lua)
> uses some other logic for determining which mods exists, and
> currently,
> that logic does not support ":" separators.
Hmm, how important is the GUI side here?  Can we sneek mods into it?

> > > +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.
> 
> It is only used by the GUI.  The GUI looks in the directory returned
> by "get_modpath" for two things:
> 
> (1) to determine which mods exist (and to find their descriptions,  
>     their dependency list, screenshots ...).  Only the mods that 
>     exist there can be enabled.
> 
> (2) to determine where mods must be installed when using Minetest's
>     built-in installer.
I see.  Is this the same GUI for all parts or different GUIs?  I think
if we can handle the world creation parts, everything should be fine,
no?

Regards,
M Aug. 3, 2021, 11:09 a.m. UTC | #4
Hi,

I've modified this patch such that:

  * the GUI will display mods in MINETEST_MOD_PATH as well as
    mods in ~/.minetest/mods

  * the built-in installer works even if MINETEST_MOD_PATH is set

  * MINETEST_MOD_PATH can contain multiple components

Please tell if there are other issues

I'll look into the upstream procedure for submitting patches.

Leo Prikler schreef op ma 02-08-2021 om 20:47 [+0200]:
> Am Montag, den 02.08.2021, 19:53 +0200 schrieb Maxime Devos:
> > > > +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/mods is still read when MINETEST_MOD_PATH is unset.
> > MINETEST_MOD_PATH is only set when some mod is actually installed.
> > So backwards compatibility should be ok.
> I mean in the sense of "have some mods in ~/.minetest/mods, that aren't
> yet packaged in Guix and have the rest sit in the profile where they
> belong".  Not everyone will like the the import to manifest approach
> that is needed while you're waiting for review.

All three points should be addresed by the new patch.

> > > > + -- 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.
> > 
> > Minetest has a class 'Strfnd' supporting iteration.  Using that,
> > it should be easy to allow MINETEST_MOD_PATH to contain ":"
> > seperators.
> > However, the GUI client code (pkgmgr.lua)
> > uses some other logic for determining which mods exists, and
> > currently,
> > that logic does not support ":" separators.
> Hmm, how important is the GUI side here?  Can we sneek mods into it?

The GUI side is very important.  All mods are disabled by default,
and in the GUI you need to choose which mods to enable.  If the GUI
doesn't know about the existence of the mod, then the mod cannot be
enabled.

> > It is only used by the GUI.  The GUI looks in the directory returned
> > by "get_modpath" for two things:
> > 
> > (1) to determine which mods exist (and to find their descriptions,  
> >     their dependency list, screenshots ...).  Only the mods that 
> >     exist there can be enabled.
> > 
> > (2) to determine where mods must be installed when using Minetest's
> >     built-in installer.
> I see.  Is this the same GUI for all parts or different GUIs?  I think
> if we can handle the world creation parts, everything should be fine,
> no?

The ‘mod selecter’ (1) and installer (2) have separate graphical interfaces,
but they use some common code 'pkgmgr.lua'.  In the new patch, 'get_modpath()'
(a string) is where mods should be installed by the built-in installer,
and 'get_modpaths()' (a list of strings) is where mods can be found.
Both interfaces work with the new patch.

Greetings,
Maxime.
diff mbox series

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
                 "062ilb7s377q3hwfhl8q06vvcw2raydz5ljzlzwy2dmyzmdcndb8"))
               (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 <maximedevos@telenet.be>
+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.
+
+  * MINETEST_MOD_PATH can only have a single component
+---
+ 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));
+ 	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
+