Repository: metacall/core (language: rust)

Issue:
Remove the need of set up environment variables in CLI or MetaCall
## 🚀 Feature

MetaCall has the need to set up multiple environment variables ( https://github.com/metacall/core/blob/develop/docs/README.md#42-environment-variables ), for example, in order to find the global configuration or plugins. This can be basically avoided if we use a trick for locating `libmetacall.so` / `metacall.dll` / `libmetacall.dylib` from the executable itself. This solution is platform dependent but we can use it just as a fallback in case the environment variables are not detected, so MetaCall is still able to work as normally it does in multiple platforms.

The trick is the following:
 1) First of all, we get the list of the modules that the current executable (usually metacallcli) is linked too, this includes dynamic loaded modules, as in the case (in the future) we will support MetaCall to be imported from `python.exe` or `node.exe` ( https://github.com/metacall/core/issues/231 ), and in this case `libmetacall.so` / `metacall.dll` / `libmetacall.dylib` will be linked dynamically by `dlopen` or `LoadLibrary`.
2) Once we get the list, we try to find `libmetacall.so` / `metacall.dll` / `libmetacall.dylib` (taking into account versioning in the future if any, probably we need some way of parsing the library name or finding a symbol inside of the library in order to verify that it is the correct library).
3) We extract the absolute path of this module, and then we can obtain all the modules from there. For example, let's say we find MetaCall library in: `/home/custom/metacall/lib/libmetacall.so`, we can take this path and obtain all the environment variables from it. All the `*_LIBRARY_PATH` would be pointing to `/home/custom/metacall/lib` and the configuration `CONFIGURATION_PATH` can be deduced from it too and it will be pointing to `/home/custom/metacall/configurations/global.json`.

With this we remove all the need for setting up metacall and we improve the user experience with it, specially when using Ports because it is problematic to set them up properly.

Something we must review and it's **critical** in order to make this work properly, some loaders rely on getting the environment variables by themselves, specially in node loader or typescript loader with `bootstrap.js` and `bootstrap.ts`. We should remove the environment variable access from them and pass them from the C/C++ side, so it works too. This can be done easily because in the `initialize` step of each loader, we pass the configuration with the required paths as values, we can take them from there and pass them into JavaScript land.

Some of the reference for implementing this can be found here:
 - Windows: https://docs.microsoft.com/en-us/windows/win32/psapi/enumerating-all-modules-for-a-process
 - Linux: https://linux.die.net/man/3/dl_iterate_phdr
 - MacOs: https://stackoverflow.com/questions/10009043/dl-iterate-phdr-equivalent-on-mac (specially on: https://stackoverflow.com/a/10053641 )

The function for finding the module path can be implemented either on dynlink or in portability (dynlink can be interesting because it contains suffixes and prefixes for the dynamic libraries for each platform).


Discussion / hints:
@viferga I would like to get started by picking up this issue. Can you please assign it to me?
The order of priority for setting up the paths is:
1) User defined environment variables.
2) Paths obtained from the mechanism defined before.
3) Compile time default paths.

The paths (and environment variables) are initialized here:
https://github.com/metacall/core/blob/35652b3399f00caa11e1b25ee87db9ec67c5bdd8/source/loader/source/loader_env.c#L30
https://github.com/metacall/core/blob/35652b3399f00caa11e1b25ee87db9ec67c5bdd8/source/detour/source/detour_singleton.c#L92
https://github.com/metacall/core/blob/35652b3399f00caa11e1b25ee87db9ec67c5bdd8/source/configuration/source/configuration.c#L44
https://github.com/metacall/core/blob/8c91e10e7001705a1acb3ff87f2595a3bbd59419/source/serial/source/serial_singleton.c#L92

As you can see the code is pretty copy pasted. It can be improved but there's no need to do so in this patch, you can focus only in removing the need of setting up the environment variables.

My suggestion is to create a function that obtains the prefix path when metacall initializes (in metacall_initialize or similar), and then this can be queried by the different modules in order to obtain the respective path and then generate its own paths.

