Branch: refs/heads/master Author: Colomban Wendling ban@herbesfolles.org Committer: Enrico Tröger enrico.troeger@uvena.de Date: Sun, 13 Nov 2016 13:55:41 UTC Commit: 265e27a7f31b470f5845d8831def9b6a34776025 https://github.com/geany/geany/commit/265e27a7f31b470f5845d8831def9b6a347760...
Log Message: ----------- 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.
Modified Paths: -------------- src/Makefile.am src/build.c src/geany-run-helper.c src/prefix.h src/utils.c src/utils.h
Modified: src/Makefile.am 7 lines changed, 7 insertions(+), 0 deletions(-) =================================================================== @@ -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
Modified: src/build.c 102 lines changed, 23 insertions(+), 79 deletions(-) =================================================================== @@ -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 */
Modified: src/geany-run-helper.c 208 lines changed, 208 insertions(+), 0 deletions(-) =================================================================== @@ -0,0 +1,208 @@ +/* + * 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/eve... + * + * 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
Modified: src/prefix.h 1 lines changed, 1 insertions(+), 0 deletions(-) =================================================================== @@ -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 */
Modified: src/utils.c 3 lines changed, 3 insertions(+), 0 deletions(-) =================================================================== @@ -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 }
Modified: src/utils.h 1 lines changed, 1 insertions(+), 0 deletions(-) =================================================================== @@ -221,6 +221,7 @@ typedef enum RESOURCE_DIR_DOC, RESOURCE_DIR_LOCALE, RESOURCE_DIR_PLUGIN, + RESOURCE_DIR_LIBEXEC,
RESOURCE_DIR_COUNT } GeanyResourceDirType;
-------------- This E-Mail was brought to you by github_commit_mail.py (Source: https://github.com/geany/infrastructure).