johannesvollmer / exrs

100% Safe Rust OpenEXR file library
Other
154 stars 24 forks source link

Update Cargo.toml dependencies and release 1.73.0 #240

Closed JamyGolden closed 1 month ago

JamyGolden commented 2 months ago

@johannesvollmer tagging you since no reviewers were added to the PR automatically

johannesvollmer commented 2 months ago

hey, thanks for your time! unfortunately, some of you suggestions should be discussed:

JamyGolden commented 2 months ago

Hey :)

The real issue I want to fix is actually just minz_oxide and change that to version ~0.8.0 to sort out dependency resolution issues, but before we get to that:

I think the Cargo.toml should probably be using the ~ prefixes instead of ^ for 0.x.x (zero major version) dependencies because the minor version number is for both for new features/non-breaking changes and breaking changes. So having ^ prefix on a semver 0 major version package is potentially very unstable.

Are you sure exrs currently builds on anything under rust 1.70.0? I get this when I build exrs master branch using 1.67.1:

🦄 …/exrs on 🌱 master[] via  v1.67.1 ( 01:10)
❯ git log -n 1
commit e018e0dba9a5a1cba3a5ddbe7a05577638fbc9e2 (HEAD -> master, upstream/master)
Merge: 861f0cf d626147
Author: Johannes Vollmer <32042925+johannesvollmer@users.noreply.github.com>
Date:   Sun Jul 7 18:38:45 2024 +0200

    Merge pull request #236 from johannesvollmer/iterate-all-attributes

    add helper to iterate all attributes in a header

🦄 …/exrs on 🌱 master[] via  v1.67.1 ( 01:10)
❯ git status
On branch master
nothing to commit, working tree clean

🦄 …/exrs on 🌱 master[] via  v1.67.1 ( 01:10)
❯ rustc --version
rustc 1.67.1 (d5a82bbd2 2023-02-07)

🦄 …/exrs on 🌱 master[] via  v1.67.1 ( 01:10)
❯ cargo build --release
error: package `half v2.4.1` cannot be built because it requires rustc 1.70 or newer, while the currently active rustc version is 1.67.1
Either upgrade to rustc 1.70 or newer, or use
cargo update -p half@2.4.1 --precise ver
where `ver` is the latest version of `half` supporting rustc 1.67.1

I'm not sure why half is resolving to 2.4.1 since it's set as 2.1.0 in Cargo.toml

johannesvollmer commented 2 months ago

Oh sorry, I just forgot that the ^ on a 0.x.x version doesn't do the same thing as on normal versions. Sorry for my misunderstanding.

Are you sure exrs currently builds on anything under rust 1.70.0? I get this when I build exrs master branch using 1.67.1:

Our CI has a test for this, but it seems that gave me a false sense of security: it only runs for PRs, but not regularly, so any dependency breaking the build might have gone unnoticed.

johannesvollmer commented 2 months ago

We did previously constrain the version of half because we wanted to avoid requiring rust 1.70.

But that was undone recently in https://github.com/johannesvollmer/exrs/commit/aba88fb2eea6d304d14610d169ebb8132ca6d740 in pr https://github.com/johannesvollmer/exrs/pull/231

It seems we decided to default to rust 1.70 but allowing users to opt-in to the old version of half manually, if desired. Maybe that would resolve the build error for 1.67?

johannesvollmer commented 2 months ago

I think the Cargo.toml should probably be using the ~ prefixes instead of ^ for 0.x.x (zero major version) dependencies because the minor version number is for both for new features/non-breaking changes and breaking changes. So having ^ prefix on a semver 0 major version package is potentially very unstable.

Agree, but not sure if a strict limit would be good. I will look into it.

johannesvollmer commented 1 month ago

I apologize for my confusion, my mind has other topics in focus, and this project is only in the background currently, and for a long time now. from my understanding, this is the current state:

JamyGolden commented 1 month ago

No problem at all with any async responses :smile: Thanks for the reply!

johannesvollmer commented 1 month ago

Awesome, thanks for the update. Does this new version still solve the version conflict in your project?

JamyGolden commented 1 month ago

Yeah it does thanks :smile: miniz_oxide was the issue.