Haze fails to delete files from the export path silently

[Nusiq]

It looks like Haze silently ignores errors in deleting the db files during the haze export <world-name> -o command. It runs as usual and exits with a message: Updated world "<world-name>" in the "minecraftWorlds" directory (overwrite).

I keep the project files on the same drive as Minecraft.

The db directory from the Regolith project

The world files (db) after opening the world in Minecraft and closing it

The same world but after running the export command from Haze

[Nusiq]

It looks like Haze doesn't even try to delete the world files. It leads to basically corrupting the world. The problem will be present both on importing and exporting.

Unless I don't understand the code correctly (which is possible because I don't know Rust very well). This function is used in export: https://github.com/salpland/haze/blob/4cabd596c9ecd74a32e7d077fc76a2b94c05074b/crates/haze_core/src/world.rs#L102-L117

[arexon]

Yeah, that's correct. The implementation is pretty naive, it just copies the files over to the "target" directory without first cleaning it up.

I will work on fixing it this weekend, along with a much needed refactor to address some other glaring issues.

[Nusiq]

Since you mentioned a refactor I won't try to make any pull requests but I made an unpolished version of the world.rs file that fixes this issue, so maybe it will be useful for you:

Before deletion it opens all of the files to make sure that nothing is using it. If some files are being used it puts that into a list in an error for the user. It only deletes the files if it can access all of them at once at the same time. The error handling in this code is pretty bad and doesn't match the style of the repository so you would have to change it.

It's able to detect when the world is open in Minecraft. I'm not sure if it actually keeps the files locked all at once. I haven't tested it that much.

Adds delete_dir and lock_dir functions.

use std::{
    env, fs, io,
    path::{Path, PathBuf},
};

use crate::error::Error;

/// Exports the given world to the `minecraftWorlds` directory.
pub fn export(name: &str, worlds: &[String], target: &str, overwrite: bool) -> Result<(), Error> {
    let from = local_worlds_dir(worlds, name)?;
    let to = mojang_worlds_dir(name, target)?;
    if to.exists() {
        if !overwrite {
            return Err(Error::CannotOverwriteWorld(name.to_string()));
        }
        match delete_dir(&to) {
            Ok(_) => {}
            Err(e) => {
                // TODO - proper error handling
                println!("{}", e);
                return Err(Error::CannotOverwriteWorld(name.to_string()));
            }
        }
    }

    copy_dir(&from, &to).map_err(|e| Error::CannotCopyWorld(e.kind(), name.to_string()))?;

    Ok(())
}

/// Imports the given world from the `minecraftWorlds` directory.
pub fn import(name: &str, worlds: &[String], target: &str) -> Result<(), Error> {
    let from = mojang_worlds_dir(name, target)?;
    let to: PathBuf = local_worlds_dir(worlds, name)?;
    match delete_dir(&to) {
        Ok(_) => {}
        Err(e) => {
            // TODO - proper error handling
            println!("{}", e);
            return Err(Error::CannotOverwriteWorld(name.to_string()));
        }
    }
    copy_dir(&from, &to).map_err(|e| Error::CannotCopyWorld(e.kind(), name.to_string()))?;

    Ok(())
}

/// Returns the list of worlds from the given glob patterns.
pub fn all_worlds(globs: &[String]) -> Result<Vec<String>, Error> {
    let worlds = globs
        .iter()
        .map(|pattern| {
            glob::glob(pattern).map_err(|e| Error::InvalidWorldsGlobPattern(e, pattern.to_string()))
        })
        .collect::<Result<Vec<_>, _>>()?
        .into_iter()
        .flatten()
        .filter_map(Result::ok)
        .filter(|p| p.is_dir())
        .map(|p| p.file_name().unwrap().to_string_lossy().to_string())
        .collect::<Vec<_>>();

    if worlds.is_empty() {
        return Err(Error::EmptyWorldsProperty);
    }

    Ok(worlds)
}

/// Returns local worlds directory from the given glob patterns.
fn local_worlds_dir(globs: &[String], name: &str) -> Result<PathBuf, Error> {
    let paths = globs
        .iter()
        .map(|pattern| {
            glob::glob(pattern).map_err(|e| Error::InvalidWorldsGlobPattern(e, pattern.to_string()))
        })
        .collect::<Result<Vec<_>, _>>()?
        .into_iter()
        .flatten()
        .filter_map(Result::ok)
        .filter(|p| p.is_dir())
        .collect::<Vec<_>>();

    if paths.is_empty() {
        return Err(Error::EmptyWorldsProperty);
    }

    match paths.iter().find(|p| p.ends_with(name)) {
        Some(path) => Ok(path.clone()),
        None => Err(Error::WorldNotFound(paths, name.to_string())),
    }
}

/// Returns the path to the mojang worlds directory.
fn mojang_worlds_dir(name: &str, target: &str) -> Result<PathBuf, Error> {
    let target = match target {
        "stable" => "Microsoft.MinecraftUWP_8wekyb3d8bbwe",
        "preview" => "Microsoft.MinecraftWindowsBeta_8wekyb3d8bbwe",
        "education" => "Microsoft.MinecraftEducationEdition_8wekyb3d8bbwe",
        path => return Ok(PathBuf::from(path)),
    };

    env::var("LOCALAPPDATA")
        .map(|base| {
            PathBuf::from(&base)
                .join("Packages")
                .join(target)
                .join("LocalState")
                .join("games")
                .join("com.mojang")
                .join("minecraftWorlds")
                .join(name)
        })
        .ok()
        .ok_or_else(Error::CannotFindLocalAppData)
}

/// Copies a directory recursively.
fn copy_dir(from: &Path, to: &Path) -> Result<(), io::Error> {
    fs::create_dir_all(to)?;

    for entry in fs::read_dir(from)? {
        let entry = entry?;
        let file_type = entry.file_type()?;
        if file_type.is_dir() {
            copy_dir(&entry.path(), &to.join(entry.file_name()))?;
        } else {
            fs::copy(&entry.path(), &to.join(entry.file_name()))?;
        }
    }

    Ok(())
}

/// Walks provided target directory and returns a list of locks to all of the files recursively.
fn lock_dir(target: &Path) -> Result<Vec<(PathBuf, Result<fs::File, io::Error>)>, io::Error> {
    lock_dir_helper(target, Vec::new())
}

fn lock_dir_helper(
    target: &Path,
    mut locks: Vec<(PathBuf, Result<fs::File, io::Error>)>,
) -> Result<Vec<(PathBuf, Result<fs::File, io::Error>)>, io::Error> {
    for entry in fs::read_dir(target)? {
        let entry = entry?;
        let file_type = entry.file_type()?;
        if file_type.is_dir() {
            locks = lock_dir_helper(&entry.path(), locks)?;
        } else {
            let lock = fs::OpenOptions::new()
                .read(true)
                .write(true)
                .open(entry.path());
            locks.push((entry.path(), lock));
        }
    }
    Ok(locks)
}

fn delete_dir(target: &Path) -> Result<(), io::Error> {
    // Lock all files in the directory to prevent any other process from accessing them.
    let mut locks = match lock_dir(target) {
        Ok(locks) => locks,
        Err(e) => {
            return Err(io::Error::new(
                io::ErrorKind::Other,
                format!(
                    concat!(
                        "Failed to prepare files for deletion:\n",
                        "Directory: {}\n",
                        "Error: {}",
                    ),
                    target.display(),
                    e
                ),
            ))
        }
    };

    // Check if any of the locks failed.
    let lock_failed_paths = locks
        .iter()
        .filter(|(_, lock)| lock.is_err())
        .map(|(path, _)| path)
        .collect::<Vec<_>>();
    // If locks failed, return an error.
    if lock_failed_paths.len() > 0 {
        let path_str = lock_failed_paths
            .iter()
            .map(|p| format!("- {}", p.to_string_lossy()))
            .collect::<Vec<_>>()
            .join("\n");
        return Err(io::Error::new(
            io::ErrorKind::Other,
            format!(
                concat!(
                    "Failed to lock files for deletion:\n",
                    "Directory: {}\n",
                    "Paths:\n",
                    "{}",
                ),
                target.display(),
                path_str
            ),
        ));
    }
    // If all locks succeeded, delete the files.
    while let Some((path, lock)) = locks.pop() {
        // Release lock and delete a file.
        // WARNING: It's potentially possible that some othe process will lock the file after
        // we release the lock and before we delete the file.
        drop(lock);
        match fs::remove_file(&path) {
            Ok(_) => {}
            Err(e) => {
                return Err(io::Error::new(
                    io::ErrorKind::Other,
                    format!(
                        concat!(
                            "Critical error occurred while deleting file. ",
                            "The target directory was only partially deleted:\n",
                            "File: {}\n",
                            "Error: {}",
                        ),
                        path.display(),
                        e
                    ),
                ))
            }
        }
    }
    Ok(())
}

[arexon]

Thanks! A mechanism to ensure files are only copied when the target world isn't being used by Minecraft/another program is something I'm going to add next.

I'm not sure if it actually keeps the files locked all at once.

Looking at the code, it doesn't. Rust standard library doesn't have file locking, you'd need external crate for it. Although, I'm not sure how Minecraft would react to world files being locked. I'd have to look into that

[Nusiq]

I just checked using the modified version of a program using a lock from fs2 crate. If you lock the world files Minecraft won't be able to open the world (obviously). You'll get a content log error complaining about not being able to run the preview icon and if you try to open the world anyways, it will fail and the world will be removed from the list of the worlds until you restart the game.

I think just checking if the files are locked (by Minecraft) might be enough. If you add your own lock it will be slightly safer but people will sometimes get a content log error about missing texture. I don't think it's realistic to run haze and than quickly switch to Minecraft to try to open the world that you definitely shouldn't open before haze finishes running.

And yes, just opening a file in Rust doesn't lock it.

[arexon]

I don't think it's realistic to run haze and than quickly switch to Minecraft to try to open the world that you definitely shouldn't open before haze finishes running.

Yeah, I agree. I think rather than implementing file locking, a good middle ground could be to print a message every time a world is exported reminding users to not open the world in MC before the process is finished. This could be toggled off with an argument.

[Nusiq]

I made a PR that solves the issue until you have time to do your planned refactor. I think it follows the error handling style of the repository.