Hello,
Sorry for the delay.
On 2023/08/24 23:47:06 -0400, Thomas Frohwein <tfrohw...@fastmail.com> wrote:
> [...]
> comments and ok's are welcome; given the complexity there might still
> be benefit in adding this early to the port and working out some
> refinements in-tree, with also more opportunity to hear of
> unanticipated bugs.
one consequence of this is that we're dropping a few arches for which
godot is now available:
% m show=MONO_ARCHS
aarch64 amd64 i386
OK, maybe there's a little point in building godot on, say, hppa and
doing this will save lot of bulk time on various architectures, but
wanted to point it out.
Ideally, I think this should be flavor. -main and -tools should also
be flavors actually and not subpackages. (shame on me, I thought only
of the tiny bit saving in builds time vs the complexity it adds.)
It's also 'sad' that we have to add USE_{WXNEEDED,NOBTCFI} for this.
Would be too hard to turn this into a flavor? Then in the future we
might eventually also move -tools to a flavor, and build a set of
packages:
godot (this would be the current -main)
godot-tools (this would be the current -tools)
godot-mono
godot-mono-tools
just an idea. I'm fine with your current diff as is if it simplify
future work, we can iterate in-tree :)
some nits below, feel free to pick them or ignore.
> [1]
> https://docs.godotengine.org/en/3.5/development/compiling/compiling_with_mono.html
> [2] https://github.com/godotengine/godot-demo-projects
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/games/godot/Makefile,v
> retrieving revision 1.49
> diff -u -p -r1.49 Makefile
> --- Makefile 14 Aug 2023 12:40:50 -0000 1.49
> +++ Makefile 25 Aug 2023 02:44:30 -0000
> [...]
> @@ -89,12 +108,18 @@ LIB_DEPENDS = archivers/zstd \
> [...]
> -# copy over to allow patching GodotSteam
> post-extract:
> + @# install backends from FILESDIR
first time i'm seeing @ added in front of a comment ^^"
I know (at least) Emacs syntax highlighting complains, but just
# ... should be enough.
> cp -R ${FILESDIR}/sndio ${WRKDIST}/drivers
> cp ${FILESDIR}/ujoy/joypad_openbsd.{cpp,h} \
> ${WRKDIST}/platform/x11/
> + @# move GodotSteam into WRKSRC to allow patching
> mv ${WRKDIR}/GodotSteam-${GODOTSTEAM_V:S/v//} ${WRKSRC}/godotsteam
> + @# set up nuget package location
> + mkdir -p ${NUGETHOME}
> + mv ${WRKDIR}/godot-${V}-nuget-packages ${NUGETHOME}/packages
> + @# move pre-generated mono glue into place
> + mv ${GLUEDIR}/mono_glue.gen.cpp ${WRKSRC}/modules/mono/glue/
> + mv ${GLUEDIR}/GodotSharp/GodotSharp/Generated \
> + ${WRKSRC}/modules/mono/glue/GodotSharp/GodotSharp/
> + mv ${GLUEDIR}/GodotSharp/GodotSharpEditor/Generated \
> + ${WRKSRC}/modules/mono/glue/GodotSharp/GodotSharpEditor/
> [...]
> Index: patches/patch-modules_mono_build_scripts_mono_configure_py
> ===================================================================
> RCS file: patches/patch-modules_mono_build_scripts_mono_configure_py
> diff -N patches/patch-modules_mono_build_scripts_mono_configure_py
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-modules_mono_build_scripts_mono_configure_py 25 Aug
> 2023 02:44:30 -0000
> @@ -0,0 +1,51 @@
> +include a suffix argument for find_file_in_dir
> +use suffix argument to look for libmonosgen-2.0.so.1.0
> +disable librt/libdl
> +
> +Index: modules/mono/build_scripts/mono_configure.py
> +--- modules/mono/build_scripts/mono_configure.py.orig
> ++++ modules/mono/build_scripts/mono_configure.py
> +@@ -31,15 +31,16 @@ def find_name_in_dir_files(directory, names, prefixes=
> + return ""
> +
> +
> +-def find_file_in_dir(directory, names, prefixes=[""], extensions=[""]):
> +- for extension in extensions:
> +- if extension and not extension.startswith("."):
> +- extension = "." + extension
> +- for prefix in prefixes:
> +- for curname in names:
> +- filename = prefix + curname + extension
> +- if os.path.isfile(os.path.join(directory, filename)):
> +- return filename
> ++def find_file_in_dir(directory, names, prefixes=[""], extensions=[""],
> suffix=[""]):
> ++ for sufx in suffix:
> ++ for extension in extensions:
> ++ if extension and not extension.startswith("."):
> ++ extension = "." + extension
> ++ for prefix in prefixes:
> ++ for curname in names:
> ++ filename = prefix + curname + extension + sufx
> ++ if os.path.isfile(os.path.join(directory, filename)):
> ++ return filename
> + return ""
> +
> +
> +@@ -335,7 +336,7 @@ def configure(env, env_mono):
> + elif is_javascript:
> + env.Append(LIBS=["m", "rt", "dl", "pthread"])
> + else:
> +- env.Append(LIBS=["m", "rt", "dl", "pthread"])
> ++ env.Append(LIBS=["m", "pthread"])
if you have the time I would open an issue upstream just to let them
know that not everyone has/needs rt and dl.
the job of a configure script would be to detect this stuff (I'm
amazed how scons makes me miss autoconf sometimes, AC_SEARCH_LIB in
this case :P)
> + if not mono_static:
> + mono_so_file = find_file_in_dir(
> +@@ -358,7 +359,7 @@ def configure(env, env_mono):
> + tmpenv.ParseConfig("pkg-config monosgen-2 --libs-only-L")
> +
> + for hint_dir in tmpenv["LIBPATH"]:
> +- file_found = find_file_in_dir(hint_dir, mono_lib_names,
> prefixes=["lib"], extensions=[sharedlib_ext])
> ++ file_found = find_file_in_dir(hint_dir, mono_lib_names,
> prefixes=["lib"], extensions=[sharedlib_ext], suffix=[".1.0"])
hardcoding the shlib version like this is ugly :(
I don't have a better suggestion though. We could at least avoid the
hardcoding by using a glob(3) and picking the highest numbered one.
Still ugly, but at least without the hardcoding.
> + if file_found:
> + mono_lib_path = hint_dir
> + mono_so_file = file_found
> [...]
> Index: patches/patch-modules_mono_csharp_script_cpp
> ===================================================================
> RCS file: patches/patch-modules_mono_csharp_script_cpp
> diff -N patches/patch-modules_mono_csharp_script_cpp
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-modules_mono_csharp_script_cpp 25 Aug 2023 02:44:30
> -0000
> @@ -0,0 +1,28 @@
> +fix error: cannot initialize return object of type 'bool' with
> +an rvalue of type 'nullptr_t'
> +
> +Index: modules/mono/csharp_script.cpp
> +--- modules/mono/csharp_script.cpp.orig
> ++++ modules/mono/csharp_script.cpp
If you have the time, I would send this one upstream so maybe we'll be
able to drop it in a future version of the 3.x series.
> +@@ -2310,7 +2310,7 @@ bool CSharpScript::_update_exports(PlaceHolderScriptIn
> +
> + GDMonoMethod *ctor =
> script_class->get_method(CACHED_STRING_NAME(dotctor), 0);
> +
> +- ERR_FAIL_NULL_V_MSG(ctor, NULL,
> ++ ERR_FAIL_NULL_V_MSG(ctor, false,
> [...]
> Index: patches/patch-modules_mono_godotsharp_dirs_cpp
> ===================================================================
> RCS file: patches/patch-modules_mono_godotsharp_dirs_cpp
> diff -N patches/patch-modules_mono_godotsharp_dirs_cpp
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-modules_mono_godotsharp_dirs_cpp 25 Aug 2023 02:44:30
> -0000
> @@ -0,0 +1,93 @@
> +fix paths for our mono and godot install dirs
> +make data_editor_prebuilt_api_dir() available also when !TOOLS_ENABLED
> +
> +Index: modules/mono/godotsharp_dirs.cpp
> +--- modules/mono/godotsharp_dirs.cpp.orig
> ++++ modules/mono/godotsharp_dirs.cpp
> [...]
> +@@ -214,6 +214,7 @@ class _GodotSharpDirs { (public)
> +
> + #else
> +
> ++ /*
> + String appname =
> ProjectSettings::get_singleton()->get("application/config/name");
> + String appname_safe =
> OS::get_singleton()->get_safe_dir_name(appname);
> + String data_dir_root = exe_dir.plus_file("data_" +
> appname_safe);
> +@@ -223,11 +224,12 @@ class _GodotSharpDirs { (public)
> +
> + String data_mono_root_dir = data_dir_root.plus_file("Mono");
> + data_mono_etc_dir = data_mono_root_dir.plus_file("etc");
> ++ */
using #if 0 may be more robust if in an update upstreams adds a
/*comment*/ inbetween.
> + #ifdef ANDROID_ENABLED
> + data_mono_lib_dir =
> gdmono::android::support::get_app_native_lib_dir();
> + #else
> +- data_mono_lib_dir = data_mono_root_dir.plus_file("lib");
> ++ data_mono_lib_dir = "${LOCALBASE}/lib";
> + data_game_assemblies_dir =
> data_dir_root.plus_file("Assemblies");
> + #endif
> +
