Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upRedirect stdout/stderr on windows #23922
Merged
Conversation
|
Sorry, to add issue 23734. |
jdm
changed the title
|
Thank you for reviewing the code. The suggestions are very good. I will implement the changes.
Angel
From: Josh Matthews <notifications@github.com>
Sent: Tuesday, August 6, 2019 8:17 PM
To: servo/servo <servo@noreply.github.com>
Cc: Angel Ortiz <aoble248@gmail.com>; Author <author@noreply.github.com>
Subject: Re: [servo/servo] Redirect stdout/stderr on windows (#23922)
@jdm requested changes on this pull request.
_____
In ports/libsimpleservo/capi/Cargo.toml <#23922 (comment)> :
@@ -16,6 +16,8 @@ bench = false
simpleservo = { path = "../api" }
log = "0.4"
env_logger = "0.6"
+libc = "0.2"
+winapi = {version = "0.3.7", features = ["winnt", "winbase", "processenv", "namedpipeapi", "ntdef", "minwindef", "handleapi", "debugapi"]}
We should use:
[target.'cfg(target_os = "windows")']
libc = "0.2"
winapi = {version = "0.3.7", features = ["winnt", "winbase", "processenv", "namedpipeapi", "ntdef", "minwindef", "handleapi", "debugapi"]}
This will ensure that the build still works on non-windows platforms.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
/// Catch any panic function used by extern "C" functions.
fn catch_any_panic<F: FnOnce() + UnwindSafe>(function: F) -> bool {
panic::catch_unwind(function).is_ok()
}
+// Function to redirect STDOUT (1) and STDERR(2) to Windows API
+// OutputDebugString().
+// Return Value: 0 - no stdout or stderr redirects
+// 1 - stdout and stderr redirects.
+// If global variable - static mut hReadPipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+// If global variable - static mut hWritePipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+fn redirect_stdout_stderr() -> bool {
We will need a version for windows and a version for non-windows:
#[cfg(target_os = "windows")]
fn redirect_stdout_stderr() -> Result<(), String> {
use winapi::shared::minwindef::TRUE;
use winapi::shared::ntdef::NULL;
use winapi::um::winbase::{STD_OUTPUT_HANDLE, STD_ERROR_HANDLE};
// etc.
}
#[cfg(not(target_os = "windows"))]
fn redirect_stdout_stderr() -> Result<(), String> {
Ok(())
}
We can move all of the new module imports into the windows-only code to avoid unused warnings on other platforms.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
/// Catch any panic function used by extern "C" functions.
fn catch_any_panic<F: FnOnce() + UnwindSafe>(function: F) -> bool {
panic::catch_unwind(function).is_ok()
}
+// Function to redirect STDOUT (1) and STDERR(2) to Windows API
+// OutputDebugString().
+// Return Value: 0 - no stdout or stderr redirects
+// 1 - stdout and stderr redirects.
+// If global variable - static mut hReadPipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+// If global variable - static mut hWritePipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+fn redirect_stdout_stderr() -> bool {
+ let mut _h_read_pipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+ let mut _h_write_pipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+ let mut _secattr: minwinbase::SECURITY_ATTRIBUTES = unsafe { mem::zeroed() };
Only prefix variable names with _ when they are unused. Since these are all used, they don't need it.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
/// Catch any panic function used by extern "C" functions.
fn catch_any_panic<F: FnOnce() + UnwindSafe>(function: F) -> bool {
panic::catch_unwind(function).is_ok()
}
+// Function to redirect STDOUT (1) and STDERR(2) to Windows API
+// OutputDebugString().
+// Return Value: 0 - no stdout or stderr redirects
+// 1 - stdout and stderr redirects.
Let's use Result<(), String> instead. The Err value can contain the string value of GetLastError.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
@@ -117,6 +223,16 @@ unsafe fn init(
) {
init_logger();
+ if redirect_stdout_stderr() == false {
Let's do this instead:
if let Err(reason) = redirect_stdout_stderr() {
warn!("Error redirecting stdout/stderr: {}", reason);
}
And earlier, we can use the GetLastError call like this:
fn redirect_stdout_stderr() -> Result<(), String> {
do_redirect_stdout_stderr()
.map_err(|()| format!("GetLastError() = {}", winapi::um::errhandlingapi::GetLastError())
}
fn do_redirect_stdout_stderr() -> Result<(), ()> {
// actual implementation that returns Err(()) for problems
}
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
+ winbase::HANDLE_FLAG_INHERIT,
+ ) == 0
+ {
+ return false;
+ }
+
+ let _h_read_pipe_fd = libc::open_osfhandle(_h_read_pipe as libc::intptr_t, libc::O_RDONLY);
+ let _h_write_pipe_fd =
+ libc::open_osfhandle(_h_write_pipe as libc::intptr_t, libc::O_WRONLY);
+
+ if _h_read_pipe_fd == -1 && _h_write_pipe_fd == -1 {
+ return false;
+ }
+
+ // 0 indicates success.
+ if libc::dup2(_h_write_pipe_fd, 1) != 0 && libc::dup2(_h_write_pipe_fd, 2) != 0 {
This should probably be ||.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
+ }
+
+ // If SetStdHandle(winbase::STD_ERROR_HANDLE, hWritePipe) is not called prior,
+ // this will fail. GetStdHandle() is used to make certain "servo" has the stderr
+ // file descriptor associated.
+ let _h_stderr = processenv::GetStdHandle(winbase::STD_ERROR_HANDLE);
+ if _h_stderr == handleapi::INVALID_HANDLE_VALUE || _h_stderr == shared::ntdef::NULL {
+ return false;
+ }
+
+ // Spawn a thread. The thread will redirect all STDOUT and STDERR messages
+ // to OutputDebugString()
+ let _handler = thread::spawn(move || {
+ loop {
+ const BUF_LENGTH: usize = 1024;
+ let mut read_buf: [i8; BUF_LENGTH] = mem::zeroed();
We should use let mut read_buf: [i8; BUF_LENGTH] = [0; BUF_LENGTH]; instead.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
+ // If SetStdHandle(winbase::STD_ERROR_HANDLE, hWritePipe) is not called prior,
+ // this will fail. GetStdHandle() is used to make certain "servo" has the stderr
+ // file descriptor associated.
+ let _h_stderr = processenv::GetStdHandle(winbase::STD_ERROR_HANDLE);
+ if _h_stderr == handleapi::INVALID_HANDLE_VALUE || _h_stderr == shared::ntdef::NULL {
+ return false;
+ }
+
+ // Spawn a thread. The thread will redirect all STDOUT and STDERR messages
+ // to OutputDebugString()
+ let _handler = thread::spawn(move || {
+ loop {
+ const BUF_LENGTH: usize = 1024;
+ let mut read_buf: [i8; BUF_LENGTH] = mem::zeroed();
+
+ let _result = {
We should do this:
let result = libc::read(...);
if result == -1 {
break;
}
OutputDebugStringA(...);
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
+ if _h_stderr == handleapi::INVALID_HANDLE_VALUE || _h_stderr == shared::ntdef::NULL {
+ return false;
+ }
+
+ // Spawn a thread. The thread will redirect all STDOUT and STDERR messages
+ // to OutputDebugString()
+ let _handler = thread::spawn(move || {
+ loop {
+ const BUF_LENGTH: usize = 1024;
+ let mut read_buf: [i8; BUF_LENGTH] = mem::zeroed();
+
+ let _result = {
+ libc::read(
+ _h_read_pipe_fd,
+ read_buf.as_mut_ptr() as *mut _,
+ read_buf.len() as u32,
We should pass read_buf.len() as u32 - 1 to ensure that there's always a terminating null byte, since OutputDebugString requires that.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#23922?email_source=notifications&email_token=AMJITWBVONVZWJFQ3QKUBATQDIPALA5CNFSM4IJYKLMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCAX5DMI#pullrequestreview-271569329> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AMJITWDMODHV562JEJXVMNDQDIPALANCNFSM4IJYKLMA> . <https://github.com/notifications/beacon/AMJITWAWIHIHOLRTR6FS7WTQDIPALA5CNFSM4IJYKLMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCAX5DMI.gif>
|
|
Implemented all changes in dd7c4d9.
Also rand test-tidy, fmt, and ran tests to verify new functionality.
To verify the Result <(), String> GetLastError() returned in a String value:
1. Test Failure:
Remove the following from capi\lib.rs and build/run Servo App. By removing the following code from capi\lib.rs, later code will fail and return a GetLastError() = 6 converted to a string.
if processenv::SetStdHandle(winbase::STD_OUTPUT_HANDLE, h_write_pipe) == 0 ||
processenv::SetStdHandle(winbase::STD_ERROR_HANDLE, h_write_pipe) == 0
{
do_redirect_stdout_stderr().map_err(|()| {
format!(
"GetLastError() = {}",
winapi::um::errhandlingapi::GetLastError()
)
})?;
}
Upon failure, the fn init() function will receive an error string in the “reason” variable. The value will be a string representation of 0x06.
if let Err(reason) = redirect_stdout_stderr() {
warn!("Error redirecting stdout/stderr: {}", reason);
} else {
println!("Capi/lib.rs: init() function called redirect_stdout_stderr() successfully.\n");
}
2. Test Success: Add the deleted code specified in Test 1, build servo and then ServoApp. Execute ServoApp and verify the println!() line in fn init() is seen in the VS2017 Output window.
if let Err(reason) = redirect_stdout_stderr() {
warn!("Error redirecting stdout/stderr: {}", reason);
} else {
println!("Capi/lib.rs: init() function called redirect_stdout_stderr() successfully.\n");
}
From: Josh Matthews <notifications@github.com>
Sent: Tuesday, August 6, 2019 8:17 PM
To: servo/servo <servo@noreply.github.com>
Cc: Angel Ortiz <aoble248@gmail.com>; Author <author@noreply.github.com>
Subject: Re: [servo/servo] Redirect stdout/stderr on windows (#23922)
@jdm requested changes on this pull request.
_____
In ports/libsimpleservo/capi/Cargo.toml <#23922 (comment)> :
@@ -16,6 +16,8 @@ bench = false
simpleservo = { path = "../api" }
log = "0.4"
env_logger = "0.6"
+libc = "0.2"
+winapi = {version = "0.3.7", features = ["winnt", "winbase", "processenv", "namedpipeapi", "ntdef", "minwindef", "handleapi", "debugapi"]}
We should use:
[target.'cfg(target_os = "windows")']
libc = "0.2"
winapi = {version = "0.3.7", features = ["winnt", "winbase", "processenv", "namedpipeapi", "ntdef", "minwindef", "handleapi", "debugapi"]}
This will ensure that the build still works on non-windows platforms.
Suggested change Complete.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
/// Catch any panic function used by extern "C" functions.
fn catch_any_panic<F: FnOnce() + UnwindSafe>(function: F) -> bool {
panic::catch_unwind(function).is_ok()
}
+// Function to redirect STDOUT (1) and STDERR(2) to Windows API
+// OutputDebugString().
+// Return Value: 0 - no stdout or stderr redirects
+// 1 - stdout and stderr redirects.
+// If global variable - static mut hReadPipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+// If global variable - static mut hWritePipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+fn redirect_stdout_stderr() -> bool {
We will need a version for windows and a version for non-windows:
#[cfg(target_os = "windows")]
fn redirect_stdout_stderr() -> Result<(), String> {
use winapi::shared::minwindef::TRUE;
use winapi::shared::ntdef::NULL;
use winapi::um::winbase::{STD_OUTPUT_HANDLE, STD_ERROR_HANDLE};
// etc.
}
Suggested change Complete.
#[cfg(not(target_os = "windows"))]
fn redirect_stdout_stderr() -> Result<(), String> {
Ok(())
}
Suggested change Complete.
We can move all of the new module imports into the windows-only code to avoid unused warnings on other platforms.
Suggested change Complete.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
/// Catch any panic function used by extern "C" functions.
fn catch_any_panic<F: FnOnce() + UnwindSafe>(function: F) -> bool {
panic::catch_unwind(function).is_ok()
}
+// Function to redirect STDOUT (1) and STDERR(2) to Windows API
+// OutputDebugString().
+// Return Value: 0 - no stdout or stderr redirects
+// 1 - stdout and stderr redirects.
+// If global variable - static mut hReadPipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+// If global variable - static mut hWritePipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+fn redirect_stdout_stderr() -> bool {
+ let mut _h_read_pipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+ let mut _h_write_pipe: winnt::HANDLE = handleapi::INVALID_HANDLE_VALUE;
+ let mut _secattr: minwinbase::SECURITY_ATTRIBUTES = unsafe { mem::zeroed() };
Only prefix variable names with _ when they are unused. Since these are all used, they don't need it.
Suggested change Complete.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
/// Catch any panic function used by extern "C" functions.
fn catch_any_panic<F: FnOnce() + UnwindSafe>(function: F) -> bool {
panic::catch_unwind(function).is_ok()
}
+// Function to redirect STDOUT (1) and STDERR(2) to Windows API
+// OutputDebugString().
+// Return Value: 0 - no stdout or stderr redirects
+// 1 - stdout and stderr redirects.
Let's use Result<(), String> instead. The Err value can contain the string value of GetLastError.
Suggested change Complete.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
@@ -117,6 +223,16 @@ unsafe fn init(
) {
init_logger();
+ if redirect_stdout_stderr() == false {
Let's do this instead:
if let Err(reason) = redirect_stdout_stderr() {
warn!("Error redirecting stdout/stderr: {}", reason);
}
Suggested change Complete.
And earlier, we can use the GetLastError call like this:
fn redirect_stdout_stderr() -> Result<(), String> {
do_redirect_stdout_stderr()
.map_err(|()| format!("GetLastError() = {}", winapi::um::errhandlingapi::GetLastError())
}
fn do_redirect_stdout_stderr() -> Result<(), ()> {
// actual implementation that returns Err(()) for problems
}
Suggested change Complete.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
+ winbase::HANDLE_FLAG_INHERIT,
+ ) == 0
+ {
+ return false;
+ }
+
+ let _h_read_pipe_fd = libc::open_osfhandle(_h_read_pipe as libc::intptr_t, libc::O_RDONLY);
+ let _h_write_pipe_fd =
+ libc::open_osfhandle(_h_write_pipe as libc::intptr_t, libc::O_WRONLY);
+
+ if _h_read_pipe_fd == -1 && _h_write_pipe_fd == -1 {
+ return false;
+ }
+
+ // 0 indicates success.
+ if libc::dup2(_h_write_pipe_fd, 1) != 0 && libc::dup2(_h_write_pipe_fd, 2) != 0 {
This should probably be ||.
Suggested change Complete.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
+ }
+
+ // If SetStdHandle(winbase::STD_ERROR_HANDLE, hWritePipe) is not called prior,
+ // this will fail. GetStdHandle() is used to make certain "servo" has the stderr
+ // file descriptor associated.
+ let _h_stderr = processenv::GetStdHandle(winbase::STD_ERROR_HANDLE);
+ if _h_stderr == handleapi::INVALID_HANDLE_VALUE || _h_stderr == shared::ntdef::NULL {
+ return false;
+ }
+
+ // Spawn a thread. The thread will redirect all STDOUT and STDERR messages
+ // to OutputDebugString()
+ let _handler = thread::spawn(move || {
+ loop {
+ const BUF_LENGTH: usize = 1024;
+ let mut read_buf: [i8; BUF_LENGTH] = mem::zeroed();
We should use let mut read_buf: [i8; BUF_LENGTH] = [0; BUF_LENGTH]; instead.
Suggested change Complete.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
+ // If SetStdHandle(winbase::STD_ERROR_HANDLE, hWritePipe) is not called prior,
+ // this will fail. GetStdHandle() is used to make certain "servo" has the stderr
+ // file descriptor associated.
+ let _h_stderr = processenv::GetStdHandle(winbase::STD_ERROR_HANDLE);
+ if _h_stderr == handleapi::INVALID_HANDLE_VALUE || _h_stderr == shared::ntdef::NULL {
+ return false;
+ }
+
+ // Spawn a thread. The thread will redirect all STDOUT and STDERR messages
+ // to OutputDebugString()
+ let _handler = thread::spawn(move || {
+ loop {
+ const BUF_LENGTH: usize = 1024;
+ let mut read_buf: [i8; BUF_LENGTH] = mem::zeroed();
+
+ let _result = {
We should do this:
let result = libc::read(...);
if result == -1 {
break;
}
OutputDebugStringA(...);
Suggested change Complete.
Question: if we break on read() error, should we do another OutputDebugStringA() to state “read() error occurred on redirect_stdout_stderr()” or something else?
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
+ if _h_stderr == handleapi::INVALID_HANDLE_VALUE || _h_stderr == shared::ntdef::NULL {
+ return false;
+ }
+
+ // Spawn a thread. The thread will redirect all STDOUT and STDERR messages
+ // to OutputDebugString()
+ let _handler = thread::spawn(move || {
+ loop {
+ const BUF_LENGTH: usize = 1024;
+ let mut read_buf: [i8; BUF_LENGTH] = mem::zeroed();
+
+ let _result = {
+ libc::read(
+ _h_read_pipe_fd,
+ read_buf.as_mut_ptr() as *mut _,
+ read_buf.len() as u32,
We should pass read_buf.len() as u32 - 1 to ensure that there's always a terminating null byte, since OutputDebugString requires that.
Suggested change Complete.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#23922?email_source=notifications&email_token=AMJITWBVONVZWJFQ3QKUBATQDIPALA5CNFSM4IJYKLMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCAX5DMI#pullrequestreview-271569329> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AMJITWDMODHV562JEJXVMNDQDIPALANCNFSM4IJYKLMA> . <https://github.com/notifications/beacon/AMJITWAWIHIHOLRTR6FS7WTQDIPALA5CNFSM4IJYKLMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCAX5DMI.gif>
|
|
Working on testing now.
Yes, the result == 1 was typo on my part.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
/// Catch any panic function used by extern "C" functions.
fn catch_any_panic<F: FnOnce() + UnwindSafe>(function: F) -> bool {
panic::catch_unwind(function).is_ok()
}
+#[cfg(not(target_os = "windows"))]
+fn redirect_stdout_stderr() -> Result<(), String> {
+ Ok(())
+}
+
+#[cfg(target_os = "windows")]
+fn do_redirect_stdout_stderr() -> Result<(), ()> {
+ // actual implementation that returns Err(()) for problems
My point was that all of the actual implementation that is currently in redirect_stdout_stderr should go in do_redirect_stdout_stderr instead. This will allow all of the cases that used to be return false to be return Err(()) instead, and then we need one place in redirect_stdout_stderr that does the map_err thing and calls GetLastError.
_____
In ports/libsimpleservo/capi/src/lib.rs <#23922 (comment)> :
- read_buf.as_mut_ptr() as *mut _,
- read_buf.len() as u32,
- );
-
- // Write to Debug port.
- debugapi::OutputDebugStringA(read_buf.as_mut_ptr() as winnt::LPSTR);
- };
+ let mut read_buf: [i8; BUF_LENGTH] = [0; BUF_LENGTH];
+
+ let result = libc::read(
+ h_read_pipe_fd,
+ read_buf.as_mut_ptr() as *mut _,
+ read_buf.len() as u32 - 1,
+ );
+
+ if result == 1 {
read returns -1 on error, not 1.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#23922?email_source=notifications&email_token=AMJITWFXOYH7ZQXDKLPPPWTQDLD5FA5CNFSM4IJYKLMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCA23JGI#pullrequestreview-271955097> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AMJITWFTVT4EIQWBH7NXYJDQDLD5FANCNFSM4IJYKLMA> . <https://github.com/notifications/beacon/AMJITWC2UXXZ4TVDRLCA2NDQDLD5FA5CNFSM4IJYKLMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCA23JGI.gif>
|
|
Looks good! Please squash and we can merge them. |
|
Squash complete. Thanks for reviewing and all the comments. |
|
The CI problems are caused by the job exceeding the timeout because we rebuild Servo from scratch twice. |
ports/libsimpleservo/capi/Cargo.toml
Outdated
| @@ -17,6 +17,10 @@ simpleservo = { path = "../api" } | |||
| log = "0.4" | |||
| env_logger = "0.6" | |||
|
|
|||
| [target.'cfg(target_os = "windows")'.dependencies] | |||
| libc = "0.2" | |||
| winapi = {version = "0.3.7", features = ["winnt", "winbase", "processenv", "namedpipeapi", "ntdef", "minwindef", "handleapi", "debugapi"]} | |||
This comment has been minimized.
This comment has been minimized.
jdm
Aug 9, 2019
Member
Could you change thus to "0.3" instead? I think we're using a different version here than the rest of Servo and we end up rebuilding everything.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
jdm
Aug 9, 2019
Member
Ok, so it's not that. We should duplicate this list of features to
Line 77 in 8f7440f
This comment has been minimized.
This comment has been minimized.
angelortiz1007
Aug 9, 2019
Author
Contributor
Ok to leave version at "0.3.7" and add the 2 features?
This comment has been minimized.
This comment has been minimized.
jdm
Aug 9, 2019
Member
I think we need to add the two features to the libsimpleservo/capi/Cargo.toml, and add all the other features to glutin/Cargo.toml.
This comment has been minimized.
This comment has been minimized.
angelortiz1007
Aug 9, 2019
Author
Contributor
Sounds good. Added the additional features to glutin/Cargo.toml. Building to verify.
…o OutputDebugStringA().
|
Squashed/Rebased new commit with the changes to the Cargo.toml files. |
|
@bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Aug 9, 2019
Redirect stdout/stderr on windows Added function redirect_stdout_stderr() to support stdout & stderr to be redirected to OutputDebugStringA(). --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `23734` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #23734 (GitHub issue number if applicable) <!-- Either: --> - [X] There are tests for these changes OR Functionality was verified by adding logic in unsafe fn init(). By building and running ServoApp, If redirect_stdout_stderr() failed, a warn!() message would be sent to the "output" window of VS2007 with the GetLastError() value. If the function redirect_stdout_stderr() succeeded, the println!("Capi/lib.rs: init() function called redirect_stdout_stderr() successfully.\n") output would be seen in the "output" window of VS2007. - [ ] These changes do not require tests because ___ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23922) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
bors-servo
added a commit
that referenced
this pull request
Aug 10, 2019
Redirect stdout/stderr on windows Added function redirect_stdout_stderr() to support stdout & stderr to be redirected to OutputDebugStringA(). --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `23734` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #23734 (GitHub issue number if applicable) <!-- Either: --> - [X] There are tests for these changes OR Functionality was verified by adding logic in unsafe fn init(). By building and running ServoApp, If redirect_stdout_stderr() failed, a warn!() message would be sent to the "output" window of VS2007 with the GetLastError() value. If the function redirect_stdout_stderr() succeeded, the println!("Capi/lib.rs: init() function called redirect_stdout_stderr() successfully.\n") output would be seen in the "output" window of VS2007. - [ ] These changes do not require tests because ___ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23922) <!-- Reviewable:end -->
|
|
bors-servo
added a commit
that referenced
this pull request
Aug 12, 2019
Immersive WebXR on Hololens This PR adds support to the OpenXR backend (servo/webxr#37) for Hololens. We support _entering_ immersive mode, but currently do nothing in immersive mode (servo/webxr#38, applying https://github.com/servo/webxr/tree/d3d11-draw should render some red). This should be ready to land as-is aside from its dependency on #23922. It can be tested on https://manishearth.github.io/webgl-to-webxr/webxr-ar.html (make sure to click through the alerts). This is based on #23922 . This builds off of @paulrouget's work. r? @jdm @paulrouget <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23945) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this pull request
Aug 12, 2019
Immersive WebXR on Hololens This PR adds support to the OpenXR backend (servo/webxr#37) for Hololens. We support _entering_ immersive mode, but currently do nothing in immersive mode (servo/webxr#38, applying https://github.com/servo/webxr/tree/d3d11-draw should render some red). This should be ready to land as-is aside from its dependency on #23922. It can be tested on https://manishearth.github.io/webgl-to-webxr/webxr-ar.html (make sure to click through the alerts). This is based on #23922 . This builds off of @paulrouget's work. r? @jdm @paulrouget <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23945) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this pull request
Aug 12, 2019
Immersive WebXR on Hololens This PR adds support to the OpenXR backend (servo/webxr#37) for Hololens. We support _entering_ immersive mode, but currently do nothing in immersive mode (servo/webxr#38, applying https://github.com/servo/webxr/tree/d3d11-draw should render some red). This should be ready to land as-is aside from its dependency on #23922. It can be tested on https://manishearth.github.io/webgl-to-webxr/webxr-ar.html (make sure to click through the alerts). This is based on #23922 . This builds off of @paulrouget's work. r? @jdm @paulrouget <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23945) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this pull request
Aug 13, 2019
Immersive WebXR on Hololens This PR adds support to the OpenXR backend (servo/webxr#37) for Hololens. We support _entering_ immersive mode, but currently do nothing in immersive mode (servo/webxr#38, applying https://github.com/servo/webxr/tree/d3d11-draw should render some red). This should be ready to land as-is aside from its dependency on #23922. It can be tested on https://manishearth.github.io/webgl-to-webxr/webxr-ar.html (make sure to click through the alerts). This is based on #23922 . This builds off of @paulrouget's work. r? @jdm @paulrouget <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23945) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
angelortiz1007 commentedAug 6, 2019
•
edited by jdm
Added function redirect_stdout_stderr() to support stdout & stderr to be redirected to OutputDebugStringA().
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThere are tests for these changes OR
Functionality was verified by adding logic in unsafe fn init(). By building and running ServoApp, If redirect_stdout_stderr() failed, a warn!() message would be sent to the "output" window of VS2007 with the GetLastError() value. If the function redirect_stdout_stderr() succeeded, the println!("Capi/lib.rs: init() function called redirect_stdout_stderr() successfully.\n") output would be seen in the "output" window of VS2007.
These changes do not require tests because ___
This change is