CasualX / pelite

Lightweight, memory-safe, zero-allocation library for reading and navigating PE binaries.
MIT License
280 stars 42 forks source link

New version of pelite-macros broke 0.8.1 #243

Closed tobywf closed 3 years ago

tobywf commented 3 years ago

In pelite 0.8.1, pelite-macros is pinned to ^0.1.0 (per crates.io). This now horribly breaks with pelite-macros 0.1.1:

   Compiling pelite v0.8.1
error[E0432]: unresolved import `pelite_macros::pattern_attribute`
  --> /Users/zunder/.cargo/registry/src/github.com-1ecc6299db9ec823/pelite-0.8.1/src/lib.rs:35:9
   |
35 | pub use pelite_macros::pattern_attribute;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `pattern_attribute` in the root

error[E0432]: unresolved import `pelite_macros::Pod`
  --> /Users/zunder/.cargo/registry/src/github.com-1ecc6299db9ec823/pelite-0.8.1/src/lib.rs:55:9
   |
55 | pub use pelite_macros::Pod;
   |         ^^^^^^^^^^^^^^^^^^ no `Pod` in the root

error[E0432]: unresolved import `pelite_macros::_Pod`
  --> /Users/zunder/.cargo/registry/src/github.com-1ecc6299db9ec823/pelite-0.8.1/src/lib.rs:58:16
   |
58 | pub(crate) use pelite_macros::_Pod;
   |                ^^^^^^^^^^^^^^^^^^^ no `_Pod` in the root

error: cannot find derive macro `Pod` in this scope
  --> /Users/zunder/.cargo/registry/src/github.com-1ecc6299db9ec823/pelite-0.8.1/src/image.rs:69:23
   |
69 | #[derive(Copy, Clone, Pod, Debug)]
   |                       ^^^
...

This can be solved by pinning pelite-macros directly:

pelite-macros = "=0.1.0"
pelite = "0.8.1"

You might consider bumping the version number more significantly for breaking changes in future? :)

pelite 0.9.0 also broke the way resources work, I guess the changelog reference "Refactored resources paths" means this? pelite::resources::RSRC_TYPES isn't really documented/is internal, so the find()/find_data()/find_dir() methods are super brittle. Looking into the code, I did finally figure out how to use the methods that take Names, which initially I thought I couldn't until I saw Name::Id() this time around.

(On that note, it's also kind of confusing the commit for 0.8.1 is not merged into the main branch. If you just look at the master history, it jumps from 0.8.0 to 0.9.0, and 0.8.1 is only accessible from the tag (although there is no tag for 0.9.0))

Alright, hope this helps! I do love using this library, it does exactly what I want and does it well. This release was a bit of a mixed bag, but small things like by_name help a lot. Cheers!

tobywf commented 3 years ago

(Closed this since I thought I'd just drop it in in case anybody else hit this, but if you plan to release 0.8.2 with a stricter pin or some other workaround, please feel free to re-open!)

CasualX commented 3 years ago

Ouch, my bad. I thought about it for a little and figured it'd be alright but of course it isn't.

I don't have much time right now to fix this but I'll see if I can do a simple strict pinning fix.

tobywf commented 3 years ago

No worries. It might not need fixing, as outlined above, consumers of 0.8.1 can self-help by either upgrade to 0.9.0, or doing the pin in their own Cargo.toml:

pelite-macros = "=0.1.0"
pelite = "0.8.1"

I found both of these acceptable, so this was more of an FYI. Tagging the commit used for 0.9.0 would be really helpful though for future investigations.