Based off #1095 and comments, plus some code I wrote ages ago (and have been 
using since then)

Main things
* Use `CreateProcessW()` on Windows
* Use an installed run script rather than a dynamically created one
* Fix replacing build command placeholders on non-Windows so that everything is 
always properly quoted.  This is based on some quoting code I wrote ages ago to 
fix a "security" issue we have that a very special filename can execute 
arbitrary commands.  This has 2 main advantages
  * placeholders replacements are always properly quoted (minus bugs in the 
code, but I'm using it for a while now in my own machine: initial version is 
from 2012 and I had one single relevant fix since then, more than one year ago)
  * placeholders in a placeholder replacement isn't replaced again.  Currently 
in master, each pattern is replaced, and then the next, lading to possibly 
finding placeholders inside a previous replacement.  With my patch, it's 
replaced in one go, and only from the source string.
  Also something like this is required to be able to replace "complex" 
placeholders, like full commands to pass to `sh -c` like we do in the run 
command.  Without something clever, it's virtually impossible to get correct 
quoting for this.
  
  A better way to fix it *could* be to replace placeholders after having split 
the arguments; however it requires lot of care and possibly conditional code 
too, in order to support all of *NIX, Window, and VTE.  And there would have to 
be an argv-to-commandline anyway, as we need to have a way to pass an argument 
to `sh -c`.
  
  Good Windows support is missing, and currently no extra quoting is done here, 
so no changes on the placeholders there apart from replacing all at once.

On Windows, it should fix all the spawning issues related to non-ASCII 
filenames.  On Linux, it should not change much apart from properly replacing 
placeholders, and possibly some uncommon things like if the run script could 
not be created for some reason.

**Warning:** currently the new run script doesn't `cd` to the working directory 
configured in the build command, and this relies on us setting the proper 
working directory when spawning the command itself.  If this breaks anything 
(like possibly a *.profile* that `cd`s somewhere?), it could be added back.
You can view, comment on, or merge this pull request online at:

  https://github.com/geany/geany/pull/1300

-- Commit Summary --

  * Windows: Convert working directory into locale encoding before spawning 
commands
  * Windows: Change codepage for directory change in created batch run script
  * Windows: Convert command into locale encoding before executing
  * Windows: Convert generated batch script into system codepage
  * spawn: Fix Windows environment encoding and other related fixes
  * spawn: Fix Windows environment building (oops)
  * spawn: convert working_directory only if non-NULL
  * Drop useless macro and use G_N_ELEMENTS() instead
  * Prevent undefined working directory and command
  * Use GetConsoleCP() to get the input console codepage
  * Merge remote-tracking branch 
'eht16/issue1076_win32_build_working_dir_locale' into eth16winspawnenc
  * spawn: Use Wide API on Windows
  * Merge branch 'issue1075_win32_build_working_dir_locale' into 
eth16winspawnenc
  * Use a program run helper rather than a script
  * Fix quoting of cmd.exe command
  * Implement autoclose feature in the run helper
  * Implement the run helper as a script
  * Fix autoclose in non-Windows run script (oops)
  * Windows: fix running a command when the helper has a space in its path
  * Fix invalid memory access (oops)
  * Windows: expand environment variables using UNICODE API
  * Windows: Expand environmen variables in build commands again
  * Don't try and unlink the run command, it's not a file anymore (oops)
  * Remove unused stuff
  * Escape replacements for build command placeholders
  * Fix quoting of the run helper path
  * Fix run command on Linux

-- File Changes --

    M src/Makefile.am (9)
    M src/build.c (380)
    A src/geany-run-helper (25)
    A src/geany-run-helper.bat (27)
    M src/prefix.h (1)
    M src/spawn.c (124)
    M src/utils.c (3)
    M src/utils.h (1)
    M src/win32.c (45)
    M src/win32.h (4)

-- Patch Links --

https://github.com/geany/geany/pull/1300.patch
https://github.com/geany/geany/pull/1300.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/1300

Reply via email to