mindeng / nom-exif

Exif/metadata parsing library written in pure Rust, both image (jpeg/heif/heic/jpg/tiff etc.) and video/audio (mov/mp4/3gp/webm/mkv/mka, etc.) files are supported.
https://crates.io/crates/nom-exif
MIT License
43 stars 6 forks source link

warning: supporting new formats is a breaking change! #11

Closed onkoe closed 1 week ago

onkoe commented 2 weeks ago

You need to add the #[non_exhaustive] attribute to the following types. Otherwise, changing them is a violation of semantic versioning and will break dependant crates:

In other words, it's a breaking change to support new formats, make new errors, or add to the recognized EXIF tag list! See: https://rust-lang.github.io/rust-clippy/rust-1.81.0/index.html#/exhaustive_enums

To prevent this after the next major version, please consider adding the following to Cargo.toml:

# other stuff...

[lints.clippy]
"exhaustive_enums" = "deny" # all `pub enum`s are `#[non_exhaustive]` 
onkoe commented 2 weeks ago

Note that adding this attribute is also a breaking change! It really does require a major version bump... 😄

mindeng commented 2 weeks ago

You need to add the #[non_exhaustive] attribute to the following types. Otherwise, changing them is a violation of semantic versioning and will break dependant crates:

  • nom_exif::error::Error
  • nom_exif::exif::tags::ExifTag
  • nom_exif::file::FileFormat

In other words, it's a breaking change to support new formats, make new errors, or add to the recognized EXIF tag list! See: https://rust-lang.github.io/rust-clippy/rust-1.81.0/index.html#/exhaustive_enums

To prevent this after the next major version, please consider adding the following to Cargo.toml:

# other stuff...

[lints.clippy]
"exhaustive_enums" = "deny" # all `pub enum`s are `#[non_exhaustive]` 

Thank you for your reminder and suggestions!

I'm currently working on the next major version, and it has a lot of changes, with a fresh new API design, some new file types supports, performance improved, etc.

Although not completely finished, the sync API already works and all test cases pass. The Async API and documentation work will take a while yet.

If you are interested, you can check out this branch.

About the #[non_exhaustive] attribute, I thought about this issue and finally decided not to add this attribute for the time being. Because this attribute will mask and hide the problem. For example, if a new enum variant is added that may be important to users, but users are completely unaware of it, this may lead to some potential, difficult-to-detect errors or problems. Rather than hiding problems, it may be better to raise them, which may be more consistent with Rust's safety philosophy.

In addition:

If you have more other considerations in this regard, please feel free to convince me, thank you!

mindeng commented 2 weeks ago

Here's what the new API looks like:

use nom_exif::*;

fn main() -> Result<()> {
    let mut parser = MediaParser::new();

    // The file can be an image, a video, or an audio.
    let ms = MediaSource::file_path("./testdata/exif.heic")?;
    if ms.has_exif() {
        // Parse the file as an Exif-compatible file
        let mut iter: ExifIter = parser.parse(ms)?;
        let exif: Exif = iter.into();
        assert_eq!(exif.get(ExifTag::Make).unwrap().as_str().unwrap(), "Apple");
    } else if ms.has_track() {
        // Parse the file as a track
    }

    let ms = MediaSource::file_path("./testdata/meta.mov")?;
    if ms.has_track() {
        // Parse the file as a track
        let info: TrackInfo = parser.parse(ms)?;
        assert_eq!(info.get(TrackInfoTag::Make), Some(&"Apple".into()));
        assert_eq!(info.get(TrackInfoTag::Model), Some(&"iPhone X".into()));
        assert_eq!(info.get(TrackInfoTag::GpsIso6709), Some(&"+27.1281+100.2508+000.000/".into()));
        assert_eq!(info.get_gps_info().unwrap().latitude_ref, 'N');
        assert_eq!(
            info.get_gps_info().unwrap().latitude,
            [(27, 1), (7, 1), (68, 100)].into(),
        );
    }

    Ok(())
}
mindeng commented 1 week ago

After rethinking it, I decided to add the #[non_exhaustive] attribute to ExifTag and TrackInfoTag.

Thank you for your reminder and suggestions, which made me rethink and make this decision! And, v2.0.0 has been released!