Implement the run helper as a script

Apparently using arguments instead of putting paths directly in the
script is enough for it to work on Windows, so use a simple script
instead of a program, so it's both shorter and easier to tune.

src/Makefile.am

@@ -184,10 +184,12 @@ signallist.i: $(glade_file) Makefile
 
 CLEANFILES += signallist.i
 
-pkglibexec_PROGRAMS = geany-run-helper
+# install the run script
-geany_run_helper_SOURCES = geany-run-helper.c
+if MINGW
-geany_run_helper_CFLAGS = $(GTK_CFLAGS)
+pkglibexec_SCRIPTS = geany-run-helper.bat
-geany_run_helper_LDADD = $(GTK_LIBS)
+else
+pkglibexec_SCRIPTS = geany-run-helper
+endif
 
 # Ubuntu ld has a bug so that libtool sees /usr/local/lib as a system path so
 # doesn't add RPATH, but ld requires explicit ldconfig there, unlike when

src/build.c

@@ -79,12 +79,6 @@ typedef struct RunInfo
 
 static RunInfo *run_info;
 
-#ifdef G_OS_WIN32
-static const gchar RUN_SCRIPT_CMD[] = "geany_run_script_XXXXXX.bat";
-#else
-static const gchar RUN_SCRIPT_CMD[] = "geany_run_script_XXXXXX.sh";
-#endif
-
 /* pack group (<8) and command (<32) into a user_data pointer */
 #define GRP_CMD_TO_POINTER(grp, cmd) GUINT_TO_POINTER((((grp)&7) << 5) | ((cmd)&0x1f))
 #define GBO_TO_POINTER(gbo) (GRP_CMD_TO_POINTER(GBO_TO_GBG(gbo), GBO_TO_CMD(gbo)))
@@ -749,13 +743,15 @@ static void build_spawn_cmd(GeanyDocument *doc, const gchar *cmd, const gchar *d
 	msgwin_compiler_add(COLOR_BLUE, _("%s (in directory: %s)"), cmd, utf8_working_dir);
 	g_free(utf8_working_dir);
 
+#ifdef G_OS_UNIX
 	cmd_string = utils_get_locale_from_utf8(cmd);
 	argv[2] = cmd_string;
-
-#ifdef G_OS_UNIX
 	cmd = NULL;  /* under Unix, use argv to start cmd via sh for compatibility */
 #else
+	/* Expand environment variables like %blah%. */
+	cmd_string = win32_expand_environment_variables(cmd);
 	argv[0] = NULL;  /* under Windows, run cmd directly */
+	cmd = cmd_string;
 #endif
 
 	/* set the build info for the message window */
@@ -789,7 +785,6 @@ static gchar *prepare_run_cmd(GeanyDocument *doc, gchar **working_dir, guint cmd
 	const gchar *cmd_working_dir;
 	gboolean autoclose = FALSE;
 	gchar *cmd_string_utf8, *working_dir_utf8, *run_cmd, *cmd_string;
-	GError *error = NULL;
 
 	cmd = get_build_cmd(doc, GEANY_GBG_EXEC, cmdindex, NULL);
 
@@ -825,13 +820,22 @@ static gchar *prepare_run_cmd(GeanyDocument *doc, gchar **working_dir, guint cmd
 	}
 #endif
 
+#ifdef G_OS_WIN32
+	/* Expand environment variables like %blah%. */
+	SETPTR(cmd_string, win32_expand_environment_variables(cmd_string));
+#endif
+
 	gchar *helper = g_build_filename(utils_resource_dir(RESOURCE_DIR_LIBEXEC), "geany-run-helper", NULL);
-	// FIXME: should we expand environment variables? build_create_shell_script() did
+
-	///* Expand environment variables like %blah%. */
+	/* escape helper appropriately */
-	//expanded_cmd = win32_expand_environment_variables(cmd);
+#ifdef G_OS_WIN32
-	// FIXME: proper quoting of the helper (or not, because it ought to be a valid path,
+	/* FIXME: check the Windows rules, but it should not matter too much here as \es and "es are not
-	// and valid paths can't contain \es or "es, so it's fine.
+	 * allowed in paths anyway */
-	SETPTR(run_cmd, g_strdup_printf("\"%s\" %d %s", helper, !!autoclose, cmd_string));
+	SETPTR(helper, g_strdup_printf("\"%s\"", helper));
+#else
+	SETPTR(helper, g_shell_quote(helper));
+#endif
+	run_cmd = g_strdup_printf("%s %d %s", helper, autoclose ? 1 : 0, cmd_string);
 	g_free(helper);
 
 	utils_free_pointers(3, cmd_string_utf8, working_dir_utf8, cmd_string, NULL);
@@ -922,7 +926,6 @@ static void build_run_cmd(GeanyDocument *doc, guint cmdindex)
 				"Check the Terminal setting in Preferences"), utf8_term_cmd, error->message);
 			g_free(utf8_term_cmd);
 			g_error_free(error);
