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.

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

(enh) adds Process.exec(cmd, [args], [cwd],[env]) #94

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

joshgoebel
Copy link
Contributor

@joshgoebel joshgoebel commented Apr 28, 2021

import "os" for Process
Process.exec("echo", ["how","are","you"])
  • It's not connected to any input/output streams.
  • Return value is the process's exit code.
  • This will sleep until the process finishes.

This was referenced Apr 28, 2021
@joshgoebel
Copy link
Contributor Author

Notes:

Process.exec(binary, args) { exec(binary, args, null, null) } , 
Process.exec(binary, args, cwd) { exec(binary, args, cwd, null) }
Process.exec(binary, args, cwd, env) { exec_ ... }

@joshgoebel joshgoebel marked this pull request as draft May 3, 2021 00:22
@joshgoebel
Copy link
Contributor Author

I think I know what you mean about tests now... I'd wager the python stuff doesn't understand the structure of the Wren code so while you can have the different branches doing slightly different things, the output needs to match for every single platform? Anyways, the tests still feel messy to me, but they cover the most basic cases. Since we don't need absolute paths I wager they likely work on other *nix as well, hence my making isWindows the special case.

We just need to find equivalencies on Windows. All tests pass for me here on OS X.

Issues:

  • I had to change the stderr to stdout to get it within the scope of tests. If there is some other way to deal with this I don't know what it is.
  • Are these errors that can happen runtime errors? We're not using the expect error and I'm not 100% sure when we're suppose to, etc.

I didn't spend too much time trying to understand the python test runner. I stopped when I had the basic tests cases covered. I'm pretty frustrated with the testing tooling and think it'd be much nicer if we picked any one of the Wren testing frameworks instead and switched. Perhaps a topic for another time.

@joshgoebel
Copy link
Contributor Author

rebased on main

@joshgoebel joshgoebel marked this pull request as ready for review May 3, 2021 01:04
src/module/os.c Outdated
{
free(env);
index += 1;
env = data->options.args[index];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in later commit.

@joshgoebel joshgoebel changed the title adds Process.exec(cmd, [args]) adds Process.exec(cmd, [args], [cwd],[env]) May 3, 2021
@joshgoebel joshgoebel changed the title adds Process.exec(cmd, [args], [cwd],[env]) (enh) adds Process.exec(cmd, [args], [cwd],[env]) May 10, 2021
@joshgoebel
Copy link
Contributor Author

joshgoebel commented May 27, 2021

Ruby, inline doesn't seem to work for me here:

Undefined symbols for architecture x86_64:
  "_cli_strdup", referenced from:
      _processExec in os.o
ld: symbol(s) not found for architecture x86_64

Removing inline allows the compile to proceed normally. Weird. Any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants