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

run_make_support: rectify symlink handling #130427

Merged
merged 2 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 130 additions & 50 deletions src/tools/run-make-support/src/fs.rs
Original file line number Diff line number Diff line change
@@ -1,42 +1,6 @@
use std::io;
use std::path::{Path, PathBuf};

// FIXME(jieyouxu): modify create_symlink to panic on windows.

/// Creates a new symlink to a path on the filesystem, adjusting for Windows or Unix.
#[cfg(target_family = "windows")]
pub fn create_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) {
if link.as_ref().exists() {
std::fs::remove_dir(link.as_ref()).unwrap();
}
if original.as_ref().is_file() {
std::os::windows::fs::symlink_file(original.as_ref(), link.as_ref()).expect(&format!(
"failed to create symlink {:?} for {:?}",
link.as_ref().display(),
original.as_ref().display(),
));
} else {
std::os::windows::fs::symlink_dir(original.as_ref(), link.as_ref()).expect(&format!(
"failed to create symlink {:?} for {:?}",
link.as_ref().display(),
original.as_ref().display(),
));
}
}

/// Creates a new symlink to a path on the filesystem, adjusting for Windows or Unix.
#[cfg(target_family = "unix")]
pub fn create_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) {
if link.as_ref().exists() {
std::fs::remove_dir(link.as_ref()).unwrap();
}
std::os::unix::fs::symlink(original.as_ref(), link.as_ref()).expect(&format!(
"failed to create symlink {:?} for {:?}",
link.as_ref().display(),
original.as_ref().display(),
));
}

/// Copy a directory into another.
pub fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) {
fn copy_dir_all_inner(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<()> {
Expand All @@ -50,7 +14,31 @@ pub fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) {
if ty.is_dir() {
copy_dir_all_inner(entry.path(), dst.join(entry.file_name()))?;
} else if ty.is_symlink() {
copy_symlink(entry.path(), dst.join(entry.file_name()))?;
// Traverse symlink once to find path of target entity.
let target_path = std::fs::read_link(entry.path())?;

let new_symlink_path = dst.join(entry.file_name());
ChrisDenton marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(windows)]
{
use std::os::windows::fs::FileTypeExt;
if ty.is_symlink_dir() {
std::os::windows::fs::symlink_dir(&target_path, new_symlink_path)?;
} else {
// Target may be a file or another symlink, in any case we can use
// `symlink_file` here.
std::os::windows::fs::symlink_file(&target_path, new_symlink_path)?;
}
}
#[cfg(unix)]
{
std::os::unix::fs::symlink(target_path, new_symlink_path)?;
}
#[cfg(not(any(windows, unix)))]
{
// Technically there's also wasi, but I have no clue about wasi symlink
// semantics and which wasi targets / environment support symlinks.
unimplemented!("unsupported target");
}
} else {
std::fs::copy(entry.path(), dst.join(entry.file_name()))?;
}
Expand All @@ -69,12 +57,6 @@ pub fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) {
}
}

fn copy_symlink<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> io::Result<()> {
let target_path = std::fs::read_link(from).unwrap();
create_symlink(target_path, to);
Ok(())
}