-			g_unlink(run_cmd);
 			run_info[cmdindex].pid = (GPid) 0;
 		}
 	}

src/geany-run-helper

@@ -0,0 +1,25 @@
+#!/bin/sh
+# USAGE: geany-run-helper AUTOCLOSE COMMAND...
+
+# save autoclose option and remove it
+autoclose=$1
+shift
+
+# spawn the child
+"$@"
+
+# show the result
+echo "
+
+------------------
+(program exited with code: $?)
+"
+
+# and if wanted, wait on the user
+if ! [ "$autoclose" = 0 ]
+then
+	echo "Press return to continue"
+	# to be more compatible with shells like dash
+	dummy_var=
+	read dummy_var
+fi

src/geany-run-helper.bat

@@ -0,0 +1,27 @@
+REM USAGE: geany-run-helper AUTOCLOSE COMMAND...
+
+REM save autoclose option and remove it
+set autoclose=%1
+shift
+
+REM spawn the child
+REM it's tricky because shift doesn't affect %*, so hack it out
+REM https://en.wikibooks.org/wiki/Windows_Batch_Scripting#Command-line_arguments
+set SPAWN=
+:argloop
+if -%1-==-- goto argloop_end
+	set SPAWN=%SPAWN% %1
+	shift
+goto argloop
+:argloop_end
+%SPAWN%
+
+REM show the result
+echo:
+echo:
+echo:------------------
+echo:(program exited with code: %ERRORLEVEL%)
+echo:
+
+REM and if wanted, wait on the user
+if not %autoclose%==1 pause

src/geany-run-helper.c

