Skip to content

Refactor core::run in order to address many of the issues... #6433

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

Merged
merged 3 commits into from
May 27, 2013

Conversation

Dretch
Copy link
Contributor

@Dretch Dretch commented May 12, 2013

...mentioned in #2625.

This change makes the module more oriented around
Process values instead of having to deal with process ids
directly.

Apart from issues mentioned in #2625, other changes include:

  • Changing the naming to be more consistent - Process/process
    is now used instead of a mixture of Program/program and
    Process/process.
  • More docs/tests.

Some io/scheduler related issues remain (mentioned in #2625). I am not sure how best to address these.

@Dretch
Copy link
Contributor Author

Dretch commented May 12, 2013

r? Is there anything I should change? Is this remotely the right thing to do?

@brson
Copy link
Contributor

brson commented May 12, 2013

This looks like a big improvement. Thanks!

I just opened #6436 that implies that there is going to be a lot more churn in core::run in the future. Essentially, all I/O in core is going to need two modes of operation - one using the native blocking APIs and one defering to the scheduler event loop.

@Dretch
Copy link
Contributor Author

Dretch commented May 14, 2013

@brson

The names unwrap_input, etc. aren't very suggestive to me, since unwrap usually implies transnferring ownership, and in light of input returning a new Writer, not an fd. Maybe we could call these input_fd or something.

No problem. I will push a new commit.

Is there a reason input is represented as a c_int (presumably an fd) and the others are a *FILE.

I don't really know (I didn't make that decision originally), but I can see a couple of things that might go someway to explaining it:

  • they (the std io handles) couldn't all be file descriptors (without changing core::io) because there is a io::fd_writer but no io::fd_reader.
  • they couldn't all be *libc::FILE because os::fdopen hard codes the "r" mode.

I am also going to push a commit to fix the test that failed on OS X.

@Dretch
Copy link
Contributor Author

Dretch commented May 15, 2013

I rebased due to conflicts... r?

@Dretch
Copy link
Contributor Author

Dretch commented May 18, 2013

I just did another rebase to fix the merge issues that bors complained about. r?

@Dretch
Copy link
Contributor Author

Dretch commented May 21, 2013

I just did another preemptive rebase. r?

gareth added 3 commits May 27, 2013 13:50
mentioned in rust-lang#2625.

This change makes the module more oriented around
Process values instead of having to deal with process ids
directly.

Apart from issues mentioned in rust-lang#2625, other changes include:
- Changing the naming to be more consistent - Process/process
  is now used instead of a mixture of Program/program and
  Process/process.
- More docs/tests.

Some io/scheduler related issues remain (mentioned in rust-lang#2625).
directory to be the parent of the current-current directory,
instead of changing to the tmp directory, which was causing
issues with OS X and its /tmp => /private/tmp symlink.
@brson. Also fix a few documentation bugs.
@Dretch
Copy link
Contributor Author

Dretch commented May 27, 2013

r?

bors added a commit that referenced this pull request May 27, 2013
...mentioned in #2625.

This change makes the module more oriented around
Process values instead of having to deal with process ids
directly.

Apart from issues mentioned in #2625, other changes include:
- Changing the naming to be more consistent - Process/process
  is now used instead of a mixture of Program/program and
  Process/process.
- More docs/tests.

Some io/scheduler related issues remain (mentioned in #2625). I am not sure how best to address these.
@bors bors closed this May 27, 2013
@bors bors merged commit 04a3935 into rust-lang:incoming May 27, 2013
@Dretch Dretch deleted the run-refactor branch May 27, 2013 14:17
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.

4 participants