Use a program run helper rather than a script

This removes all encoding issues from passing on a script to cmd.exe on
Windows, as it now uses proper wide character API there.

Not much changes on other OSes, but we don't create temporary scripts
anymore.

src/Makefile.am

@@ -138,6 +138,7 @@ AM_CPPFLAGS += \
 	-DGEANY_DATADIR=\"data\" \
 	-DGEANY_DOCDIR=\"\" \
 	-DGEANY_LIBDIR=\"\" \
+	-DGEANY_LIBEXECDIR=\"\" \
 	-DGEANY_LOCALEDIR=\"\" \
 	-DGEANY_PREFIX=\"\"
 
@@ -162,6 +163,7 @@ AM_CPPFLAGS += \
 	-DGEANY_DATADIR=\""$(datadir)"\" \
 	-DGEANY_DOCDIR=\""$(docdir)"\" \
 	-DGEANY_LIBDIR=\""$(libdir)"\" \
+	-DGEANY_LIBEXECDIR=\""$(libexecdir)"\" \
 	-DGEANY_LOCALEDIR=\""$(localedir)"\" \
 	-DGEANY_PREFIX=\""$(prefix)"\"
 
@@ -182,6 +184,11 @@ signallist.i: $(glade_file) Makefile
 
 CLEANFILES += signallist.i
 
+pkglibexec_PROGRAMS = geany-run-helper
+geany_run_helper_SOURCES = geany-run-helper.c
+geany_run_helper_CFLAGS = $(GTK_CFLAGS)
+geany_run_helper_LDADD = $(GTK_LIBS)
+
 # 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
 # installing in /usr/lib.  So, workaround this by calling it explicitly when

src/build.c

@@ -115,7 +115,6 @@ static guint build_items_count = 9;
 
 static void build_exit_cb(GPid pid, gint status, gpointer user_data);
 static void build_iofunc(GString *string, GIOCondition condition, gpointer data);
-static gchar *build_create_shellscript(const gchar *working_dir, const gchar *cmd, gboolean autoclose, GError **error);
 static void build_spawn_cmd(GeanyDocument *doc, const gchar *cmd, const gchar *dir);
 static void set_stop_button(gboolean stop);
 static void run_exit_cb(GPid child_pid, gint status, gpointer user_data);
@@ -826,14 +825,14 @@ static gchar *prepare_run_cmd(GeanyDocument *doc, gchar **working_dir, guint cmd
 	}
 #endif
 
-	run_cmd = build_create_shellscript(*working_dir, cmd_string, autoclose, &error);
-	if (!run_cmd)
-	{
-		ui_set_statusbar(TRUE, _("Failed to execute \"%s\" (start-script could not be created: %s)"),
-			!EMPTY(cmd_string_utf8) ? cmd_string_utf8 : NULL, error->message);
-		g_error_free(error);
-		g_free(*working_dir);
-	}
+	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%. */
+	//expanded_cmd = win32_expand_environment_variables(cmd);
+	// FIXME: proper quoting of the helper (or not, because it ought to be a valid path,
+	// and valid paths can't contain \es or "es, so it's fine.
+	SETPTR(run_cmd, g_strdup_printf("\"%s\" %d %s", helper, !!autoclose, cmd_string));
+	g_free(helper);
 
 	utils_free_pointers(3, cmd_string_utf8, working_dir_utf8, cmd_string, NULL);
 	return run_cmd;
@@ -892,6 +891,21 @@ static void build_run_cmd(GeanyDocument *doc, guint cmdindex)
 		gchar *locale_term_cmd = utils_get_locale_from_utf8(tool_prefs.term_cmd);
 		GError *error = NULL;
 