Just a reminder, we snake case (function_hello) instead of camel case (functionHello).

@viferga I wanted to discuss this approach with you. I was planning to define a function `set_env_path_from_metacall_lib` which does the following job:
- finds the custom path e.g. `/usr/local` using the location of shared library. Let's call this `metacall_custom_path`.
- then for each env variable, it would check if the env variable has been setup by the user. If any env is NULL or an empty string, it would calculate it's value using the `metacall_custom_path` and then set this as the value of the env variable.
Then we can call this function as soon as metacallcli is initialised before any env variables are accessed. Then, effectively we would use default values wherever env variables were not initialised after the 2 checks. This would help reduce copy pasting the code at multiple places where env variables are accessed. 
I'm not sure if this method have some loophole, or is not the right way to do.
I think it's fine but I would start doing the function that gives you the path, and only that, then create a unit test for it.

Once you have this, which works independently of metacall, then you can try to integrate this into the codebase. With unit tests you ensure your function works properly. I can help you to set up them. We use a lot Test Driven Development in this project, it helps us a lot to verify that the changes we do work properly.

Also, depending on the module we are, usually we set the module name as a prefix. For example metacall module has functions starting by `metacall_*`, the loader module has functions starting by `loader_*`, etc.

Fix patch:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index dc29eac5c..f88559f55 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -102,7 +102,7 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
 endif()
 
 # Binary directory
-if(MSVC AND NOT ("${CMAKE_GENERATOR}" STREQUAL "NMake Makefiles" OR "${CMAKE_GENERATOR}" STREQUAL "NMake Makefiles JOM"))
+if ("${CMAKE_GENERATOR}" MATCHES "^(Visual Studio)")
 	set(PROJECT_OUTPUT_DIR "${CMAKE_BINARY_DIR}/${CMAKE_BUILD_TYPE}")
 else()
 	set(PROJECT_OUTPUT_DIR "${CMAKE_BINARY_DIR}")
diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 28609cf8b..99ed976bf 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -15,3 +15,4 @@ Anas Al Barghouthy <anas@heavenlyx.com>
 ketangupta34 <ketangupta34@gmail.com>
 onkardahale <dahaleonkar@gmail.com>
 Akshit Garg <garg.akshit@gmail.com>
+burnerlee <avi.aviral140@gmail.com>
diff --git a/source/dynlink/source/dynlink_impl_win32.c b/source/dynlink/source/dynlink_impl_win32.c
index 02036a2f2..c501aed9c 100644
--- a/source/dynlink/source/dynlink_impl_win32.c
+++ b/source/dynlink/source/dynlink_impl_win32.c
@@ -29,6 +29,9 @@
 #include <string.h>
 
 #define WIN32_LEAN_AND_MEAN
+#include <psapi.h>
+#include <stdio.h>
+#include <tchar.h>
 #include <windows.h>
 
 /* -- Methods -- */
@@ -40,6 +43,15 @@ const char *dynlink_impl_interface_extension_win32(void)
 	return extension_win32;
 }
 
+static void dynlink_impl_interface_get_name_str_win32(dynlink_name name, dynlink_name_impl name_impl, size_t length)
+{
+	strncat(name_impl, name, length);
+
+	strncat(name_impl, ".", length);
+
+	strncat(name_impl, dynlink_impl_extension(), length);
+}
+
 void dynlink_impl_interface_get_name_win32(dynlink handle, dynlink_name_impl name_impl, size_t length)
 {
 	strncpy(name_impl, dynlink_get_name(handle), length);
@@ -89,13 +101,52 @@ int dynlink_impl_interface_unload_win32(dynlink handle, dynlink_impl impl)
 	return (FreeLibrary(impl) == FALSE);
 }
 
-char *dynlink_impl_interface_lib_path_win32(dynlink_name name, int (*comparator)(dynlink_path, dynlink_name))
+static char *dynlink_impl_interface_strip_lib_path_win32(char *metacall_lib_path, char *metacall_lib_name)
 {
-	/* TODO */
-	(void)name;
-	(void)comparator;
+	size_t lib_path_len = strlen(metacall_lib_path) - (strlen(metacall_lib_name) + 1);
+	char *custom_lib_path = malloc(sizeof(char) * (lib_path_len + 1));
+	custom_lib_path[lib_path_len] = 0;
+	strncpy(custom_lib_path, metacall_lib_path, lib_path_len);
+	return custom_lib_path;
+}
 
-	return NULL;
+char *dynlink_impl_interface_lib_path_win32(dynlink_name name, int (*comparator)(dynlink_path, dynlink_name))
+{
+	dynlink_name_impl metacall_lib_name = { 0 };
+	dynlink_impl_interface_get_name_str_win32(name, metacall_lib_name, DYNLINK_NAME_IMPL_SIZE - 1);
+	HMODULE handle_modules[1024];
+	HANDLE handle_process;
+	DWORD cb_needed;
+	unsigned int i;
+	handle_process = GetCurrentProcess();
+	char *metacall_lib_path = NULL;
+	int found_lib_path = 0;
+	if (EnumProcessModules(handle_process, handle_modules, sizeof(handle_modules), &cb_needed))
+	{
+		for (i = 0; i < (cb_needed / sizeof(HMODULE)); i++)
+		{
+			TCHAR lib_path[MAX_PATH];
+
+			// Get the full path to the module's file.
+
+			if (GetModuleFileNameEx(handle_process, handle_modules[i], lib_path,
+					sizeof(lib_path) / sizeof(TCHAR)))
+			{
+				if (comparator(lib_path, metacall_lib_name) == 0)
+				{
+					found_lib_path = 1;
+					metacall_lib_path = dynlink_impl_interface_strip_lib_path_win32(lib_path, metacall_lib_name);
+					break;
+				}
+			}
+		}
+	}
+	if (!found_lib_path)
+	{
+		free(metacall_lib_path);
+		return NULL;
+	}
+	return metacall_lib_path;
 }
 
 dynlink_impl_interface dynlink_impl_interface_singleton_win32(void)


Test patch:
diff --git a/source/tests/metacall_dynlink_path_test/CMakeLists.txt b/source/tests/metacall_dynlink_path_test/CMakeLists.txt
index bd60f41fb..64bfc4412 100644
--- a/source/tests/metacall_dynlink_path_test/CMakeLists.txt
+++ b/source/tests/metacall_dynlink_path_test/CMakeLists.txt
@@ -89,11 +89,17 @@ target_link_libraries(${target}
 # Compile definitions
 #
 
+if(WIN32)
+	string(REPLACE "/" "\\\\" METACALL_LIBRARY_PATH "${PROJECT_OUTPUT_DIR}")
+else()
+	set(METACALL_LIBRARY_PATH "${PROJECT_OUTPUT_DIR}")
+endif()
+
 target_compile_definitions(${target}
 	PRIVATE
 	${DEFAULT_COMPILE_DEFINITIONS}
 	# For metacall library path test
-	METACALL_LIBRARY_PATH="${PROJECT_OUTPUT_DIR}"
+	METACALL_LIBRARY_PATH="${METACALL_LIBRARY_PATH}"
 )
 
 #
diff --git a/source/tests/metacall_dynlink_path_test/source/metacall_dynlink_path_test.cpp b/source/tests/metacall_dynlink_path_test/source/metacall_dynlink_path_test.cpp
index 2e2f31b80..27ac692e9 100644
--- a/source/tests/metacall_dynlink_path_test/source/metacall_dynlink_path_test.cpp
+++ b/source/tests/metacall_dynlink_path_test/source/metacall_dynlink_path_test.cpp
@@ -47,7 +47,7 @@ TEST_F(metacall_dynlink_path_test, DefaultConstructor)
 
 	ASSERT_NE((char *)NULL, (char *)path);
 
-	ASSERT_EQ((int)0, (int)strncmp(path, METACALL_LIBRARY_PATH, strlen(METACALL_LIBRARY_PATH)));
+	ASSERT_EQ((int)0, (int)strcmp(path, METACALL_LIBRARY_PATH));
 
 	free(path);