Skip to content

Commit

Permalink
Auto merge of #79060 - dtolnay:symlinkarg, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Disambiguate symlink argument names

The current argument naming in the following standard library functions is horribly ambiguous.

- std::os::unix::fs::symlink: https://doc.rust-lang.org/1.47.0/std/os/unix/fs/fn.symlink.html
- std::os::windows::fs::symlink_file: https://doc.rust-lang.org/1.47.0/std/os/windows/fs/fn.symlink_file.html
- std::os::windows::fs::symlink_dir: https://doc.rust-lang.org/1.47.0/std/os/windows/fs/fn.symlink_dir.html

**Notice that Swift uses one of the same names we do (`dst`) to refer to the opposite thing.**

<br>

| | the&nbsp;one&nbsp;that&nbsp;exists | the&nbsp;one&nbsp;that&nbsp;is<br>being&nbsp;created | reference |
| --- | --- | --- | --- |
| Rust | `src` | `dst` | |
| Swift | `withDestinationPath`<br>`destPath` | `atPath`<br>`path` | <sub>https://developer.apple.com/documentation/foundation/filemanager/1411007-createsymboliclink</sub> |
| D | `original` | `link` | <sub>https://dlang.org/library/std/file/symlink.html</sub> |
| Go | `oldname` | `newname` | <sub>https://golang.org/pkg/os/#Symlink</sub> |
| C++| `target` | `link` | <sub>https://en.cppreference.com/w/cpp/filesystem/create_symlink</sub> |
| POSIX | `path1` | `path2` | <sub>https://pubs.opengroup.org/onlinepubs/9699919799/functions/symlink.html</sub> |
| Linux | `target` | `linkpath` | <sub>https://man7.org/linux/man-pages/man2/symlink.2.html</sub> |

Out of these I happen to like D's argument names and am proposing that we adopt them.
  • Loading branch information