@@ -1,208 +0,0 @@
-/*
- *      geany-run-helper.c - this file is part of Geany, a fast and lightweight IDE
- *
- *      Copyright 2016 Colomban Wendling <ban(at)herbesfolles(dot)org>
- *
- *      This program is free software; you can redistribute it and/or modify
- *      it under the terms of the GNU General Public License as published by
- *      the Free Software Foundation; either version 2 of the License, or
- *      (at your option) any later version.
- *
- *      This program is distributed in the hope that it will be useful,
- *      but WITHOUT ANY WARRANTY; without even the implied warranty of
- *      MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *      GNU General Public License for more details.
- *
- *      You should have received a copy of the GNU General Public License along
- *      with this program; if not, write to the Free Software Foundation, Inc.,
- *      51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */
-
-/* Helper program to run a command, print its return code and wait on the user */
-
-#include <glib.h>
-#include <stdio.h>
-
-#ifdef G_OS_WIN32
-
-/*
- * Uses GetCommandLineW() and CreateProcessW().  It would be a lot shorter to use
- * _wspawnvp(), but like other argv-based Windows APIs (exec* family) it is broken
- * when it comes to "control" characters in the arguments like spaces and quotes:
- * it seems to basically do `CreateProcessW(" ".join(argv))`, which means it
- * re-interprets it as a command line a second time.
- *
- * Interesting read: https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
- *
- * FIXME: maybe just use spawn.c itself?  That would make the actual logic more
- * convoluted (trip around from commandline (UTF16) -> argv (UTF8) -> commandline (UTF16))
- * but would have all the spawn logic in one place.
- *
- * FIXME: handle cmd.exe's quoting rules?  Does that even apply on cmd.exe's
- * command line itself, or only inside the script? (it probably applies to the
- * argument of /C I guess).  That would mean to have a special mode for this,
- * just know we're calling it, or inspect the first argument of what we are
- * supposed to launch and figure out whether it's cmd.exe or not.  Darn it.
- */
-
-#include <windows.h>
-
-static void w32_perror(const gchar *prefix)
-{
-	gchar *msg = g_win32_error_message(GetLastError());
-	fprintf(stderr, "%s: %s\n", prefix, msg);
-	g_free(msg);
-}
-
-/* Based on spawn_get_program_name().
- * FIXME: this seems unable to handle an argument containing an escaped quote,
- * but OTOH we expect the cmdline to be valid and Windows doesn't allow quotes
- * in filenames */
-static LPWSTR w32_strip_first_arg(LPWSTR command_line)
-{
-	while (*command_line && wcschr(L" \t\r\n", *command_line))
-		command_line++;
-
-	if (*command_line == L'"')
-	{
-		command_line++;
-		LPWSTR p = wcschr(command_line, L'"');
-		if (p)
-			command_line = p + 1;
-		else
-			command_line = wcschr(command_line, L'\0');
-	}
-	else
-	{
-		while (*command_line && ! wcschr(L" \t", *command_line))
-			command_line++;
-	}
-
-	while (*command_line && wcschr(L" \t\r\n", *command_line))
-		command_line++;
-
-	return command_line;
-}
-
-int main(void)
-{
-	int exit_status = 1;
-	STARTUPINFOW startup;
-	PROCESS_INFORMATION process;
-	LPWSTR command_line = GetCommandLineW();
-	LPWSTR auto_close_arg;
-
-	ZeroMemory(&startup, sizeof startup);
-	startup.cb = sizeof startup;
-
-	auto_close_arg = command_line = w32_strip_first_arg(command_line); // strip argv[0]
-	command_line = w32_strip_first_arg(command_line); // strip argv[1]
-	if (! command_line || ! *command_line)
-		fprintf(stderr, "Invalid or missing command\n");
-	else if ((auto_close_arg[0] != L'0' && auto_close_arg[0] != L'1') ||
-	         ! isspace(auto_close_arg[1]))
-		fprintf(stderr, "USAGE: geany-run-script 0|1 command...\n");
-	else if (! CreateProcessW(NULL, command_line, NULL, NULL, TRUE, 0, NULL, NULL, &startup, &process))
-		w32_perror("CreateProcessW()");
-	else
-	{
-		DWORD code;
-
-		CloseHandle(process.hThread);
-		if (WaitForSingleObject(process.hProcess, INFINITE) == WAIT_FAILED)
-			w32_perror("WaitForSingleObject()");
-		else if (! GetExitCodeProcess(process.hProcess, &code))
-			w32_perror("GetExitCodeProcess()");
-		else
-		{
-			printf("\n\n------------------\n");
-			printf("(program exited with status %d)\n", code);
-			exit_status = code;
-		}
-		CloseHandle(process.hProcess);
-	}
-
-	if (*auto_close_arg != L'1')
-	{
-		printf("Press return to continue\n");
-		getc(stdin);
-	}
-
-	return exit_status;
-}
-
-#else
-
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <unistd.h>
-#include <stdio.h>
-#include <errno.h>
-
-int main(int argc, char **argv)
-{
-	int exit_status = 1;
-	const char *auto_close_arg;
-
-	if (argc < 3 || ((argv[1][0] != '0' && argv[1][0] != '1') || argv[1][1] != 0))
-	{
-		fprintf(stderr, "USAGE: %s 1|0 command...\n", argv[0]);
-		return 1;
-	}
-
-	auto_close_arg = argv[1];
-	/* strip argv[0] and auto-close argument */
-	argv += 2;
-	argc -= 2;
-
-	pid_t pid = fork();
-	if (pid < 0)
-		perror("fork()");
-	else if (pid == 0)
-	{
-		/* in the child */
-		execvp(*argv, argv);
-		perror("execvp()");
-		return 127;
-	}
-	else
-	{
-		int status;
-		int ret;
-
-		do
-		{
-			ret = waitpid(pid, &status, 0);
-		}
-		while (ret == -1 && errno == EINTR);
-
-		printf("\n\n------------------\n");
-		if (ret == -1)
-			perror("waitpid()");
-		else if (WIFEXITED(status))
-		{
-			printf("(program exited with status %d)\n", WEXITSTATUS(status));
-			exit_status = WEXITSTATUS(status);
-		}
-		else if (WIFSIGNALED(status))
-		{
-			printf("(program exited with signal %d)\n", WTERMSIG(status));
-#ifdef WCOREDUMP
-			if (WCOREDUMP(status))
-				printf("(core dumped)\n");
-#endif
-		}
-		else
-			fprintf(stderr, "something funky happened to the child\n");
-	}
-
-	if (*auto_close_arg != '1')
-	{
-		printf("Press return to continue\n");
-		getc(stdin);
-	}
-
-	return exit_status;
-}
-
-#endif

src/win32.c

@@ -891,14 +891,19 @@ void win32_init_debug_code(void)
 }
 
 
+/* expands environment placeholders in @str.  input and output is in UTF-8 */
 gchar *win32_expand_environment_variables(const gchar *str)
 {
-	gchar expCmdline[32768]; /* 32768 is the limit for ExpandEnvironmentStrings() */
+	wchar_t *cmdline = g_utf8_to_utf16(str, -1, NULL, NULL, NULL);
+	wchar_t expCmdline[32768]; /* 32768 is the limit for ExpandEnvironmentStrings() */
+	gchar *expanded = NULL;
 
-	if (ExpandEnvironmentStrings((LPCTSTR) str, (LPTSTR) expCmdline, sizeof(expCmdline)) != 0)
+	if (cmdline && ExpandEnvironmentStringsW(cmdline, expCmdline, sizeof(expCmdline)) != 0)
-		return g_strdup(expCmdline);
+		expanded = g_utf16_to_utf8(expCmdline, -1, NULL, NULL, NULL);
-	else
+
-		return g_strdup(str);
+	g_free(cmdline);
+
+	return expanded ? expanded : g_strdup(str);
 }
 
 
@@ -1032,34 +1037,4 @@ gchar *win32_get_user_config_dir(void)
 	return g_build_filename(g_get_user_config_dir(), "geany", NULL);
 }
 
-
-/* Retrieve the console codepage
- * In case GetConsoleCP() returns 0 (i.e. the application doesn't have an own console window
- * fallback to GetOEMCP(). */
-guint win32_get_console_codepage(void)
-{
-	guint codepage = GetConsoleCP();
-	if (codepage == 0)
-		codepage = GetOEMCP();
-	return codepage;
-}
-
-
-/* Convert a string into the system's default codepage, this is different from the
- * locale (e.g. default codepage is 850 but locale is CP1252).
- * This assumes the input string is encoded as UTF-8, otherwise a copy of
- * the input string is returned. */
-gchar *win32_convert_to_system_codepage(const gchar *str, GError **error)
-{
-	if (g_utf8_validate(str, -1, NULL))
-	{
-		guint codepage_code = win32_get_console_codepage();
-		gchar codepage[8] = { 0 };
-		g_snprintf(codepage, G_N_ELEMENTS(codepage), "%u", codepage_code);
-		return g_convert(str, -1, codepage, "utf-8", NULL, NULL, error);
-	}
-	else
-		return g_strdup(str);
-}
-
 #endif

src/win32.h

@@ -68,10 +68,6 @@ gchar *win32_expand_environment_variables(const gchar *str);
 
 gchar *win32_get_user_config_dir(void);
 
-guint win32_get_console_codepage(void);
-
-gchar *win32_convert_to_system_codepage(const gchar *str, GError **error);
-
 G_END_DECLS
 
 #endif /* G_OS_WIN32 */