Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions nob.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ bool build_and_run_test(Cmd *cmd, const char *test_name)

const char *test_cwd_path = temp_sprintf("%s%s%s.cwd", BUILD_FOLDER, TESTS_FOLDER, test_name);
if (!mkdir_if_not_exists(test_cwd_path)) return false;
if (!set_current_dir(test_cwd_path)) return false;
cmd_append(cmd, temp_sprintf("../%s", test_name));
if (!cmd_run(cmd)) return false;
if (!set_current_dir("../../../")) return false;
cmd_append(cmd, temp_sprintf("%s%s%s", BUILD_FOLDER, TESTS_FOLDER, test_name));
if (!cmd_run(cmd, .pwd = test_cwd_path)) return false;
Comment on lines +44 to +45
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a mistake here.
Setting pwd to be test_cwd_path and then executing test_cwd_path/test_name doesn't make sense.
When pwd is test_cwd_path the relative path to the test would be ../test_name.

Suggested change
cmd_append(cmd, temp_sprintf("%s%s%s", BUILD_FOLDER, TESTS_FOLDER, test_name));
if (!cmd_run(cmd, .pwd = test_cwd_path)) return false;
cmd_append(cmd, temp_sprintf("../%s", test_name));
if (!cmd_run(cmd, .pwd = test_cwd_path)) return false;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code I wrote worked for windows,

CreateProcess expects the relative program path to be relative to the current working directory, not the passed in working directory. At no point does CreateProcessA search the target working directory for the application binary.

chdir in the child process makes exec not search the parent working directory. This makes the behavior between the two platforms incompatible for relative binary names.

I defaulted to windows' behavior because that's what I use.


nob_log(INFO, "--- %s finished ---", bin_path);

Expand Down
34 changes: 22 additions & 12 deletions nob.h
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,8 @@ typedef struct {
const char *stdout_path;
// Redirect stderr to file
const char *stderr_path;
// program working directory NULL means current
const char* pwd;
} Nob_Cmd_Opt;

// Run the command with options.
Expand Down Expand Up @@ -768,7 +770,7 @@ NOBDEF char *nob_win32_error_message(DWORD err);
static int nob__proc_wait_async(Nob_Proc proc, int ms);

// Starts the process for the command. Its main purpose is to be the base for nob_cmd_run() and nob_cmd_run_opt().
static Nob_Proc nob__cmd_start_process(Nob_Cmd cmd, Nob_Fd *fdin, Nob_Fd *fdout, Nob_Fd *fderr);
static Nob_Proc nob__cmd_start_process(Nob_Cmd cmd, Nob_Fd *fdin, Nob_Fd *fdout, Nob_Fd *fderr, const char* pwd);

// Any messages with the level below nob_minimal_log_level are going to be suppressed.
Nob_Log_Level nob_minimal_log_level = NOB_INFO;
Expand Down Expand Up @@ -1031,6 +1033,7 @@ NOBDEF bool nob_cmd_run_opt(Nob_Cmd *cmd, Nob_Cmd_Opt opt)
Nob_Fd *opt_fdin = NULL;
Nob_Fd *opt_fdout = NULL;
Nob_Fd *opt_fderr = NULL;
const char *opt_pwd = NULL;

size_t max_procs = opt.max_procs > 0 ? opt.max_procs : (size_t) nob_nprocs() + 1;

Expand Down Expand Up @@ -1062,7 +1065,10 @@ NOBDEF bool nob_cmd_run_opt(Nob_Cmd *cmd, Nob_Cmd_Opt opt)
if (fderr == NOB_INVALID_FD) nob_return_defer(false);
opt_fderr = &fderr;
}
Nob_Proc proc = nob__cmd_start_process(*cmd, opt_fdin, opt_fdout, opt_fderr);
if (opt.pwd) {
opt_pwd = opt.pwd;
}
Nob_Proc proc = nob__cmd_start_process(*cmd, opt_fdin, opt_fdout, opt_fderr, opt_pwd);

if (opt.async) {
if (proc == NOB_INVALID_PROC) nob_return_defer(false);
Expand Down Expand Up @@ -1104,10 +1110,10 @@ NOBDEF uint64_t nob_nanos_since_unspecified_epoch(void)

NOBDEF Nob_Proc nob_cmd_run_async_redirect(Nob_Cmd cmd, Nob_Cmd_Redirect redirect)
{
return nob__cmd_start_process(cmd, redirect.fdin, redirect.fdout, redirect.fderr);
return nob__cmd_start_process(cmd, redirect.fdin, redirect.fdout, redirect.fderr, NULL);
}

static Nob_Proc nob__cmd_start_process(Nob_Cmd cmd, Nob_Fd *fdin, Nob_Fd *fdout, Nob_Fd *fderr)
static Nob_Proc nob__cmd_start_process(Nob_Cmd cmd, Nob_Fd *fdin, Nob_Fd *fdout, Nob_Fd *fderr, const char* pwd)
{
if (cmd.count < 1) {
nob_log(NOB_ERROR, "Could not run empty command");
Expand Down Expand Up @@ -1140,7 +1146,7 @@ static Nob_Proc nob__cmd_start_process(Nob_Cmd cmd, Nob_Fd *fdin, Nob_Fd *fdout,

nob__win32_cmd_quote(cmd, &sb);
nob_sb_append_null(&sb);
BOOL bSuccess = CreateProcessA(NULL, sb.items, NULL, NULL, TRUE, 0, NULL, NULL, &siStartInfo, &piProcInfo);
BOOL bSuccess = CreateProcessA(NULL, sb.items, NULL, NULL, TRUE, 0, NULL, pwd, &siStartInfo, &piProcInfo);
nob_sb_free(sb);

if (!bSuccess) {
Expand Down Expand Up @@ -1186,6 +1192,10 @@ static Nob_Proc nob__cmd_start_process(Nob_Cmd cmd, Nob_Fd *fdin, Nob_Fd *fdout,
nob_da_append_many(&cmd_null, cmd.items, cmd.count);
nob_cmd_append(&cmd_null, NULL);

if (pwd) {
NOB_TODO("changing working directory not yet supported on linux")
}

Comment on lines +1195 to +1198
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested chdir on linux and it worked correctly.
I think you made a mistake in build_and_run_test that made it appear as if it did not work correctly.

Suggested change
if (pwd) {
NOB_TODO("changing working directory not yet supported on linux")
}
if (pwd) {
if (chdir(pwd) < 0){
nob_log(NOB_ERROR, "Could not change directory for child process: %s", strerror(errno));
exit(1);
}
}

if (execvp(cmd.items[0], (char * const*) cmd_null.items) < 0) {
nob_log(NOB_ERROR, "Could not exec child process for %s: %s", cmd.items[0], strerror(errno));
exit(1);
Expand All @@ -1199,19 +1209,19 @@ static Nob_Proc nob__cmd_start_process(Nob_Cmd cmd, Nob_Fd *fdin, Nob_Fd *fdout,

NOBDEF Nob_Proc nob_cmd_run_async(Nob_Cmd cmd)
{
return nob__cmd_start_process(cmd, NULL, NULL, NULL);
return nob__cmd_start_process(cmd, NULL, NULL, NULL, NULL);
}

NOBDEF Nob_Proc nob_cmd_run_async_and_reset(Nob_Cmd *cmd)
{
Nob_Proc proc = nob__cmd_start_process(*cmd, NULL, NULL, NULL);
Nob_Proc proc = nob__cmd_start_process(*cmd, NULL, NULL, NULL, NULL);
cmd->count = 0;
return proc;
}

NOBDEF Nob_Proc nob_cmd_run_async_redirect_and_reset(Nob_Cmd *cmd, Nob_Cmd_Redirect redirect)
{
Nob_Proc proc = nob__cmd_start_process(*cmd, redirect.fdin, redirect.fdout, redirect.fderr);
Nob_Proc proc = nob__cmd_start_process(*cmd, redirect.fdin, redirect.fdout, redirect.fderr, NULL);
cmd->count = 0;
if (redirect.fdin) {
nob_fd_close(*redirect.fdin);
Expand Down Expand Up @@ -1468,26 +1478,26 @@ NOBDEF bool nob_procs_append_with_flush(Nob_Procs *procs, Nob_Proc proc, size_t

NOBDEF bool nob_cmd_run_sync_redirect(Nob_Cmd cmd, Nob_Cmd_Redirect redirect)
{
Nob_Proc p = nob__cmd_start_process(cmd, redirect.fdin, redirect.fdout, redirect.fderr);
Nob_Proc p = nob__cmd_start_process(cmd, redirect.fdin, redirect.fdout, redirect.fderr, NULL);
return nob_proc_wait(p);
}

NOBDEF bool nob_cmd_run_sync(Nob_Cmd cmd)
{
Nob_Proc p = nob__cmd_start_process(cmd, NULL, NULL, NULL);
Nob_Proc p = nob__cmd_start_process(cmd, NULL, NULL, NULL, NULL);
return nob_proc_wait(p);
}

NOBDEF bool nob_cmd_run_sync_and_reset(Nob_Cmd *cmd)
{
Nob_Proc p = nob__cmd_start_process(*cmd, NULL, NULL, NULL);
Nob_Proc p = nob__cmd_start_process(*cmd, NULL, NULL, NULL, NULL);
cmd->count = 0;
return nob_proc_wait(p);
}

NOBDEF bool nob_cmd_run_sync_redirect_and_reset(Nob_Cmd *cmd, Nob_Cmd_Redirect redirect)
{
Nob_Proc p = nob__cmd_start_process(*cmd, redirect.fdin, redirect.fdout, redirect.fderr);
Nob_Proc p = nob__cmd_start_process(*cmd, redirect.fdin, redirect.fdout, redirect.fderr, NULL);
cmd->count = 0;
if (redirect.fdin) {
nob_fd_close(*redirect.fdin);
Expand Down