bors committed Nov 19, 2020
2 parents fe98231 + 29128a5 commit 09c9c9f
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 60 deletions.
24 changes: 13 additions & 11 deletions library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1698,12 +1698,14 @@ pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> io::Result<u64> {

/// Creates a new hard link on the filesystem.
///
/// The `dst` path will be a link pointing to the `src` path. Note that systems
/// often require these two paths to both be located on the same filesystem.
/// The `link` path will be a link pointing to the `original` path. Note that
/// systems often require these two paths to both be located on the same
/// filesystem.
///
/// If `src` names a symbolic link, it is platform-specific whether the symbolic
/// link is followed. On platforms where it's possible to not follow it, it is
/// not followed, and the created hard link points to the symbolic link itself.
/// If `original` names a symbolic link, it is platform-specific whether the
/// symbolic link is followed. On platforms where it's possible to not follow
/// it, it is not followed, and the created hard link points to the symbolic
/// link itself.
///
/// # Platform-specific behavior
///
Expand All @@ -1718,7 +1720,7 @@ pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> io::Result<u64> {
/// This function will return an error in the following situations, but is not
/// limited to just these cases:
///
/// * The `src` path is not a file or doesn't exist.
/// * The `original` path is not a file or doesn't exist.
///
/// # Examples
///
Expand All @@ -1731,13 +1733,13 @@ pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> io::Result<u64> {
/// }
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn hard_link<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::Result<()> {
fs_imp::link(src.as_ref(), dst.as_ref())
pub fn hard_link<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
fs_imp::link(original.as_ref(), link.as_ref())
}

/// Creates a new symbolic link on the filesystem.
///
/// The `dst` path will be a symbolic link pointing to the `src` path.
/// The `link` path will be a symbolic link pointing to the `original` path.
/// On Windows, this will be a file symlink, not a directory symlink;
/// for this reason, the platform-specific [`std::os::unix::fs::symlink`]
/// and [`std::os::windows::fs::symlink_file`] or [`symlink_dir`] should be
Expand All @@ -1763,8 +1765,8 @@ pub fn hard_link<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::Result<(
reason = "replaced with std::os::unix::fs::symlink and \
std::os::windows::fs::{symlink_file, symlink_dir}"
)]
pub fn soft_link<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::Result<()> {
fs_imp::symlink(src.as_ref(), dst.as_ref())
pub fn soft_link<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
fs_imp::symlink(original.as_ref(), link.as_ref())
}

/// Reads a symbolic link, returning the file that the link points to.
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/cloudabi/shims/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ pub fn readlink(_p: &Path) -> io::Result<PathBuf> {
unsupported()
}

pub fn symlink(_src: &Path, _dst: &Path) -> io::Result<()> {
pub fn symlink(_original: &Path, _link: &Path) -> io::Result<()> {
unsupported()
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/hermit/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,11 @@ pub fn readlink(_p: &Path) -> io::Result<PathBuf> {
unsupported()
}

pub fn symlink(_src: &Path, _dst: &Path) -> io::Result<()> {
pub fn symlink(_original: &Path, _link: &Path) -> io::Result<()> {
unsupported()
}

pub fn link(_src: &Path, _dst: &Path) -> io::Result<()> {
pub fn link(_original: &Path, _link: &Path) -> io::Result<()> {
unsupported()
}

Expand Down
6 changes: 3 additions & 3 deletions library/std/src/sys/unix/ext/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ impl DirEntryExt for fs::DirEntry {

/// Creates a new symbolic link on the filesystem.
///
/// The `dst` path will be a symbolic link pointing to the `src` path.
/// The `link` path will be a symbolic link pointing to the `original` path.
///
/// # Examples
///
Expand All @@ -854,8 +854,8 @@ impl DirEntryExt for fs::DirEntry {
/// }
/// ```
#[stable(feature = "symlink", since = "1.1.0")]
pub fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::Result<()> {
sys::fs::symlink(src.as_ref(), dst.as_ref())
pub fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
sys::fs::symlink(original.as_ref(), link.as_ref())
}

/// Unix-specific extensions to [`fs::DirBuilder`].
Expand Down
18 changes: 9 additions & 9 deletions library/std/src/sys/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1071,28 +1071,28 @@ pub fn readlink(p: &Path) -> io::Result<PathBuf> {
}
}

pub fn symlink(src: &Path, dst: &Path) -> io::Result<()> {
let src = cstr(src)?;
let dst = cstr(dst)?;
cvt(unsafe { libc::symlink(src.as_ptr(), dst.as_ptr()) })?;
pub fn symlink(original: &Path, link: &Path) -> io::Result<()> {
let original = cstr(original)?;
let link = cstr(link)?;
cvt(unsafe { libc::symlink(original.as_ptr(), link.as_ptr()) })?;
Ok(())
}

pub fn link(src: &Path, dst: &Path) -> io::Result<()> {
let src = cstr(src)?;
let dst = cstr(dst)?;
pub fn link(original: &Path, link: &Path) -> io::Result<()> {
let original = cstr(original)?;
let link = cstr(link)?;
cfg_if::cfg_if! {
if #[cfg(any(target_os = "vxworks", target_os = "redox", target_os = "android"))] {
// VxWorks, Redox, and old versions of Android lack `linkat`, so use
// `link` instead. POSIX leaves it implementation-defined whether
// `link` follows symlinks, so rely on the `symlink_hard_link` test
// in library/std/src/fs/tests.rs to check the behavior.
cvt(unsafe { libc::link(src.as_ptr(), dst.as_ptr()) })?;
cvt(unsafe { libc::link(original.as_ptr(), link.as_ptr()) })?;
} else {
// Use `linkat` with `AT_FDCWD` instead of `link` as `linkat` gives
// us a flag to specify how symlinks should be handled. Pass 0 as
// the flags argument, meaning don't follow symlinks.
cvt(unsafe { libc::linkat(libc::AT_FDCWD, src.as_ptr(), libc::AT_FDCWD, dst.as_ptr(), 0) })?;
cvt(unsafe { libc::linkat(libc::AT_FDCWD, original.as_ptr(), libc::AT_FDCWD, link.as_ptr(), 0) })?;
}
}
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/unsupported/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ pub fn readlink(_p: &Path) -> io::Result<PathBuf> {
unsupported()
}

pub fn symlink(_src: &Path, _dst: &Path) -> io::Result<()> {
pub fn symlink(_original: &Path, _link: &Path) -> io::Result<()> {
unsupported()
}

Expand Down
20 changes: 10 additions & 10 deletions library/std/src/sys/wasi/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,19 +549,19 @@ fn read_link(fd: &WasiFd, file: &Path) -> io::Result<PathBuf> {
}
}

pub fn symlink(src: &Path, dst: &Path) -> io::Result<()> {
let (dst, dst_file) = open_parent(dst)?;
dst.symlink(osstr2str(src.as_ref())?, osstr2str(dst_file.as_ref())?)
pub fn symlink(original: &Path, link: &Path) -> io::Result<()> {
let (link, link_file) = open_parent(link)?;
link.symlink(osstr2str(original.as_ref())?, osstr2str(link_file.as_ref())?)
}

pub fn link(src: &Path, dst: &Path) -> io::Result<()> {
let (src, src_file) = open_parent(src)?;
let (dst, dst_file) = open_parent(dst)?;
src.link(
pub fn link(original: &Path, link: &Path) -> io::Result<()> {
let (original, original_file) = open_parent(original)?;
let (link, link_file) = open_parent(link)?;
original.link(
wasi::LOOKUPFLAGS_SYMLINK_FOLLOW,
osstr2str(src_file.as_ref())?,
&dst,
osstr2str(dst_file.as_ref())?,
osstr2str(original_file.as_ref())?,
&link,
osstr2str(link_file.as_ref())?,
)
}

Expand Down
12 changes: 6 additions & 6 deletions library/std/src/sys/windows/ext/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ impl FileTypeExt for fs::FileType {

/// Creates a new file symbolic link on the filesystem.
///
/// The `dst` path will be a file symbolic link pointing to the `src`
/// The `link` path will be a file symbolic link pointing to the `original`
/// path.
///
/// # Examples
Expand All @@ -533,13 +533,13 @@ impl FileTypeExt for fs::FileType {
/// }
/// ```
#[stable(feature = "symlink", since = "1.1.0")]
pub fn symlink_file<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::Result<()> {
sys::fs::symlink_inner(src.as_ref(), dst.as_ref(), false)
pub fn symlink_file<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
sys::fs::symlink_inner(original.as_ref(), link.as_ref(), false)
}

/// Creates a new directory symlink on the filesystem.
///
/// The `dst` path will be a directory symbolic link pointing to the `src`
/// The `link` path will be a directory symbolic link pointing to the `original`
/// path.
///
/// # Examples
Expand All @@ -553,6 +553,6 @@ pub fn symlink_file<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::Resul
/// }
/// ```
#[stable(feature = "symlink", since = "1.1.0")]
pub fn symlink_dir<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::Result<()> {
sys::fs::symlink_inner(src.as_ref(), dst.as_ref(), true)
pub fn symlink_dir<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
sys::fs::symlink_inner(original.as_ref(), link.as_ref(), true)
}
39 changes: 22 additions & 17 deletions library/std/src/sys/windows/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -759,30 +759,32 @@ pub fn readlink(path: &Path) -> io::Result<PathBuf> {
file.readlink()
}

pub fn symlink(src: &Path, dst: &Path) -> io::Result<()> {
symlink_inner(src, dst, false)
pub fn symlink(original: &Path, link: &Path) -> io::Result<()> {
symlink_inner(original, link, false)
}

pub fn symlink_inner(src: &Path, dst: &Path, dir: bool) -> io::Result<()> {
let src = to_u16s(src)?;
let dst = to_u16s(dst)?;
pub fn symlink_inner(original: &Path, link: &Path, dir: bool) -> io::Result<()> {
let original = to_u16s(original)?;
let link = to_u16s(link)?;
let flags = if dir { c::SYMBOLIC_LINK_FLAG_DIRECTORY } else { 0 };
// Formerly, symlink creation required the SeCreateSymbolicLink privilege. For the Windows 10
// Creators Update, Microsoft loosened this to allow unprivileged symlink creation if the
// computer is in Developer Mode, but SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE must be
// added to dwFlags to opt into this behaviour.
let result = cvt(unsafe {
c::CreateSymbolicLinkW(
dst.as_ptr(),
src.as_ptr(),
link.as_ptr(),
original.as_ptr(),
flags | c::SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE,
) as c::BOOL
});
if let Err(err) = result {
if err.raw_os_error() == Some(c::ERROR_INVALID_PARAMETER as i32) {
// Older Windows objects to SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE,
// so if we encounter ERROR_INVALID_PARAMETER, retry without that flag.
cvt(unsafe { c::CreateSymbolicLinkW(dst.as_ptr(), src.as_ptr(), flags) as c::BOOL })?;
cvt(unsafe {
c::CreateSymbolicLinkW(link.as_ptr(), original.as_ptr(), flags) as c::BOOL
})?;
} else {
return Err(err);
}
Expand All @@ -791,15 +793,15 @@ pub fn symlink_inner(src: &Path, dst: &Path, dir: bool) -> io::Result<()> {
}

#[cfg(not(target_vendor = "uwp"))]
pub fn link(src: &Path, dst: &Path) -> io::Result<()> {
let src = to_u16s(src)?;
let dst = to_u16s(dst)?;
cvt(unsafe { c::CreateHardLinkW(dst.as_ptr(), src.as_ptr(), ptr::null_mut()) })?;
pub fn link(original: &Path, link: &Path) -> io::Result<()> {
let original = to_u16s(original)?;
let link = to_u16s(link)?;
cvt(unsafe { c::CreateHardLinkW(link.as_ptr(), original.as_ptr(), ptr::null_mut()) })?;
Ok(())
}

#[cfg(target_vendor = "uwp")]
pub fn link(_src: &Path, _dst: &Path) -> io::Result<()> {
pub fn link(_original: &Path, _link: &Path) -> io::Result<()> {
return Err(io::Error::new(io::ErrorKind::Other, "hard link are not supported on UWP"));
}

Expand Down Expand Up @@ -883,8 +885,11 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
}

#[allow(dead_code)]
pub fn symlink_junction<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::Result<()> {
symlink_junction_inner(src.as_ref(), dst.as_ref())
pub fn symlink_junction<P: AsRef<Path>, Q: AsRef<Path>>(
original: P,
junction: Q,
) -> io::Result<()> {
symlink_junction_inner(original.as_ref(), junction.as_ref())
}

// Creating a directory junction on windows involves dealing with reparse
Expand All @@ -893,7 +898,7 @@ pub fn symlink_junction<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::R
//
// http://www.flexhex.com/docs/articles/hard-links.phtml
#[allow(dead_code)]
fn symlink_junction_inner(target: &Path, junction: &Path) -> io::Result<()> {
fn symlink_junction_inner(original: &Path, junction: &Path) -> io::Result<()> {
let d = DirBuilder::new();
d.mkdir(&junction)?;

Expand All @@ -911,7 +916,7 @@ fn symlink_junction_inner(target: &Path, junction: &Path) -> io::Result<()> {
// FIXME: this conversion is very hacky
let v = br"\??\";
let v = v.iter().map(|x| *x as u16);
for c in v.chain(target.as_os_str().encode_wide()) {
for c in v.chain(original.as_os_str().encode_wide()) {
*buf.offset(i) = c;
i += 1;
}
Expand Down

0 comments on commit 09c9c9f

Please sign in to comment.