+#ifdef G_OS_WIN32
+		if (g_regex_match_simple("^[ \"]*cmd([.]exe)?[\" ]", locale_term_cmd, 0, 0))
+		{
+			/* if passing an argument to cmd.exe, respect its quoting rules */
+			GString *escaped_run_cmd = g_string_new(NULL);
+			for (gchar *p = run_cmd; *p; p++)
+			{
+				if (strchr("()%!^\"<>&|", *p)) // cmd.exe metacharacters
+					g_string_append_c(escaped_run_cmd, '^');
+				g_string_append_c(escaped_run_cmd, *p);
+			}
+			SETPTR(run_cmd, g_string_free(escaped_run_cmd, FALSE));
+		}
+#endif
+
 		utils_str_replace_all(&locale_term_cmd, "%c", run_cmd);
 
 		if (spawn_async(working_dir, locale_term_cmd, NULL, NULL, &(run_info[cmdindex].pid),
@@ -1059,76 +1073,6 @@ static void run_exit_cb(GPid child_pid, gint status, gpointer user_data)
 	build_menu_update(NULL);
 }
 
-
-/* write a little shellscript to call the executable (similar to anjuta_launcher but "internal")
- * working_dir and cmd are both in the locale encoding
- * it returns the full file name (including path) of the created script in the locale encoding */
-static gchar *build_create_shellscript(const gchar *working_dir, const gchar *cmd, gboolean autoclose, GError **error)
-{
-	gint fd;
-	gchar *str, *fname;
-	gboolean success = TRUE;
-#ifdef G_OS_WIN32
-	gchar *expanded_cmd;
-	gchar *utf8_script_content;
-#else
-	gchar *escaped_dir;
-#endif
-	fd = g_file_open_tmp (RUN_SCRIPT_CMD, &fname, error);
-	if (fd < 0)
-		return NULL;
-	close(fd);
-
-#ifdef G_OS_WIN32
-	/* Expand environment variables like %blah%. */
-	expanded_cmd = win32_expand_environment_variables(cmd);
-	str = g_strdup_printf(
-		"cd \"%s\"\n\n\n%s\n\n%s\ndel \"%%0\"\n\npause\n",
-		working_dir, expanded_cmd, (autoclose) ? "" : "pause");
-	g_free(expanded_cmd);
-	/* Convert script content into system codepage */
-	utf8_script_content = win32_convert_to_system_codepage(str, error);
-	if (utf8_script_content == NULL)
-		return NULL;
-	SETPTR(str, utf8_script_content);
-#else
-	escaped_dir = g_shell_quote(working_dir);
-	str = g_strdup_printf(
-		"#!/bin/sh\n\nrm $0\n\ncd %s\n\n%s\n\necho \"\n\n------------------\n(program exited with code: $?)\" \
-		\n\n%s\n", escaped_dir, cmd, (autoclose) ? "" :
-		"\necho \"Press return to continue\"\n#to be more compatible with shells like "
-			"dash\ndummy_var=\"\"\nread dummy_var");
-	g_free(escaped_dir);
-#endif
-
-	if (!g_file_set_contents(fname, str, -1, error))
-		success = FALSE;
-	g_free(str);
-#ifdef __APPLE__
-	if (success && g_chmod(fname, 0777) != 0)
-	{
-		if (error)
-		{
-			gint errsv = errno;
-
-			g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errsv),
-					"Failed to make file executable: %s", g_strerror(errsv));
-		}
-		success = FALSE;
-	}
-#endif
-
-	if (!success)
-	{
-		g_unlink(fname);
-		g_free(fname);
-		fname = NULL;
-	}
-
-	return fname;
-}
-
-
 typedef void Callback(GtkWidget *w, gpointer u);
 
 /* run the command catenating cmd_cat if present */

src/geany-run-helper.c

+/*
+ *      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/prefix.h

@@ -81,6 +81,7 @@ G_BEGIN_DECLS
 	#define GEANY_PREFIX		(br_thread_local_store (br_locate_prefix ((void *) "")))
 	#define GEANY_DATADIR		(br_thread_local_store (br_prepend_prefix ((void *) "", "/share")))
 	#define GEANY_LIBDIR		(br_thread_local_store (br_prepend_prefix ((void *) "", "/lib")))
+	#define GEANY_LIBEXECDIR	(br_thread_local_store (br_prepend_prefix ((void *) "", "/libexec/geany")))
 	#define GEANY_DOCDIR		(br_thread_local_store (br_prepend_prefix ((void *) "", "/share/doc/geany")))
 	#define GEANY_LOCALEDIR		(br_thread_local_store (br_prepend_prefix ((void *) "", "/share/locale")))
 #endif /* BR_NO_MACROS */

src/utils.c

@@ -2103,6 +2103,7 @@ const gchar *utils_resource_dir(GeanyResourceDirType type)
 		resdirs[RESOURCE_DIR_DOC] = g_build_filename(prefix, "share", "doc", "geany", "html", NULL);
 		resdirs[RESOURCE_DIR_LOCALE] = g_build_filename(prefix, "share", "locale", NULL);
 		resdirs[RESOURCE_DIR_PLUGIN] = g_build_filename(prefix, "lib", "geany", NULL);
+		resdirs[RESOURCE_DIR_LIBEXEC] = g_build_filename(prefix, "libexec", "geany", NULL);
 		g_free(prefix);
 #else
 		if (is_osx_bundle())
@@ -2115,6 +2116,7 @@ const gchar *utils_resource_dir(GeanyResourceDirType type)
 			resdirs[RESOURCE_DIR_DOC] = g_build_filename(prefix, "share", "doc", "geany", "html", NULL);
 			resdirs[RESOURCE_DIR_LOCALE] = g_build_filename(prefix, "share", "locale", NULL);
 			resdirs[RESOURCE_DIR_PLUGIN] = g_build_filename(prefix, "lib", "geany", NULL);
+			resdirs[RESOURCE_DIR_LIBEXEC] = g_build_filename(prefix, "libexec", "geany", NULL);
 			g_free(prefix);
 # endif
 		}
@@ -2125,6 +2127,7 @@ const gchar *utils_resource_dir(GeanyResourceDirType type)
 			resdirs[RESOURCE_DIR_DOC] = g_build_filename(GEANY_DOCDIR, "html", NULL);
 			resdirs[RESOURCE_DIR_LOCALE] = g_build_filename(GEANY_LOCALEDIR, NULL);
 			resdirs[RESOURCE_DIR_PLUGIN] = g_build_filename(GEANY_LIBDIR, "geany", NULL);
+			resdirs[RESOURCE_DIR_LIBEXEC] = g_build_filename(GEANY_LIBEXECDIR, "geany", NULL);
 		}
 #endif
 	}

src/utils.h

@@ -221,6 +221,7 @@ typedef enum
 	RESOURCE_DIR_DOC,
 	RESOURCE_DIR_LOCALE,
 	RESOURCE_DIR_PLUGIN,
+	RESOURCE_DIR_LIBEXEC,
 
 	RESOURCE_DIR_COUNT
 } GeanyResourceDirType;