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

libstd fuchsia fixes #64023

Merged
merged 4 commits into from
Sep 7, 2019
Merged

libstd fuchsia fixes #64023

merged 4 commits into from
Sep 7, 2019

Conversation

tmandry
Copy link
Member

@tmandry tmandry commented Aug 30, 2019

This fixes two bugs in libstd on Fuchsia:

  • zx_time_t was changed to an i64, but this never made it into libstd
  • When spawning processes where any of the stdio were null, libstd attempts to open /dev/null, which doesn't exist on Fuchsia

r? @cramertj

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 30, 2019
@tmandry tmandry changed the title libstd fuchsia fixes [WIP] libstd fuchsia fixes Aug 30, 2019
@@ -301,6 +314,7 @@ impl Stdio {
Ok((ChildStdio::Owned(theirs.into_fd()), Some(ours)))
}

#[cfg(not(target_os = "fuchsia"))]
Copy link
Member

Choose a reason for hiding this comment

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

I'm not in love with these inline cfgs, but they seem to match the way redox was treated above, and I don't think this would be more cleanly expressed via a separate file. 🤷‍♀️

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed.

@cramertj
Copy link
Member

LGTM-- why is this tagged WIP?

@tmandry
Copy link
Member Author

tmandry commented Aug 30, 2019

why is this tagged WIP?

Because I hit another issue with this implementation.. update coming soon!

@tmandry tmandry changed the title [WIP] libstd fuchsia fixes libstd fuchsia fixes Aug 31, 2019
@cramertj
Copy link
Member

cramertj commented Sep 4, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Sep 4, 2019

📌 Commit 5f91ad0 has been approved by cramertj

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2019
Centril added a commit to Centril/rust that referenced this pull request Sep 4, 2019
…mertj

libstd fuchsia fixes

This fixes two bugs in libstd on Fuchsia:

- `zx_time_t` was changed to an `i64`, but this never made it into libstd
- When spawning processes where any of the stdio were null, libstd attempts to open `/dev/null`, which doesn't exist on Fuchsia

r? @cramertj
bors added a commit that referenced this pull request Sep 4, 2019
Rollup of 10 pull requests

Successful merges:

 - #63166 (Add Result::cloned{,_err} and Result::copied{,_err})
 - #63930 (Account for doc comments coming from proc macros without spans)
 - #63985 (Stabilize pin_into_inner in 1.39.0)
 - #64023 (libstd fuchsia fixes)
 - #64030 (Fix unlock ordering in SGX synchronization primitives)
 - #64041 (use TokenStream rather than &[TokenTree] for built-in macros)
 - #64043 (Add some more tests for underscore imports)
 - #64092 (Update xLTO compatibility table in rustc book.)
 - #64120 (Move path parsing earlier)
 - #64123 (Added warning around code with reference to uninit bytes)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Sep 5, 2019
…mertj

libstd fuchsia fixes

This fixes two bugs in libstd on Fuchsia:

- `zx_time_t` was changed to an `i64`, but this never made it into libstd
- When spawning processes where any of the stdio were null, libstd attempts to open `/dev/null`, which doesn't exist on Fuchsia

r? @cramertj
use crate::sys::pipe::{self, AnonPipe};
use crate::sys_common::process::{CommandEnv, DefaultEnvKey};
use crate::collections::BTreeMap;

#[cfg(not(target_os = "fuchsia"))]
Copy link
Member

Choose a reason for hiding this comment

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

Since this file is shared amongst most platforms, would it be possible to avoid the fuchsia-specific cfgs? Could Null be used on all platforms and only at the very end it's lowered to /dev/null?

Centril added a commit to Centril/rust that referenced this pull request Sep 7, 2019
…mertj

libstd fuchsia fixes

This fixes two bugs in libstd on Fuchsia:

- `zx_time_t` was changed to an `i64`, but this never made it into libstd
- When spawning processes where any of the stdio were null, libstd attempts to open `/dev/null`, which doesn't exist on Fuchsia

r? @cramertj
Centril added a commit to Centril/rust that referenced this pull request Sep 7, 2019
…mertj

libstd fuchsia fixes

This fixes two bugs in libstd on Fuchsia:

- `zx_time_t` was changed to an `i64`, but this never made it into libstd
- When spawning processes where any of the stdio were null, libstd attempts to open `/dev/null`, which doesn't exist on Fuchsia

r? @cramertj
@bors
Copy link
Contributor

bors commented Sep 7, 2019

⌛ Testing commit 5f91ad0 with merge 0c5c836ae77483667ac410b7ffcf9969e5298a9e...

Centril added a commit to Centril/rust that referenced this pull request Sep 7, 2019
…mertj

libstd fuchsia fixes

This fixes two bugs in libstd on Fuchsia:

- `zx_time_t` was changed to an `i64`, but this never made it into libstd
- When spawning processes where any of the stdio were null, libstd attempts to open `/dev/null`, which doesn't exist on Fuchsia

r? @cramertj
@Centril
Copy link
Contributor

Centril commented Sep 7, 2019

@bors retry rolled up.

bors added a commit that referenced this pull request Sep 7, 2019
Rollup of 7 pull requests

Successful merges:

 - #64023 (libstd fuchsia fixes)
 - #64098 (Ensure edition lints and internal lints are enabled with deny-warnings=false)
 - #64139 (Migrate internal diagnostic registration to macro_rules)
 - #64226 (Aggregation of cosmetic changes made during work on REPL PRs: libsyntax)
 - #64227 (Aggregation of cosmetic changes made during work on REPL PRs: librustc)
 - #64235 (Upgrade env_logger to 0.6)
 - #64258 (compiletest: Match suffixed environments)

Failed merges:

r? @ghost
@rust-highfive
Copy link
Collaborator

Your PR failed (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Sep 7, 2019

⌛ Testing commit 5f91ad0 with merge 43a5ff4...

@bors bors merged commit 5f91ad0 into rust-lang:master Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants