Browse Source

fix os2.process_exec on non-windows and add a smoke test

Laytan Laats 10 months ago
parent
commit
76806080ef
4 changed files with 78 additions and 35 deletions
  1. 5 3
      core/os/os2/pipe_posix.odin
  2. 44 32
      core/os/os2/process.odin
  3. 1 0
      tests/core/normal.odin
  4. 28 0
      tests/core/os/os2/process.odin

+ 5 - 3
core/os/os2/pipe_posix.odin

@@ -49,7 +49,7 @@ _pipe_has_data :: proc(r: ^File) -> (ok: bool, err: Error) {
 	if r == nil || r.impl == nil {
 		return false, nil
 	}
-	fd := posix.FD((^File_Impl)(r.impl).fd)
+	fd := __fd(r)
 	poll_fds := []posix.pollfd {
 		posix.pollfd {
 			fd = fd,
@@ -57,8 +57,10 @@ _pipe_has_data :: proc(r: ^File) -> (ok: bool, err: Error) {
 		},
 	}
 	n := posix.poll(raw_data(poll_fds), u32(len(poll_fds)), 0)
-	if n != 1 {
+	if n < 0 {
 		return false, _get_platform_error()
+	} else if n != 1 {
+		return false, nil
 	}
 	pipe_events := poll_fds[0].revents
 	if pipe_events >= {.IN} {
@@ -68,4 +70,4 @@ _pipe_has_data :: proc(r: ^File) -> (ok: bool, err: Error) {
 		return false, .Broken_Pipe
 	}
 	return false, nil
-}
+}

+ 44 - 32
core/os/os2/process.odin

@@ -371,16 +371,18 @@ process_exec :: proc(
 	loc := #caller_location,
 ) -> (
 	state: Process_State,
-	stdout: []u8,
-	stderr: []u8,
+	stdout: []byte,
+	stderr: []byte,
 	err: Error,
 ) {
 	assert(desc.stdout == nil, "Cannot redirect stdout when it's being captured", loc)
 	assert(desc.stderr == nil, "Cannot redirect stderr when it's being captured", loc)
+
 	stdout_r, stdout_w := pipe() or_return
 	defer close(stdout_r)
 	stderr_r, stderr_w := pipe() or_return
 	defer close(stdout_w)
+
 	process: Process
 	{
 		// NOTE(flysand): Make sure the write-ends are closed, regardless
@@ -392,44 +394,54 @@ process_exec :: proc(
 		desc.stderr = stderr_w
 		process = process_start(desc) or_return
 	}
+
 	stdout_builder := strings.builder_make(allocator) or_return
 	stderr_builder := strings.builder_make(allocator) or_return
-	read_data: for {
-		buf: [1024]u8
+
+	has_stdout, has_stderr: bool
+	read_data: for !has_stdout || !has_stderr {
+		buf: [1024]u8 = ---
 		n: int
 		has_data: bool
-		hangup := false
-		has_data, err = pipe_has_data(stdout_r)
-		if has_data {
-			n, err = read(stdout_r, buf[:])
-			strings.write_bytes(&stdout_builder, buf[:n])
-		}
-		switch err {
-		case nil: // nothing
-		case .Broken_Pipe:
-			hangup = true
-		case:
-			return
-		}
-		has_data, err = pipe_has_data(stderr_r)
-		if has_data {
-			n, err = read(stderr_r, buf[:])
-			strings.write_bytes(&stderr_builder, buf[:n])
-		}
-		switch err {
-		case nil: // nothing
-		case .Broken_Pipe:
-			hangup = true
-		case:
-			return
+
+		if !has_stdout {
+			has_data, err = pipe_has_data(stdout_r)
+			if has_data {
+				n, err = read(stdout_r, buf[:])
+				if strings.write_bytes(&stdout_builder, buf[:n]) != n {
+					err = .Out_Of_Memory
+				}
+			}
+			switch err {
+			case nil: // nothing
+			case .EOF, .Broken_Pipe:
+				stdout     = stdout_builder.buf[:]
+				has_stdout = true
+			case:
+				return
+			}
 		}
-		if hangup {
-			break read_data
+
+		if !has_stderr {
+			has_data, err = pipe_has_data(stderr_r)
+			if has_data {
+				n, err = read(stderr_r, buf[:])
+				if strings.write_bytes(&stderr_builder, buf[:n]) != n {
+					err = .Out_Of_Memory
+				}
+			}
+			switch err {
+			case nil: // nothing
+			case .EOF, .Broken_Pipe:
+				stderr     = stderr_builder.buf[:]
+				has_stderr = true
+			case:
+				return
+			}
 		}
 	}
+
 	err = nil
-	stdout = transmute([]u8) strings.to_string(stdout_builder)
-	stderr = transmute([]u8) strings.to_string(stderr_builder)
 	state = process_wait(process) or_return
 	return
 }

+ 1 - 0
tests/core/normal.odin

@@ -33,6 +33,7 @@ download_assets :: proc() {
 @(require) import "net"
 @(require) import "odin"
 @(require) import "os"
+@(require) import "os/os2"
 @(require) import "path/filepath"
 @(require) import "reflect"
 @(require) import "runtime"

+ 28 - 0
tests/core/os/os2/process.odin

@@ -0,0 +1,28 @@
+package tests_core_os_os2
+
+import    "base:runtime"
+
+import    "core:log"
+import os "core:os/os2"
+import    "core:testing"
+
+_ :: log
+
+@(test)
+test_process_exec :: proc(t: ^testing.T) {
+	state, stdout, stderr, err := os.process_exec({
+		command = {"echo", "hellope"},
+	}, context.allocator)
+	defer delete(stdout)
+	defer delete(stderr)
+
+	when (ODIN_OS not_in runtime.Odin_OS_Types{.Linux, .Darwin, .Windows}) {
+		testing.expect_value(t, err, os.General_Error.Unsupported)
+		_ = state
+	} else {
+		testing.expect_value(t, state.exited,  true)
+		testing.expect_value(t, state.success, true)
+		testing.expect_value(t, err, nil)
+		testing.expect_value(t, string(stdout), "hellope\n")
+	}
+}