Skip to content

Commit

Permalink
Resolve compile errors and lints on Windows
Browse files Browse the repository at this point in the history
Despite the Windows build in CI passing, running `cargo build` on the
project on Windows had numerous compile errors (for me).

Signed-off-by: J Robert Ray <[email protected]>
  • Loading branch information
jrray committed Dec 3, 2024
1 parent 9cf3488 commit d574b5e
Show file tree
Hide file tree
Showing 22 changed files with 130 additions and 32 deletions.
2 changes: 2 additions & 0 deletions crates/spfs-cli/cmd-enter/src/cmd_enter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ use clap::{Args, Parser};
#[cfg(feature = "sentry")]
use cli::configure_sentry;
use miette::{Context, Result};
#[cfg(unix)]
use spfs::monitor::SPFS_MONITOR_FOREGROUND_LOGGING_VAR;
use spfs::storage::fs::RenderSummary;
use spfs_cli_common as cli;
use spfs_cli_common::CommandName;
#[cfg(unix)]
use tokio::io::AsyncWriteExt;

// The runtime setup process manages the current namespace
Expand Down
24 changes: 22 additions & 2 deletions crates/spfs-cli/cmd-fuse/src/cmd_fuse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,28 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/spkenv/spk

#[cfg(unix)]
use std::sync::Arc;
#[cfg(unix)]
use std::time::Duration;

use clap::Parser;
#[cfg(unix)]
use fuser::MountOption;
use miette::{bail, miette, Context, IntoDiagnostic, Result};
use miette::Result;
#[cfg(unix)]
use miette::{bail, miette, Context, IntoDiagnostic};
use spfs::tracking::EnvSpec;
#[cfg(unix)]
use spfs::Error;
use spfs_cli_common::{self as cli, warn_and_sentry_event};
#[cfg(unix)]
use spfs_cli_common::warn_and_sentry_event;
use spfs_cli_common::{self as cli};
#[cfg(unix)]
use spfs_vfs::{Config, Session};
#[cfg(unix)]
use tokio::signal::unix::{signal, SignalKind};
#[cfg(unix)]
use tokio::time::timeout;

// The runtime setup process manages the current namespace
Expand Down Expand Up @@ -107,6 +118,7 @@ impl cli::CommandName for CmdFuse {
}

