-
Notifications
You must be signed in to change notification settings - Fork 13.3k
rustbuild fixes #36986
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
rustbuild fixes #36986
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,29 @@ pub fn staticlib(name: &str, target: &str) -> String { | |
} | ||
} | ||
|
||
#[cfg(not(windows))] | ||
pub fn build_path(path: &str) -> PathBuf { | ||
PathBuf::from(path) | ||
} | ||
|
||
#[cfg(windows)] | ||
pub fn build_path(path: &str) -> PathBuf { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is somewhat innocuously named, but perhaps it could be contained to just Perhaps this could be called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could use a nicer name |
||
use std::io::{stderr, Write}; | ||
use build_helper::output; | ||
|
||
if path.chars().next() == Some('/') { | ||
let output = output(&mut Command::new("cygpath").arg("-w").arg(path)); | ||
let win_path = output.trim_right(); | ||
writeln!(&mut stderr(), | ||
"note: Converted Unix path '{}' to Windows path '{}'", | ||
path, | ||
win_path).ok(); | ||
PathBuf::from(win_path) | ||
} else { | ||
PathBuf::from(path) | ||
} | ||
} | ||
|
||
/// Returns the last-modified time for `path`, or zero if it doesn't exist. | ||
pub fn mtime(path: &Path) -> FileTime { | ||
fs::metadata(path).map(|f| { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this check
LD_LIBRARY_PATH
on Unix as well?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that should be required (even if it currently is)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually we could perhaps update this so you didn't have to set that env var, but for now if we're already checking OSX we may as well check Linux, and it's definitely required today at least.