-
Notifications
You must be signed in to change notification settings - Fork 426
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
Implement directory based runfiles lookup [#130] #132
Changes from all commits
aaa50df
04b699e
22794b0
614e912
68924fe
bffc8c5
e2b99c1
6873ab8
c3647eb
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 |
---|---|---|
@@ -1,2 +1,8 @@ | ||
# vim | ||
*.swp | ||
|
||
# bazel | ||
/bazel-* | ||
|
||
# rustfmt | ||
*.rs.bk |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
extern crate runfiles; | ||
|
||
use std::io::prelude::*; | ||
use std::fs::File; | ||
|
||
use runfiles::Runfiles; | ||
|
||
fn main() { | ||
let r = Runfiles::create().unwrap(); | ||
|
||
let mut f = File::open(r.rlocation("examples/hello_runfiles/hello_runfiles.rs")).unwrap(); | ||
|
||
let mut buffer = String::new(); | ||
f.read_to_string(&mut buffer).unwrap(); | ||
|
||
assert_eq!(buffer.len(), 427); | ||
println!("This program's source is:\n```\n{}\n```", buffer); | ||
} |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
load( | ||
"@io_bazel_rules_rust//rust:rust.bzl", | ||
"rust_doc_test", | ||
"rust_library", | ||
"rust_test", | ||
) | ||
|
||
rust_library( | ||
name = "runfiles", | ||
srcs = ["runfiles.rs"], | ||
visibility = ["//visibility:public"], | ||
) | ||
|
||
rust_test( | ||
name = "runfiles_test", | ||
data = ["data/sample.txt"], | ||
deps = [":runfiles"], | ||
) | ||
|
||
rust_doc_test( | ||
name = "runfiles_doc_test", | ||
dep = ":runfiles", | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
//! Runfiles lookup library for Bazel-built Rust binaries and tests. | ||
//! | ||
//! USAGE: | ||
//! | ||
//! 1. Depend on this runfiles library from your build rule: | ||
//! ```python | ||
//! rust_binary( | ||
//! name = "my_binary", | ||
//! ... | ||
//! data = ["//path/to/my/data.txt"], | ||
//! deps = ["@io_bazel_rules_rust//tools/runfiles"], | ||
//! ) | ||
//! ``` | ||
//! | ||
//! 2. Import the runfiles library. | ||
//! ``` | ||
//! extern crate runfiles; | ||
//! | ||
//! use runfiles::Runfiles; | ||
//! ``` | ||
//! | ||
//! 3. Create a Runfiles object and use rlocation to look up runfile paths: | ||
//! ```ignore -- This doesn't work under rust_doc_test because argv[0] is not what we expect. | ||
//! | ||
//! use runfiles::Runfiles; | ||
//! | ||
//! let r = Runfiles::create().unwrap(); | ||
//! let path = r.rlocation("my_workspace/path/to/my/data.txt"); | ||
//! | ||
//! let f = File::open(path).unwrap(); | ||
//! // ... | ||
//! ``` | ||
|
||
use std::io; | ||
use std::fs; | ||
use std::path::PathBuf; | ||
use std::path::Path; | ||
use std::env; | ||
|
||
pub struct Runfiles { | ||
runfiles_dir: PathBuf, | ||
} | ||
|
||
impl Runfiles { | ||
/// Creates a directory based Runfiles object. | ||
/// | ||
/// Manifest based creation is not currently supported. | ||
pub fn create() -> io::Result<Self> { | ||
Ok(Runfiles { runfiles_dir: find_runfiles_dir()? }) | ||
} | ||
|
||
/// Returns the runtime path of a runfile. | ||
/// | ||
/// Runfiles are data-dependencies of Bazel-built binaries and tests. | ||
/// The returned path may not be valid. The caller should check the path's | ||
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. Under what conditions would the path not be valid? I'm unclear on whether this just refers to non-existent paths, or if some bazel-related circumstances can arise to cause a file not to appear. 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. I think this means that you can call rlocation on some path, and it will return |
||
/// validity and that the path exists. | ||
pub fn rlocation(&self, path: impl AsRef<Path>) -> PathBuf { | ||
let path = path.as_ref(); | ||
if path.is_absolute() { | ||
return path.to_path_buf(); | ||
} | ||
self.runfiles_dir.join(path) | ||
} | ||
} | ||
|
||
/// Returns the .runfiles directory for the currently executing binary. | ||
fn find_runfiles_dir() -> io::Result<PathBuf> { | ||
let exec_path = std::env::args().nth(0).expect("arg 0 was not set"); | ||
|
||
let mut binary_path = PathBuf::from(&exec_path); | ||
loop { | ||
// Check for our neighboring $binary.runfiles directory. | ||
let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); | ||
runfiles_name.push(".runfiles"); | ||
|
||
let runfiles_path = binary_path.with_file_name(&runfiles_name); | ||
if runfiles_path.is_dir() { | ||
return Ok(runfiles_path); | ||
} | ||
|
||
// Check if we're already under a *.runfiles directory. | ||
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. Why would this happen? 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. a. If the executing binary is part of another's runfiles tree, eg. a py_binary calling out to our rust_binary. I have also seen many binaries deployed with a merged .runfiles tree. |
||
{ | ||
// TODO: 1.28 adds Path::ancestors() which is a little simpler. | ||
let mut next = binary_path.parent(); | ||
while let Some(ancestor) = next { | ||
if ancestor.file_name().map_or(false, |f| { | ||
f.to_string_lossy().ends_with(".runfiles") | ||
}) | ||
{ | ||
return Ok(ancestor.to_path_buf()); | ||
} | ||
next = ancestor.parent(); | ||
} | ||
} | ||
|
||
if !fs::symlink_metadata(&binary_path)?.file_type().is_symlink() { | ||
break; | ||
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. Nit: The control flow here strikes me as unusual. This seems to be breaking out of the Perhaps the comment here should be revisited? 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. Wrote the comment before flipping the if statement, will move it down. |
||
} | ||
// Follow symlinks and keep looking. | ||
binary_path = binary_path.read_link()?; | ||
if binary_path.is_relative() { | ||
binary_path = env::current_dir()?.join(binary_path) | ||
} | ||
} | ||
|
||
panic!("Failed to find .runfiles directory."); | ||
} | ||
|
||
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
|
||
use std::io::prelude::*; | ||
use std::fs::File; | ||
|
||
#[test] | ||
fn test_can_read_data_from_runfiles() { | ||
let r = Runfiles::create().unwrap(); | ||
|
||
let mut f = File::open(r.rlocation( | ||
"io_bazel_rules_rust/tools/runfiles/data/sample.txt", | ||
)).unwrap(); | ||
|
||
let mut buffer = String::new(); | ||
f.read_to_string(&mut buffer).unwrap(); | ||
|
||
assert_eq!("Example Text!", buffer); | ||
} | ||
} |
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 dont feel strongly about this, but we probably should conform to Rust idioms:
Unsure what Rust zeitgeist has to say about
new
methods not returning Self directly (though it's obviously justified here).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.
runfiles.py struck me as unidiomatic as well, so I leaned towards looking the same as other runfiles libraries. I also don't feel strongly about this.
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.
OK, matching python (in defying local norms) SGTM