Skip to content

media/migrations: use a platform-neutral file symlink helper - #549

Open
obodnikov wants to merge 1 commit into
matrix-construct:mainfrom
obodnikov:fix/media-symlink-platform-neutral
Open

media/migrations: use a platform-neutral file symlink helper#549
obodnikov wants to merge 1 commit into
matrix-construct:mainfrom
obodnikov:fix/media-symlink-platform-neutral

Conversation

@obodnikov

Copy link
Copy Markdown
Contributor

What does this PR do?

tokio::fs::symlink is exported only on unix, so the three legacy-link call sites in
media/migrations.rs do not resolve elsewhere:

error[E0425]: cannot find function `symlink` in module `tokio::fs`
  --> src/service/media/migrations.rs:55:16
   |
55 |     tokio::fs::symlink(&path, &old_path).await?;
   |                ^^^^^^^ not found in `tokio::fs`

Every one of the three links to a file, and Windows separates that case from linking to
a directory, so this routes them through a small helper that picks the platform's
spelling:

/// Links `link` to the file at `target`.
///
/// `tokio::fs::symlink` is unix-only. Windows distinguishes a link to a file
/// from a link to a directory and offers `symlink_file` for the former, which
/// is what every caller here wants. Elsewhere there is no portable equivalent
/// to call.
async fn symlink_file(target: impl AsRef<Path>, link: impl AsRef<Path>) -> io::Result<()> {
	#[cfg(unix)]
	{
		tokio::fs::symlink(target, link).await
	}

	#[cfg(windows)]
	{
		tokio::fs::symlink_file(target, link).await
	}

	#[cfg(not(any(unix, windows)))]
	{
		let _ = (target, link);

		Err(io::Error::new(
			io::ErrorKind::Unsupported,
			"Symlinks are not supported on this platform.",
		))
	}
}

It takes impl AsRef<Path> for both arguments, matching the signature of the tokio
functions it wraps, so the call sites change only in the name. The third arm keeps
targets with neither spelling compiling rather than trading one unresolved name for
another.

tokio::fs::symlink_metadata and tokio::fs::remove_file, the other two filesystem
calls in this file, are already available on every target.

On the Windows arm. Creating a link there can still fail for want of the privilege
to make one. That surfaces as the same error a failing symlink already returns on unix,
and only under media_compat_file_link, which is off by default and exists for
compatibility with a directory layout that predates the sha256 migration.

No behaviour change on unix. Same class of fix as #526, #527, #528 and #536.

On verification. CI builds Linux only, where this is a no-op both before and after,
so CI cannot demonstrate the change. It was observed building v1.8.3 for
x86_64-pc-windows-msvc, and the error above is reproducible on any host with a one-file
crate that depends on tokio with the fs feature and calls tokio::fs::symlink.
cargo +nightly fmt --check, cargo check and cargo clippy on the unix path are clean
before and after.

Checklist

  • Code is formatted with nightly cargo fmt and satisfies clippy and
    rustc lints; any allowed lint is justified by an obvious reason or a
    comment.
  • Complement compliance changes (new passes or new failures), if any,
    are noted in the description above. — none; the unix path resolves to
    the same tokio::fs::symlink call it made before.
  • Config option changes were made in src/core/config/mod.rs doc
    comments and the regenerated tuwunel-example.toml is committed. — n/a
  • User-facing changes are reflected in docs/. — n/a
  • I agree that my changes may be licensed under the Apache-2.0 licence
    and my conduct is in line with the Contributor's Covenant and
    Tuwunel's Code of Conduct.

`tokio::fs::symlink` is exported only on unix, so the three legacy-link call
sites in `media/migrations.rs` do not resolve elsewhere:

    error[E0425]: cannot find function `symlink` in module `tokio::fs`
     --> src/service/media/migrations.rs:55:16
      |
      |     tokio::fs::symlink(&path, &old_path).await?;
      |                ^^^^^^^ not found in `tokio::fs`

Every one of them links to a file, and Windows separates that case from linking
to a directory, so this routes them through a small helper that picks the
platform's spelling: `symlink` on unix, `symlink_file` on Windows, and an
`Unsupported` error where there is no equivalent to call. It takes
`impl AsRef<Path>` for both arguments, matching the signature of the tokio
functions it wraps, so the call sites are unchanged apart from the name.

On Windows the call itself can still fail without the privilege to create a
link. That surfaces as the same error a failing `symlink` already returns on
unix, and only under `media_compat_file_link`, which is off by default.

No behaviour change on unix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant