Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make stdin a psuedoterminal when isatty is set #1265

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Make stdin a psuedoterminal when isatty is set #1265

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf43ccc
Make stdin a psuedoterminal when isatty is set
JakeSiFive May 24, 2023
693d297
small fixes
JakeSiFive May 24, 2023
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
10 changes: 9 additions & 1 deletion src/runtime/job.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,9 +829,17 @@ static void launch(JobTable *jobtable) {
jobtable->imp->pipes[stdout_stream[0]] = entry;
jobtable->imp->pipes[stderr_stream[0]] = entry;
clock_gettime(CLOCK_REALTIME, &entry->job->start);
std::string stdin_file_str = "/dev/null";
int stdin_stream[2];
Copy link
Contributor

@V-FEXrt V-FEXrt May 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not going to block, but I really think this should be explicit via a tuple data in wakelang.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean exactly?

Copy link
Contributor

Choose a reason for hiding this comment

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

Mentioned in chat but something like

data StdInOption =
  DevNull
  Interactive
  File (filename)


def job = 
  makeExecPlan ......
  | setPlanStdinOption DevNull
  | runJob

if (!task.stdin_file.empty()) {
stdin_file_str = task.stdin_file;
} else if (task.is_atty) {
create_psuedoterminal(stdin_stream);
stdin_file_str = "#" + std::to_string(stdin_stream[0]);
}
std::stringstream prelude;
prelude << find_execpath() << "/../lib/wake/shim-wake" << '\0'
<< (task.stdin_file.empty() ? "/dev/null" : task.stdin_file.c_str()) << '\0'
<< stdin_file_str << '\0'
<< std::to_string(stdout_stream[1]) << '\0' << std::to_string(stderr_stream[1]) << '\0'
<< task.dir << '\0';
std::string shim = prelude.str() + task.cmdline;
Expand Down
15 changes: 11 additions & 4 deletions tools/shim-wake/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,17 @@ int main(int argc, char **argv) {
return 127;
}

stdin_fd = open(argv[1], O_RDONLY);
if (stdin_fd == -1) {
fprintf(stderr, "open: %s: %s\n", argv[1], strerror(errno));
return 127;
// Because we want to be able to read in both file descirptors and
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: descirptors

// files for stdin, we use a '#' to denote a file descriptor.
const char* stdin_file = argv[1];
if (stdin_file[0] == '#') {
stdin_fd = atoi(stdin_file + 1);
} else {
stdin_fd = open(stdin_file, O_RDONLY);
if (stdin_fd == -1) {
fprintf(stderr, "open: %s: %s\n", argv[1], strerror(errno));
return 127;
Comment on lines +157 to +159
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this if after the if/else in case atoi fails for some reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

atoi does not tell you if it fails so we can't catch the error :(

Copy link
Contributor

Choose a reason for hiding this comment

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

I just meant something like

if (stdin_file[0] == '#') {
    stdin_fd = atoi(stdin_file + 1);
  } else {
    stdin_fd = open(stdin_file, O_RDONLY);
}

if (stdin_fd == -1) {
      fprintf(stderr, "open: %s: %s\n", argv[1], strerror(errno));
      return 127;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that's incorrect for the atoi case. The dup/dup2 below will fail if something is wrong with atoi....maybe the reality is that we should not being using atoi because its a terrible interface lol.

}
}

stdout_fd = atoi(argv[2]);
Expand Down