From: Jean Privat Date: Mon, 20 Mar 2017 20:08:59 +0000 (-0400) Subject: Merge: Windows: path, PATH, and exec improvements X-Git-Url: http://nitlanguage.org?hp=02ccb44ca23e9b730b6058fefdfb5eb9f1d64f9c Merge: Windows: path, PATH, and exec improvements This PR further improves Windows support in the Nit standard library and testing tools: * Improve Windows path manipulation and standardize the solution. On Windows, services working on path strings first replace all `\` by `/`. This allows sharing the main code with other OS, data copy should be limited since only the first operation on a string should update it. All `file` services depending on the path separator should have been updated, except for `relpath`. Note that there is still no special treatment for FS root like `C:/`, so further work is needed. * Support for Windows' PATH where the separator is `;`. * Fix pipes left open on the parent side after `CreateProcess`. * Intro a basic skip list for MINGW64 Windows system with projects that are either incompatible (cocoa & ios) or not available with msys2 and that I don't plan to configure manually on the test server in the near future. There are still issues (including an int overflow in `Float::to_s`), but this PR allows a few larger projects to work on Windows. With #2390 gamnit projects can be compiled entirely from a Windows machine (or cross-compiled). --- The unit tests are marked `~~~nitish` because they are only valid when `is_windows == true`. Maybe we could have a `~~~nitwindows` or something like that for such tests in the future. Note that you can set `-D is_windows=true` to try them (but not on nitunit sadly). The Jenkins Windows test server is currently broken. I'll see if I can fix it, otherwise, I'll share the results form my test server later. Pull-Request: #2391 Reviewed-by: Jean-Christophe Beaupré --- diff --git a/lib/core/environ.nit b/lib/core/environ.nit index 941bd1c..4ef4c99 100644 --- a/lib/core/environ.nit +++ b/lib/core/environ.nit @@ -46,10 +46,12 @@ redef class String # Search for the program `self` in all directories from `PATH` fun program_is_in_path: Bool do + var sep = if is_windows then ";" else ":" var full_path = "PATH".environ - var paths = full_path.split(":") + var paths = full_path.split(sep) for path in paths do if path.file_exists then if path.join_path(self).file_exists then return true + if is_windows and (path / self + ".exe").file_exists then return true end return false diff --git a/lib/core/exec.nit b/lib/core/exec.nit index d5ab952..fedc5de 100644 --- a/lib/core/exec.nit +++ b/lib/core/exec.nit @@ -164,7 +164,7 @@ class Process return NULL; } start_info.hStdInput = in_fd[0]; - result->in_fd = _open_osfhandle((intptr_t)in_fd[1], _O_APPEND); + result->in_fd = _open_osfhandle((intptr_t)in_fd[1], _O_WRONLY); if ( !SetHandleInformation(in_fd[1], HANDLE_FLAG_INHERIT, 0) ) return NULL; } else { @@ -214,6 +214,10 @@ class Process &start_info, &proc_info); + if (pipeflag & 1) CloseHandle(in_fd[0]); + if (pipeflag & 2) CloseHandle(out_fd[1]); + if (pipeflag & 3) CloseHandle(err_fd[1]); + // Error? if (!created) { result->running = 0; diff --git a/lib/core/file.nit b/lib/core/file.nit index 6701588..09b20d9 100644 --- a/lib/core/file.nit +++ b/lib/core/file.nit @@ -963,30 +963,28 @@ redef class String # assert "path/to".basename(".ext") == "to" # assert "path/to/".basename(".ext") == "to" # assert "path/to".basename == "to" - # assert "path".basename("") == "path" - # assert "/path".basename("") == "path" - # assert "/".basename("") == "/" - # assert "".basename("") == "" + # assert "path".basename == "path" + # assert "/path".basename == "path" + # assert "/".basename == "/" + # assert "".basename == "" + # + # On Windows, '\' are replaced by '/': + # + # ~~~nitish + # assert "C:\\path\\to\\a_file.ext".basename(".ext") == "a_file" + # assert "C:\\".basename == "C:" + # ~~~ fun basename(extension: nullable String): String do var n = self - if is_windows then - var l = length - 1 # Index of the last char - while l > 0 and (self.chars[l] == '/' or chars[l] == '\\') do l -= 1 # remove all trailing `/` - if l == 0 then return "/" - var pos = chars.last_index_of_from('/', l) - pos = pos.max(last_index_of_from('\\', l)) - if pos >= 0 then - n = substring(pos+1, l-pos) - end - else - var l = length - 1 # Index of the last char - while l > 0 and self.chars[l] == '/' do l -= 1 # remove all trailing `/` - if l == 0 then return "/" - var pos = chars.last_index_of_from('/', l) - if pos >= 0 then - n = substring(pos+1, l-pos) - end + if is_windows then n = n.replace("\\", "/") + + var l = length - 1 # Index of the last char + while l > 0 and self.chars[l] == '/' do l -= 1 # remove all trailing `/` + if l == 0 then return "/" + var pos = chars.last_index_of_from('/', l) + if pos >= 0 then + n = substring(pos+1, l-pos) end if extension != null then @@ -1004,13 +1002,23 @@ redef class String # assert "/path".dirname == "/" # assert "/".dirname == "/" # assert "".dirname == "." + # + # On Windows, '\' are replaced by '/': + # + # ~~~nitish + # assert "C:\\path\\to\\a_file.ext".dirname == "C:/path/to" + # assert "C:\\file".dirname == "C:" + # ~~~ fun dirname: String do + var s = self + if is_windows then s = s.replace("\\", "/") + var l = length - 1 # Index of the last char - while l > 0 and self.chars[l] == '/' do l -= 1 # remove all trailing `/` - var pos = chars.last_index_of_from('/', l) + while l > 0 and s.chars[l] == '/' do l -= 1 # remove all trailing `/` + var pos = s.chars.last_index_of_from('/', l) if pos > 0 then - return substring(0, pos) + return s.substring(0, pos) else if pos == 0 then return "/" else @@ -1055,10 +1063,18 @@ redef class String # assert "./../dir".simplify_path == "../dir" # assert "./dir".simplify_path == "dir" # ~~~ + # + # On Windows, '\' are replaced by '/': + # + # ~~~nitish + # assert "C:\\some\\.\\complex\\../../path/to/a_file.ext".simplify_path == "C:/path/to/a_file.ext" + # assert "C:\\".simplify_path == "C:" + # ~~~ fun simplify_path: String do - var path_sep = if is_windows then "\\" else "/" - var a = self.split_with(path_sep) + var s = self + if is_windows then s = s.replace("\\", "/") + var a = s.split_with("/") var a2 = new Array[String] for x in a do if x == "." and not a2.is_empty then continue # skip `././` @@ -1183,6 +1199,7 @@ redef class String # assert "/" + "/".relpath(".") == getcwd fun relpath(dest: String): String do + # TODO windows support var cwd = getcwd var from = (cwd/self).simplify_path.split("/") if from.last.is_empty then from.pop # case for the root directory @@ -1215,8 +1232,10 @@ redef class String fun mkdir(mode: nullable Int): nullable Error do mode = mode or else 0o777 + var s = self + if is_windows then s = s.replace("\\", "/") - var dirs = self.split_with("/") + var dirs = s.split_with("/") var path = new FlatBuffer if dirs.is_empty then return null if dirs[0].is_empty then @@ -1235,7 +1254,7 @@ redef class String error = new IOError("Cannot create directory `{path}`: {sys.errno.strerror}") end end - var res = self.to_cstring.file_mkdir(mode) + var res = s.to_cstring.file_mkdir(mode) if not res and error == null then error = new IOError("Cannot create directory `{path}`: {sys.errno.strerror}") end @@ -1356,29 +1375,19 @@ redef class FlatString end redef fun basename(extension) do + var s = self + if is_windows then s = s.replace("\\", "/").as(FlatString) + var bname - if is_windows then - var l = last_byte - var its = _items - var min = _first_byte - var sl = '/'.ascii - var ls = '\\'.ascii - while l > min and (its[l] == sl or its[l] == ls) do l -= 1 - if l == min then return "\\" - var ns = l - while ns >= min and its[ns] != sl and its[ns] != ls do ns -= 1 - bname = new FlatString.with_infos(its, l - ns, ns + 1) - else - var l = last_byte - var its = _items - var min = _first_byte - var sl = '/'.ascii - while l > min and its[l] == sl do l -= 1 - if l == min then return "/" - var ns = l - while ns >= min and its[ns] != sl do ns -= 1 - bname = new FlatString.with_infos(its, l - ns, ns + 1) - end + var l = s.last_byte + var its = s._items + var min = s._first_byte + var sl = '/'.ascii + while l > min and its[l] == sl do l -= 1 + if l == min then return "/" + var ns = l + while ns >= min and its[ns] != sl do ns -= 1 + bname = new FlatString.with_infos(its, l - ns, ns + 1) return if extension != null then bname.strip_extension(extension) else bname end diff --git a/tests/MINGW64_NT.skip b/tests/MINGW64_NT.skip new file mode 100644 index 0000000..619b61b --- /dev/null +++ b/tests/MINGW64_NT.skip @@ -0,0 +1,16 @@ +cocoa_extern_types +cocoa_message_box +hello_cocoa +hello_ios +test_platform_ios +mnit +shoot_linux +dino_linux +ballz_linux +mpi +emscripten +neo_doxygen +neo4j +mongo +pernicious_numbers +frankuchredux diff --git a/tests/sav/test_exec.res b/tests/sav/test_exec.res index 3647533..fcfb6f7 100644 --- a/tests/sav/test_exec.res +++ b/tests/sav/test_exec.res @@ -1,7 +1,8 @@ A hello world! 0 -B hello world!0 +B hello world!true +0 C hello world! 0 diff --git a/tests/test_exec.nit b/tests/test_exec.nit index efb94b7..58b6343 100644 --- a/tests/test_exec.nit +++ b/tests/test_exec.nit @@ -24,6 +24,7 @@ print "" var ip = new ProcessReader("echo", "B hello world!") ip.read_line.output +ip.eof.output ip.wait print ip.status diff --git a/tests/tests.sh b/tests/tests.sh index 308ff84..2fbab89 100755 --- a/tests/tests.sh +++ b/tests/tests.sh @@ -154,6 +154,8 @@ else HOSTNAME="hostname -s" fi +UNAME=`uname | sed s/-.*//` + # $1 is the pattern of the test # $2 is the file to compare to # the result is: @@ -373,7 +375,7 @@ need_skip() fi # Skip by OS - local os_skip_file=`uname`.skip + local os_skip_file=$UNAME.skip if test -e $os_skip_file && echo "$1" | grep -f "$os_skip_file" >/dev/null 2>&1; then echo "=> $2: [skip os]" echo >>$xml "" @@ -530,7 +532,7 @@ case $engine in ;; esac -savdirs="sav/`$HOSTNAME` sav/`uname` sav/$engine $savdirs sav/" +savdirs="sav/`$HOSTNAME` sav/$UNAME sav/$engine $savdirs sav/" # The default nitc compiler [ -z "$NITC" ] && find_nitc