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

Do not error on _set_perms(path) on Wasm32 #297

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fschutt
Copy link

@fschutt fschutt commented Jul 6, 2022

Since Wasm32 does not yet have support for file permissions it is better to silently ignore them rather than to fail, since the rest of the library can still work on wasm32.

Since Wasm32 does not yet have support for file permissions
it is better to silently ignore them rather than to fail,
since the rest of the library can still work on Wasm32.
@alexcrichton
Copy link
Owner

I think there's already configuration to ignore setting permissions, so I think it might be better to enable that instead of silently ignoring file permissions?

@fschutt
Copy link
Author

fschutt commented Jul 7, 2022

It errors even if using set_preserve_permissions(false). I just found this issue because file.unpack() triggers this case on wasm32-wasi:

type UnpackedFiles = BTreeMap<DirOrFile, Vec<u8>>;

/// Unzips a .tar.gz file, returning the [FileName => FileContents]
pub fn unpack_tar_gz(bytes: Vec<u8>) -> Result<UnpackedFiles, ConvertError> {
    use flate2::read::GzDecoder;
    use std::io::{Cursor, Read};
    use tar::{Archive, EntryType, Unpacked};

    let mut cursor = Cursor::new(bytes);
    let mut archive = Archive::new(GzDecoder::new(cursor));

    // mtime is not implemented on WASM
    #[cfg(target_arch = "wasm32")] {
        archive.set_preserve_mtime(false);
        archive.set_preserve_permissions(false);
    }

    // TODO(felix): it would be ideal if the .tar.gz file could
    // be unpacked in-memory instead of requiring disk access.

    // Use a random directory name for unpacking: in case the
    // tool is ran in parallel, this would otherwise lead to
    // file conflicts
    let rand_dir = rand::random::<u64>();
    let tempdir = webc::webc_temp_dir()
        .join("wapm-to-webc")
        .join(&format!("{rand_dir}"));

    let _ = std::fs::remove_dir(&tempdir); // no error if dir doesn't exist
    let _ = std::fs::create_dir_all(&tempdir)
        .map_err(|e| ConvertError::Io(format!("{}", tempdir.display()), e))?;

    let mut files = BTreeMap::default();

    for (i, file) in archive.entries().unwrap().enumerate() {
        let mut file = file.map_err(|e| ConvertError::Io(format!("{}", tempdir.display()), e))?;

        let file_type = file.header().entry_type();

        let path = file
            .path()
            .map_err(|e| ConvertError::Io(format!("{}", tempdir.display()), e))?
            .to_owned()
            .to_path_buf();

        let outpath = tempdir.clone().join(&format!("{i}.bin"));

        let _ = file
            .unpack(&outpath) // <------------------------------- ERROR
            .map_err(|e| ConvertError::Io(format!("{}", outpath.display()), e))?;

        let path = match file_type {
            EntryType::Regular => DirOrFile::File(path),
            EntryType::Directory => DirOrFile::Dir(path),
            e => {
                return Err(ConvertError::Other(anyhow::anyhow!(
                    "Invalid file_type for path \"{}\": {:?}",
                    path.display(),
                    e
                )));
            }
        };

        let bytes = match &path {
            DirOrFile::File(_) => std::fs::read(&outpath)
                .map_err(|e| ConvertError::Io(format!("{}", outpath.display()), e))?,
            DirOrFile::Dir(_) => Vec::new(),
        };

        files.insert(path, bytes);
    }

    nuke_dir(tempdir)?;

    Ok(files)
}

@alexcrichton
Copy link
Owner

If archive.set_preserve_permissions(false) doesn't work then I think that's the bug to fix rather than ignoring it on wasm

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.

2 participants