impl CmdFuse {
#[cfg(unix)]
pub fn run(&mut self, config: &spfs::Config) -> Result<i32> {
let calling_uid = nix::unistd::geteuid();
let calling_gid = nix::unistd::getegid();
Expand Down Expand Up @@ -356,8 +368,15 @@ impl CmdFuse {
result?.into_diagnostic()?;
Ok(0)
}

#[cfg(windows)]
pub fn run(&mut self, _config: &spfs::Config) -> Result<i32> {
eprintln!("spfs-fuse is not supported on Windows.");
Ok(1)
}
}

#[cfg(unix)]
/// Copies from the private [`fuser::MountOption::from_str`]
fn parse_options_from_args(args: &[String]) -> Vec<MountOption> {
args.iter()
Expand Down Expand Up @@ -386,6 +405,7 @@ fn parse_options_from_args(args: &[String]) -> Vec<MountOption> {
.collect()
}

#[cfg(unix)]
/// Checks if fusermount3 is available to be used on this system
fn fuse3_available() -> bool {
spfs::which("fusermount3").is_some()
Expand Down
10 changes: 9 additions & 1 deletion crates/spfs-cli/cmd-monitor/src/cmd_monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ use clap::Parser;
#[cfg(feature = "sentry")]
use cli::configure_sentry;
use miette::{Context, IntoDiagnostic, Result};
#[cfg(unix)]
use spfs::Error;
use spfs_cli_common as cli;
use spfs_cli_common::CommandName;
use tokio::io::AsyncReadExt;
#[cfg(unix)]
use tokio::signal::unix::{signal, SignalKind};
use tokio::time::timeout;

Expand Down Expand Up @@ -87,6 +89,7 @@ impl CmdMonitor {
// clean up this runtime and all other threads before detaching
drop(rt);

#[cfg(unix)]
nix::unistd::daemon(self.no_chdir, self.no_close)
.into_diagnostic()
.wrap_err("Failed to daemonize the monitor process")?;
Expand Down Expand Up @@ -142,10 +145,13 @@ impl CmdMonitor {
}

pub async fn run_async(&mut self, config: &spfs::Config) -> Result<i32> {
#[cfg(unix)]
let mut interrupt = signal(SignalKind::interrupt())
.map_err(|err| Error::process_spawn_error("signal()", err, None))?;
#[cfg(unix)]
let mut quit = signal(SignalKind::quit())
.map_err(|err| Error::process_spawn_error("signal()", err, None))?;
#[cfg(unix)]
let mut terminate = signal(SignalKind::terminate())
.map_err(|err| Error::process_spawn_error("signal()", err, None))?;

Expand All @@ -158,6 +164,7 @@ impl CmdMonitor {
tracing::trace!("upgraded to owned runtime, waiting for empty runtime");

let fut = spfs::monitor::wait_for_empty_runtime(&owned, config);
#[cfg(unix)]
let res = tokio::select! {
res = fut => {
tracing::info!("Monitor detected no more processes, cleaning up runtime...");
Expand All @@ -169,7 +176,8 @@ impl CmdMonitor {
_ = interrupt.recv() => Err(spfs::Error::String("Interrupt signal received, cleaning up runtime early".to_string())),
_ = quit.recv() => Err(spfs::Error::String("Quit signal received, cleaning up runtime early".to_string())),
};
tracing::trace!("runtime empty of processes ");
#[cfg(windows)]
let res = fut.await;

// need to reload the runtime here to get any changes made to
// the runtime while it was running so we don't blast them the
Expand Down
4 changes: 2 additions & 2 deletions crates/spfs-vfs/src/winfsp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub struct Config {
/// that are managed via the [`Router`].
pub struct Service {
//repos: Vec<Arc<spfs::storage::RepositoryHandle>>,
config: Config,
_config: Config,
router: Router,
host: HostController,
// ttl: Duration,
Expand Down Expand Up @@ -92,7 +92,7 @@ impl Service {
Ok(Arc::new(Self {
host,
router,
config,
_config: config,
}))
}

Expand Down
8 changes: 8 additions & 0 deletions crates/spfs/src/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ mod bootstrap_test;

/// Environment variable used to store the original value of HOME
/// when launching through certain shells (tcsh).
// Allow: not used on Windows.
#[allow(dead_code)]
const SPFS_ORIGINAL_HOME: &str = "SPFS_ORIGINAL_HOME";

/// The environment variable used to store the message
Expand Down Expand Up @@ -275,6 +277,8 @@ pub(crate) fn build_spfs_remove_durable_command(
})
}

// Allow: not used on Windows.
#[allow(dead_code)]
pub(crate) fn build_spfs_remount_command(rt: &runtime::Runtime) -> Result<Command> {
let exe = match which_spfs("enter") {
None => return Err(Error::MissingBinary("spfs-enter")),
Expand All @@ -296,6 +300,8 @@ pub(crate) fn build_spfs_remount_command(rt: &runtime::Runtime) -> Result<Comman
})
}

// Allow: not used on Windows.
#[allow(dead_code)]
pub(crate) fn build_spfs_exit_command(rt: &runtime::Runtime) -> Result<Command> {
let exe = match which_spfs("enter") {
None => return Err(Error::MissingBinary("spfs-enter")),
Expand All @@ -317,6 +323,8 @@ pub(crate) fn build_spfs_exit_command(rt: &runtime::Runtime) -> Result<Command>
})
}

// Allow: not used on Windows.
#[allow(dead_code)]
pub(crate) fn build_spfs_change_to_durable_command(rt: &runtime::Runtime) -> Result<Command> {
let exe = match which_spfs("enter") {
None => return Err(Error::MissingBinary("spfs-enter")),
Expand Down
8 changes: 8 additions & 0 deletions crates/spfs/src/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,13 +685,21 @@ where
let meta = tokio::fs::symlink_metadata(&path).await.map_err(|err| {
Error::StorageReadError("metadata on proxy file", path.clone(), err)
})?;
// Allow: remove when todo!() is removed.
#[allow(unused_variables)]
let mtime = meta.modified().map_err(|err| {
Error::StorageReadError("modified time on proxy file", path.clone(), err)
})?;
#[cfg(unix)]
// Allow: remove when todo!() is removed.
#[allow(unused_variables)]
let has_hardlinks = meta.st_nlink() > 1;
#[cfg(windows)]
// Allow: remove when todo!() is removed.
#[allow(unused_variables)]
let has_hardlinks = todo!();
// Allow: remove when todo!() is removed.
#[allow(unreachable_code)]
let is_old_enough = DateTime::<Utc>::from(mtime) < self.must_be_older_than;
if has_hardlinks || !is_old_enough {
Ok(None)
Expand Down
2 changes: 1 addition & 1 deletion crates/spfs/src/monitor_win.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn spawn_monitor_for_runtime(_rt: &runtime::Runtime) -> Result<tokio::proces
///
/// This is a privileged operation that may fail with a permission
/// issue if the calling process is not root or CAP_NET_ADMIN
pub async fn wait_for_empty_runtime(_rt: &runtime::Runtime) -> Result<()> {
pub async fn wait_for_empty_runtime(_rt: &runtime::Runtime, _config: &crate::Config) -> Result<()> {
todo!()
}

Expand Down
2 changes: 2 additions & 0 deletions crates/spfs/src/repeating_timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ pin_project! {
pub struct Elapsed(());

impl<S: Stream> RepeatingTimeout<S> {
// Allow: not used on Windows.
#[allow(dead_code)]
pub(super) fn new(stream: S, duration: Duration) -> Self {
let next = Instant::now() + duration;
let deadline = tokio::time::sleep_until(next);
Expand Down
10 changes: 10 additions & 0 deletions crates/spfs/src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ pub struct RenderResult {
///
/// The return value is defined only if the spfs-render output could be parsed
/// successfully into a [`RenderResult`].
// Allow: not used on Windows.
#[allow(dead_code)]
async fn render_via_subcommand(
spec: tracking::EnvSpec,
kept_runtime: bool,
Expand Down Expand Up @@ -205,6 +207,8 @@ pub async fn compute_object_manifest(
/// If `skip_runtime_save` is true, the runtime will not be saved, even if
/// the `flattened_layers` property is modified. Only pass true here if the
/// runtime is unconditionally saved shortly after calling this function.
// Allow: not used on Windows.
#[allow(dead_code)]
pub(crate) async fn resolve_overlay_dirs<R>(
runtime: &mut runtime::Runtime,
repo: R,
Expand All @@ -224,6 +228,8 @@ where
impl ResolvedManifest {
/// Iterate over all the "existing" manifests contained within this
/// manifest.
// Allow: not used on Windows.
#[allow(dead_code)]
fn existing(self) -> impl Iterator<Item = graph::Manifest> {
// Find all the `Existing` manifests in this recursive structure,
// returning them in an order based on their original order, to
Expand All @@ -243,6 +249,8 @@ where
result.into_iter().map(|(_, m)| m)
}

// Allow: not used on Windows.
#[allow(dead_code)]
fn manifest(&self) -> &graph::Manifest {
match self {
ResolvedManifest::Existing { manifest, .. } => manifest,
Expand Down Expand Up @@ -354,6 +362,8 @@ where
/// If `skip_runtime_save` is true, the runtime will not be saved, even if
/// the `flattened_layers` property is modified. Only pass true here if the
/// runtime is unconditionally saved shortly after calling this function.
// Allow: not used on Windows.
#[allow(dead_code)]
pub(crate) async fn resolve_and_render_overlay_dirs(
runtime: &mut runtime::Runtime,
skip_runtime_save: bool,
Expand Down
3 changes: 3 additions & 0 deletions crates/spfs/src/runtime/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1313,6 +1313,9 @@ pub fn makedirs_with_perms<P: AsRef<Path>>(dirname: P, perms: u32) -> std::io::R
let dirname = dirname.as_ref();
#[cfg(unix)]
let perms = std::fs::Permissions::from_mode(perms);
#[cfg(windows)]
// Avoid unused variable warning.
let _perms = perms;

if !dirname.is_absolute() {
return Err(std::io::Error::new(
Expand Down
2 changes: 1 addition & 1 deletion crates/spfs/src/runtime/winfsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/spkenv/spk

pub fn is_removed_entry(meta: &std::fs::Metadata) -> bool {
pub fn is_removed_entry(_meta: &std::fs::Metadata) -> bool {
// WinFSP does not have a working directory that stores whiteout files (yet)
// so this function always returns false
false
Expand Down
6 changes: 3 additions & 3 deletions crates/spfs/src/status_win.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ pub async fn exit_runtime(_rt: &runtime::Runtime) -> Result<()> {

/// Turn the given runtime into a durable runtime, this should only
/// ever be called with the active runtime
pub async fn make_runtime_durable(rt: &runtime::Runtime) -> Result<()> {
pub async fn make_runtime_durable(_rt: &runtime::Runtime) -> Result<()> {
todo!()
}
/// Reinitialize the current spfs runtime as a durable rt (after runtime config changes).
///
pub async fn change_to_durable_runtime(rt: &mut runtime::Runtime) -> Result<RenderSummary> {
pub async fn change_to_durable_runtime(_rt: &mut runtime::Runtime) -> Result<RenderSummary> {
Err(Error::OverlayFsUnsupportedOnWindows)
}

Expand All @@ -40,7 +40,7 @@ pub async fn reinitialize_runtime(_rt: &mut runtime::Runtime) -> Result<RenderSu
/// the mount namespacing operated per-thread and so restricts our ability to move execution.
pub async fn initialize_runtime(rt: &mut runtime::Runtime) -> Result<RenderSummary> {
tracing::debug!("computing runtime manifest");
let manifest = super::compute_runtime_manifest(rt).await?;
let _manifest = super::compute_runtime_manifest(rt).await?;

let configurator = env::RuntimeConfigurator::default();
match rt.config.mount_backend {
Expand Down
5 changes: 5 additions & 0 deletions crates/spfs/src/storage/fs/hash_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,11 @@ impl FsHashStore {
}
}

#[cfg(windows)]
if created_new_file || object_permissions.is_some() {
// avoid unused variable warning
}

Ok((digest, copied))
}

Expand Down
2 changes: 1 addition & 1 deletion crates/spfs/src/storage/fs/renderer_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ impl ManifestRenderPath for OpenFsRepository {
struct BlobSemaphore(Arc<Semaphore>);

/// A newtype to represent holding the permit specifically for the blob semaphore.
// dead_code: .0 is never read, but it still serves a purpose.
// Allow: .0 is never read, but it still serves a purpose.
#[allow(dead_code)]
struct BlobSemaphorePermit<'a>(tokio::sync::SemaphorePermit<'a>);

Expand Down
12 changes: 9 additions & 3 deletions crates/spfs/src/storage/fs/renderer_win.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,21 @@ impl OpenFsRepository {
}

/// A semaphore for limiting the concurrency of blob renders.
// Allow: not used on Windows (yet?).
#[allow(dead_code)]
struct BlobSemaphore(Arc<Semaphore>);

/// A newtype to represent holding the permit specifically for the blob semaphore.
// Allow: .0 is never read, but it still serves a purpose.
#[allow(dead_code)]
struct BlobSemaphorePermit<'a>(tokio::sync::SemaphorePermit<'a>);

impl BlobSemaphore {
/// Acquires a permit from the blob semaphore.
///
/// Wrapper around [`tokio::sync::Semaphore::acquire`].
// Allow: not used on Windows (yet?).
#[allow(dead_code)]
async fn acquire(&self) -> BlobSemaphorePermit<'_> {
BlobSemaphorePermit(
self.0
Expand All @@ -172,7 +178,7 @@ impl BlobSemaphore {
/// Renders manifest data to a directory on disk
pub struct Renderer<'repo, Repo, Reporter: RenderReporter = SilentRenderReporter> {
repo: &'repo Repo,
reporter: Arc<Reporter>,
_reporter: Arc<Reporter>,
blob_semaphore: BlobSemaphore,
max_concurrent_branches: usize,
}
Expand All @@ -181,7 +187,7 @@ impl<'repo, Repo> Renderer<'repo, Repo, SilentRenderReporter> {
pub fn new(repo: &'repo Repo) -> Self {
Self {
repo,
reporter: Arc::new(SilentRenderReporter),
_reporter: Arc::new(SilentRenderReporter),
blob_semaphore: BlobSemaphore(Arc::new(Semaphore::new(DEFAULT_MAX_CONCURRENT_BLOBS))),
max_concurrent_branches: DEFAULT_MAX_CONCURRENT_BRANCHES,
}
Expand All @@ -201,7 +207,7 @@ where
{
Renderer {
repo: self.repo,
reporter: reporter.into(),
_reporter: reporter.into(),
blob_semaphore: self.blob_semaphore,
max_concurrent_branches: self.max_concurrent_branches,
}
Expand Down
Loading

0 comments on commit d574b5e

Please sign in to comment.