/// Helper for reading entries in a given directory.
pub fn read_dir_entries<P: AsRef<Path>, F: FnMut(&Path)>(dir: P, mut callback: F) {
for entry in read_dir(dir) {
Expand All @@ -85,8 +67,17 @@ pub fn read_dir_entries<P: AsRef<Path>, F: FnMut(&Path)>(dir: P, mut callback: F
/// A wrapper around [`std::fs::remove_file`] which includes the file path in the panic message.
#[track_caller]
pub fn remove_file<P: AsRef<Path>>(path: P) {
std::fs::remove_file(path.as_ref())
.expect(&format!("the file in path \"{}\" could not be removed", path.as_ref().display()));
if let Err(e) = std::fs::remove_file(path.as_ref()) {
panic!("failed to remove file at `{}`: {e}", path.as_ref().display());
}
}

/// A wrapper around [`std::fs::remove_dir`] which includes the directory path in the panic message.
#[track_caller]
pub fn remove_dir<P: AsRef<Path>>(path: P) {
if let Err(e) = std::fs::remove_dir(path.as_ref()) {
panic!("failed to remove directory at `{}`: {e}", path.as_ref().display());
}
}

/// A wrapper around [`std::fs::copy`] which includes the file path in the panic message.
Expand Down Expand Up @@ -165,13 +156,32 @@ pub fn create_dir_all<P: AsRef<Path>>(path: P) {
));
}

/// A wrapper around [`std::fs::metadata`] which includes the file path in the panic message.
/// A wrapper around [`std::fs::metadata`] which includes the file path in the panic message. Note
/// that this will traverse symlinks and will return metadata about the target file. Use
/// [`symlink_metadata`] if you don't want to traverse symlinks.
///
/// See [`std::fs::metadata`] docs for more details.
#[track_caller]
pub fn metadata<P: AsRef<Path>>(path: P) -> std::fs::Metadata {
std::fs::metadata(path.as_ref()).expect(&format!(
"the file's metadata in path \"{}\" could not be read",
path.as_ref().display()
))
match std::fs::metadata(path.as_ref()) {
Ok(m) => m,
Err(e) => panic!("failed to read file metadata at `{}`: {e}", path.as_ref().display()),
}
}

/// A wrapper around [`std::fs::symlink_metadata`] which includes the file path in the panic
/// message. Note that this will not traverse symlinks and will return metadata about the filesystem
/// entity itself. Use [`metadata`] if you want to traverse symlinks.
///
/// See [`std::fs::symlink_metadata`] docs for more details.
#[track_caller]
pub fn symlink_metadata<P: AsRef<Path>>(path: P) -> std::fs::Metadata {
match std::fs::symlink_metadata(path.as_ref()) {
Ok(m) => m,
Err(e) => {
panic!("failed to read file metadata (shallow) at `{}`: {e}", path.as_ref().display())
}
}
}

/// A wrapper around [`std::fs::rename`] which includes the file path in the panic message.
Expand Down Expand Up @@ -205,3 +215,73 @@ pub fn shallow_find_dir_entries<P: AsRef<Path>>(dir: P) -> Vec<PathBuf> {
}
output
}

/// Create a new symbolic link to a directory.
///
/// # Removing the symlink
///
/// - On Windows, a symlink-to-directory needs to be removed with a corresponding [`fs::remove_dir`]
/// and not [`fs::remove_file`].
/// - On Unix, remove the symlink with [`fs::remove_file`].
///
/// [`fs::remove_dir`]: crate::fs::remove_dir
/// [`fs::remove_file`]: crate::fs::remove_file
pub fn symlink_dir<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) {
#[cfg(unix)]
{
if let Err(e) = std::os::unix::fs::symlink(original.as_ref(), link.as_ref()) {
panic!(
"failed to create symlink: original=`{}`, link=`{}`: {e}",
original.as_ref().display(),
link.as_ref().display()
);
}
}
#[cfg(windows)]
{
if let Err(e) = std::os::windows::fs::symlink_dir(original.as_ref(), link.as_ref()) {
panic!(
"failed to create symlink-to-directory: original=`{}`, link=`{}`: {e}",
original.as_ref().display(),
link.as_ref().display()
);
}
}
#[cfg(not(any(windows, unix)))]
{
unimplemented!("target family not currently supported")
}
}

/// Create a new symbolic link to a file.
///
/// # Removing the symlink
///
/// On both Windows and Unix, a symlink-to-file needs to be removed with a corresponding
/// [`fs::remove_file`](crate::fs::remove_file) and not [`fs::remove_dir`](crate::fs::remove_dir).
pub fn symlink_file<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) {
#[cfg(unix)]
{
if let Err(e) = std::os::unix::fs::symlink(original.as_ref(), link.as_ref()) {
panic!(
"failed to create symlink: original=`{}`, link=`{}`: {e}",
original.as_ref().display(),
link.as_ref().display()
);
}
}
#[cfg(windows)]
{
if let Err(e) = std::os::windows::fs::symlink_file(original.as_ref(), link.as_ref()) {
panic!(
"failed to create symlink-to-file: original=`{}`, link=`{}`: {e}",
original.as_ref().display(),
link.as_ref().display()
);
}
}
#[cfg(not(any(windows, unix)))]
{
unimplemented!("target family not currently supported")
}
}
17 changes: 9 additions & 8 deletions tests/run-make/invalid-symlink-search-path/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
// In this test, the symlink created is invalid (valid relative to the root, but not
// relatively to where it is located), and used to cause an internal
// compiler error (ICE) when passed as a library search path. This was fixed in #26044,
// and this test checks that the invalid symlink is instead simply ignored.
// In this test, the symlink created is invalid (valid relative to the root, but not relatively to
// where it is located), and used to cause an internal compiler error (ICE) when passed as a library
// search path. This was fixed in #26044, and this test checks that the invalid symlink is instead
// simply ignored.
//
// See https://github.com/rust-lang/rust/issues/26006

