zip-rs / zip-old

Zip implementation in Rust
MIT License
731 stars 204 forks source link

Restore MIPS support? #284

Closed n3vu0r closed 2 years ago

n3vu0r commented 2 years ago

276 removed MIPS (mips-unknown-linux-gnu) support. Is it possible to use smaller atomics to restore support for that platform? Or to disable this optimization for this target.

n3vu0r commented 2 years ago

I think it's possible on MIPS by using atomic-shim. I will give it a try.

paolobarbolini commented 2 years ago

A few other options:

zamazan4ik commented 2 years ago

Sorry for that. I have no MIPS hardware (and do not want to set up QEMU emulator for that, sorry).

It would be awesome if you can provide a PR with the fixes for working on MIPS again since I think there is no reason to break compatibility with this arch now.

Maybe for us it is also a good sign to think about testing the library on CI not only on x86-64 architecture.

n3vu0r commented 2 years ago

I got all tests passing with atomic-shim on a MIPS docker and can provide a PR.

@paolobarbolini Can you please explain why you don't like this solution? atomic-shim depends on crossbeam-utils and implements the fallback of AtomicU64 with ShardedLock. AtomicCell doesn't provide get_mut() and without this method it doesn't seem to be a trivial drop-in replacement.

n3vu0r commented 2 years ago

Ah I guess we could use a cfg-dependency if we use crossbeam-utils directly within zip::types::AtomicU64.

paolobarbolini commented 2 years ago

I got all tests passing with atomic-shim on a MIPS docker and can provide a PR.

@paolobarbolini Can you please explain why you don't like this solution? atomic-shim depends on crossbeam-utils and implements the fallback of AtomicU64 with ShardedLock. AtomicCell doesn't provide get_mut() and without this method it doesn't seem to be a trivial drop-in replacement.

It's just that atomic-shim is so simple of a crate that I'd prefer seeing crossbeam-utils used directly instead. I like your SharedLock proposal

messense commented 2 years ago

0.6.1 is still broken for armv5te:

Compiling zip v0.6.1
error[E0412]: cannot find type `AtomicU64` in module `atomic`
    --> /root/.cargo/registry/src/github.com-1ecc62[9](https://github.com/messense/aliyundrive-webdav/runs/5796858930?check_suite_focus=true#step:5:9)9db9ec823/zip-0.6.1/src/types.rs:258:30
     |
258  |   pub struct AtomicU64(atomic::AtomicU64);
     |                                ^^^^^^^^^ help: a struct with a similar name exists: `AtomicU[16](https://github.com/messense/aliyundrive-webdav/runs/5796858930?check_suite_focus=true#step:5:16)`
     |
note: struct `crate::write::zip_writer::AtomicU64` exists but is inaccessible
    --> /root/.cargo/registry/src/github.com-1ecc6299db9ec823/zip-0.6.1/src/types.rs:258:1
     |
258  | pub struct AtomicU64(atomic::AtomicU64);
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not accessible

error[E0433]: failed to resolve: could not find `AtomicU64` in `atomic`
    --> /root/.cargo/registry/src/github.com-1ecc6299db9ec823/zip-0.6.1/src/types.rs:262:[22](https://github.com/messense/aliyundrive-webdav/runs/5796858930?check_suite_focus=true#step:5:22)
     |
262  |           Self(atomic::AtomicU64::new(v))
     |                        ^^^^^^^^^ help: a struct with a similar name exists: `AtomicU16`
     |
note: struct `crate::write::zip_writer::AtomicU64` exists but is inaccessible
    --> /root/.cargo/registry/src/github.com-1ecc6299db9ec8[23](https://github.com/messense/aliyundrive-webdav/runs/5796858930?check_suite_focus=true#step:5:23)/zip-0.6.1/src/types.rs:[25](https://github.com/messense/aliyundrive-webdav/runs/5796858930?check_suite_focus=true#step:5:25)8:1
     |
258  | pub struct AtomicU64(atomic::AtomicU64);
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not accessible

error[E0433]: failed to resolve: could not find `AtomicU64` in `atomic`
    --> /root/.cargo/registry/src/github.com-1ecc6299db9ec823/zip-0.6.1/src/types.rs:[28](https://github.com/messense/aliyundrive-webdav/runs/5796858930?check_suite_focus=true#step:5:28)0:22
     |
280  |           Self(atomic::AtomicU64::new(self.load()))
     |                        ^^^^^^^^^ help: a struct with a similar name exists: `AtomicU16`
     |
note: struct `crate::write::zip_writer::AtomicU64` exists but is inaccessible
    --> /root/.cargo/registry/src/github.com-1ecc6[29](https://github.com/messense/aliyundrive-webdav/runs/5796858930?check_suite_focus=true#step:5:29)9db9ec823/zip-0.6.1/src/types.rs:258:1
     |
258  | pub struct AtomicU64(atomic::AtomicU64);
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not accessible