don't use the Cool popup-generating eprintln in trampoline for warnings (#11295)

Also I refactored the code a bit to centralize all the calls of
eprintln.

Fixes #10706

---------

Co-authored-by: Zanie Blue <contact@zanie.dev>
This commit is contained in:
Aria Desires 2025-02-07 13:33:12 -05:00 committed by GitHub
parent 84b9ae2b92
commit 161eb42cb9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 42 additions and 39 deletions

View file

@ -32,7 +32,7 @@ use windows::Win32::{
},
};
use crate::{eprintln, format};
use crate::{error, format, warn};
const PATH_LEN_SIZE: usize = size_of::<u32>();
const MAX_PATH_LEN: u32 = 32 * 1024;
@ -68,8 +68,7 @@ impl TrampolineKind {
/// depending on the [`TrampolineKind`].
fn make_child_cmdline() -> CString {
let executable_name = std::env::current_exe().unwrap_or_else(|_| {
eprintln!("Failed to get executable name");
exit_with_status(1);
error_and_exit("Failed to get executable name");
});
let (kind, python_exe) = read_trampoline_metadata(executable_name.as_ref());
let mut child_cmdline = Vec::<u8>::new();
@ -94,15 +93,14 @@ fn make_child_cmdline() -> CString {
child_cmdline.push(b'\0');
// Helpful when debugging trampoline issues
// eprintln!(
// warn!(
// "executable_name: '{}'\nnew_cmdline: {}",
// &*executable_name.to_string_lossy(),
// std::str::from_utf8(child_cmdline.as_slice()).unwrap()
// );
CString::from_vec_with_nul(child_cmdline).unwrap_or_else(|_| {
eprintln!("Child command line is not correctly null terminated");
exit_with_status(1);
error_and_exit("Child command line is not correctly null terminated");
})
}
@ -175,8 +173,7 @@ fn read_trampoline_metadata(executable_name: &Path) -> (TrampolineKind, PathBuf)
buffer.truncate(read_bytes);
let Some(inner_kind) = TrampolineKind::from_buffer(&buffer) else {
eprintln!("Magic number 'UVSC' or 'UVPY' not found at the end of the file. Did you append the magic number, the length and the path to the python executable at the end of the file?");
exit_with_status(1);
error_and_exit("Magic number 'UVSC' or 'UVPY' not found at the end of the file. Did you append the magic number, the length and the path to the python executable at the end of the file?");
};
kind = inner_kind;
@ -186,21 +183,18 @@ fn read_trampoline_metadata(executable_name: &Path) -> (TrampolineKind, PathBuf)
let path_len = match buffer.get(buffer.len() - PATH_LEN_SIZE..) {
Some(path_len) => {
let path_len = u32::from_le_bytes(path_len.try_into().unwrap_or_else(|_| {
eprintln!("Slice length is not equal to 4 bytes");
exit_with_status(1);
error_and_exit("Slice length is not equal to 4 bytes");
}));
if path_len > MAX_PATH_LEN {
eprintln!("Only paths with a length up to 32KBs are supported but the python path has a length of {}", path_len);
exit_with_status(1);
error_and_exit(&format!("Only paths with a length up to 32KBs are supported but the python path has a length of {}", path_len));
}
// SAFETY: path len is guaranteed to be less than 32KBs
path_len as usize
}
None => {
eprintln!("Python executable length missing. Did you write the length of the path to the Python executable before the Magic number?");
exit_with_status(1);
error_and_exit("Python executable length missing. Did you write the length of the path to the Python executable before the Magic number?");
}
};
@ -211,8 +205,7 @@ fn read_trampoline_metadata(executable_name: &Path) -> (TrampolineKind, PathBuf)
buffer.drain(..path_offset);
break String::from_utf8(buffer).unwrap_or_else(|_| {
eprintln!("Python executable path is not a valid UTF-8 encoded path");
exit_with_status(1);
error_and_exit("Python executable path is not a valid UTF-8 encoded path");
});
} else {
// SAFETY: Casting to u32 is safe because `path_len` is guaranteed to be less than 32KBs,
@ -220,8 +213,7 @@ fn read_trampoline_metadata(executable_name: &Path) -> (TrampolineKind, PathBuf)
bytes_to_read = (path_len + kind.magic_number().len() + PATH_LEN_SIZE) as u32;
if u64::from(bytes_to_read) > file_size {
eprintln!("The length of the python executable path exceeds the file size. Verify that the path length is appended to the end of the launcher script as a u32 in little endian");
exit_with_status(1);
error_and_exit("The length of the python executable path exceeds the file size. Verify that the path length is appended to the end of the launcher script as a u32 in little endian");
}
}
};
@ -233,8 +225,7 @@ fn read_trampoline_metadata(executable_name: &Path) -> (TrampolineKind, PathBuf)
let parent_dir = match executable_name.parent() {
Some(parent) => parent,
None => {
eprintln!("Executable path has no parent directory");
exit_with_status(1);
error_and_exit("Executable path has no parent directory");
}
};
parent_dir.join(path)
@ -242,8 +233,7 @@ fn read_trampoline_metadata(executable_name: &Path) -> (TrampolineKind, PathBuf)
// NOTICE: dunce adds 5kb~
let path = dunce::canonicalize(path.as_path()).unwrap_or_else(|_| {
eprintln!("Failed to canonicalize script path");
exit_with_status(1);
error_and_exit("Failed to canonicalize script path");
});
(kind, path)
@ -331,11 +321,11 @@ fn spawn_child(si: &STARTUPINFOA, child_cmdline: CString) -> HANDLE {
if (si.dwFlags & STARTF_USESTDHANDLES).0 != 0 {
// ignore errors, if the handles are not inheritable/valid, then nothing we can do
unsafe { SetHandleInformation(si.hStdInput, HANDLE_FLAG_INHERIT.0, HANDLE_FLAG_INHERIT) }
.unwrap_or_else(|_| eprintln!("Making stdin inheritable failed"));
.unwrap_or_else(|_| warn!("Making stdin inheritable failed"));
unsafe { SetHandleInformation(si.hStdOutput, HANDLE_FLAG_INHERIT.0, HANDLE_FLAG_INHERIT) }
.unwrap_or_else(|_| eprintln!("Making stdout inheritable failed"));
.unwrap_or_else(|_| warn!("Making stdout inheritable failed"));
unsafe { SetHandleInformation(si.hStdError, HANDLE_FLAG_INHERIT.0, HANDLE_FLAG_INHERIT) }
.unwrap_or_else(|_| eprintln!("Making stderr inheritable failed"));
.unwrap_or_else(|_| warn!("Making stderr inheritable failed"));
}
let mut child_process_info = PROCESS_INFORMATION::default();
unsafe {
@ -370,14 +360,14 @@ fn spawn_child(si: &STARTUPINFOA, child_cmdline: CString) -> HANDLE {
// https://github.com/huangqinjin/ucrt/blob/10.0.19041.0/lowio/ioinit.cpp#L190-L223
fn close_handles(si: &STARTUPINFOA) {
// See distlib/PC/launcher.c::cleanup_standard_io()
// Unlike cleanup_standard_io(), we don't close STD_ERROR_HANDLE to retain eprintln!
// Unlike cleanup_standard_io(), we don't close STD_ERROR_HANDLE to retain warn!
for std_handle in [STD_INPUT_HANDLE, STD_OUTPUT_HANDLE] {
if let Ok(handle) = unsafe { GetStdHandle(std_handle) } {
unsafe { CloseHandle(handle) }.unwrap_or_else(|_| {
eprintln!("Failed to close standard device handle {}", handle.0 as u32);
warn!("Failed to close standard device handle {}", handle.0 as u32);
});
unsafe { SetStdHandle(std_handle, INVALID_HANDLE_VALUE) }.unwrap_or_else(|_| {
eprintln!("Failed to modify standard device handle {}", std_handle.0);
warn!("Failed to modify standard device handle {}", std_handle.0);
});
}
}
@ -401,7 +391,7 @@ fn close_handles(si: &STARTUPINFOA) {
continue;
}
unsafe { CloseHandle(handle) }.unwrap_or_else(|_| {
eprintln!("Failed to close child file descriptors at {}", i);
warn!("Failed to close child file descriptors at {}", i);
});
}
}
@ -430,14 +420,14 @@ fn clear_app_starting_state(child_handle: HANDLE) {
unsafe {
// End the launcher's "app starting" cursor state.
PostMessageA(None, 0, WPARAM(0), LPARAM(0)).unwrap_or_else(|_| {
eprintln!("Failed to post a message to specified window");
warn!("Failed to post a message to specified window");
});
if GetMessageA(&mut msg, None, 0, 0) != TRUE {
eprintln!("Failed to retrieve posted window message");
warn!("Failed to retrieve posted window message");
}
// Proxy the child's input idle event.
if WaitForInputIdle(child_handle, INFINITE) != 0 {
eprintln!("Failed to wait for input from window");
warn!("Failed to wait for input from window");
}
// Signal the process input idle event by creating a window and pumping
// sent messages. The window class isn't important, so just use the
@ -484,7 +474,7 @@ pub fn bounce(is_gui: bool) -> ! {
// (best effort) Switch to some innocuous directory, so we don't hold the original cwd open.
// See distlib/PC/launcher.c::switch_working_directory
if std::env::set_current_dir(std::env::temp_dir()).is_err() {
eprintln!("Failed to set cwd to temp dir");
warn!("Failed to set cwd to temp dir");
}
// We want to ignore control-C/control-Break/logout/etc.; the same event will
@ -509,6 +499,12 @@ pub fn bounce(is_gui: bool) -> ! {
exit_with_status(exit_code);
}
#[cold]
fn error_and_exit(message: &str) -> ! {
error!("{}", message);
exit_with_status(1);
}
#[cold]
fn print_last_error_and_exit(message: &str) -> ! {
let err = std::io::Error::last_os_error();
@ -518,13 +514,13 @@ fn print_last_error_and_exit(message: &str) -> ! {
.unwrap_or_default();
// we can't access sys::os::error_string directly so err.kind().to_string()
// is the closest we can get to while avoiding bringing in a large chunk of core::fmt
eprintln!(
let message = format!(
"(uv internal error) {}: {}.{}",
message,
err.kind().to_string(),
err_no_str
);
exit_with_status(1);
error_and_exit(&message);
}
#[cold]

View file

@ -9,9 +9,16 @@ use windows::core::PCSTR;
use windows::Win32::UI::WindowsAndMessaging::{MessageBoxA, MESSAGEBOX_STYLE};
#[macro_export]
macro_rules! eprintln {
macro_rules! error {
($($tt:tt)*) => {{
$crate::diagnostics::write_diagnostic(&$crate::format!($($tt)*));
$crate::diagnostics::write_diagnostic(&$crate::format!($($tt)*), true);
}}
}
#[macro_export]
macro_rules! warn {
($($tt:tt)*) => {{
$crate::diagnostics::write_diagnostic(&$crate::format!($($tt)*), false);
}}
}
@ -37,11 +44,11 @@ impl uWrite for StringBuffer {
}
#[cold]
pub(crate) fn write_diagnostic(message: &str) {
pub(crate) fn write_diagnostic(message: &str, is_error: bool) {
let mut stderr = std::io::stderr();
if !stderr.as_raw_handle().is_null() {
let _ = stderr.write_all(message.as_bytes());
} else {
} else if is_error {
let nul_terminated = unsafe { CString::new(message.as_bytes()).unwrap_unchecked() };
let pcstr_message = PCSTR::from_raw(nul_terminated.as_ptr() as *const _);
unsafe { MessageBoxA(None, pcstr_message, None, MESSAGEBOX_STYLE(0)) };