//@ needs-symlink
//Reason: symlink requires elevated permission in Windows

use run_make_support::{rfs, rustc};
use run_make_support::{path, rfs, rustc};

fn main() {
// We create two libs: `bar` which depends on `foo`. We need to compile `foo` first.
Expand All @@ -20,9 +21,9 @@ fn main() {
.metadata("foo")
.output("out/foo/libfoo.rlib")
.run();
rfs::create_dir("out/bar");
rfs::create_dir("out/bar/deps");
rfs::create_symlink("out/foo/libfoo.rlib", "out/bar/deps/libfoo.rlib");
rfs::create_dir_all("out/bar/deps");
rfs::symlink_file(path("out/foo/libfoo.rlib"), path("out/bar/deps/libfoo.rlib"));

// Check that the invalid symlink does not cause an ICE
rustc()
.input("in/bar/lib.rs")
Expand Down
24 changes: 12 additions & 12 deletions tests/run-make/symlinked-extern/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
// Crates that are resolved normally have their path canonicalized and all
// symlinks resolved. This did not happen for paths specified
// using the --extern option to rustc, which could lead to rustc thinking
// that it encountered two different versions of a crate, when it's
// actually the same version found through different paths.
// See https://github.com/rust-lang/rust/pull/16505

// This test checks that --extern and symlinks together
// can result in successful compilation.
// Crates that are resolved normally have their path canonicalized and all symlinks resolved. This
// did not happen for paths specified using the `--extern` option to rustc, which could lead to
// rustc thinking that it encountered two different versions of a crate, when it's actually the same
// version found through different paths.
//
// This test checks that `--extern` and symlinks together can result in successful compilation.
//
// See <https://github.com/rust-lang/rust/pull/16505>.

//@ ignore-cross-compile
//@ needs-symlink

use run_make_support::{cwd, rfs, rustc};
use run_make_support::{cwd, path, rfs, rustc};

fn main() {
rustc().input("foo.rs").run();
rfs::create_dir_all("other");
rfs::create_symlink("libfoo.rlib", "other");
rfs::symlink_file(path("libfoo.rlib"), path("other").join("libfoo.rlib"));

rustc().input("bar.rs").library_search_path(cwd()).run();
rustc().input("baz.rs").extern_("foo", "other").library_search_path(cwd()).run();
rustc().input("baz.rs").extern_("foo", "other/libfoo.rlib").library_search_path(cwd()).run();
}
15 changes: 6 additions & 9 deletions tests/run-make/symlinked-libraries/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
// When a directory and a symlink simultaneously exist with the same name,
// setting that name as the library search path should not cause rustc
// to avoid looking in the symlink and cause an error. This test creates
// a directory and a symlink named "other", and places the library in the symlink.
// If it succeeds, the library was successfully found.
// See https://github.com/rust-lang/rust/issues/12459
// Avoid erroring on symlinks pointing to the same file that are present in the library search path.
//
// See <https://github.com/rust-lang/rust/issues/12459>.

//@ ignore-cross-compile
//@ needs-symlink

use run_make_support::{dynamic_lib_name, rfs, rustc};
use run_make_support::{cwd, dynamic_lib_name, path, rfs, rustc};

fn main() {
rustc().input("foo.rs").arg("-Cprefer-dynamic").run();
rfs::create_dir_all("other");
rfs::create_symlink(dynamic_lib_name("foo"), "other");
rustc().input("bar.rs").library_search_path("other").run();
rfs::symlink_file(dynamic_lib_name("foo"), path("other").join(dynamic_lib_name("foo")));
rustc().input("bar.rs").library_search_path(cwd()).library_search_path("other").run();
}
2 changes: 1 addition & 1 deletion tests/run-make/symlinked-rlib/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ use run_make_support::{cwd, rfs, rustc};

fn main() {
rustc().input("foo.rs").crate_type("rlib").output("foo.xxx").run();
rfs::create_symlink("foo.xxx", "libfoo.rlib");
rfs::symlink_file("foo.xxx", "libfoo.rlib");
rustc().input("bar.rs").library_search_path(cwd()).run();
}
Loading