linebender / resvg

An SVG rendering library.
Apache License 2.0
2.84k stars 229 forks source link

Update fontdb #800

Closed jcdickinson closed 3 months ago

jcdickinson commented 3 months ago

Please update fontdb.

Your project PR template declares:

Pull requests that include [...] dependencies updates [...] will not be accepted.

https://github.com/jcdickinson/resvg/tree/update-fontdb-rustybuzz updates both fontdb and rustybuzz, and fixes relevant compilation errors (tests seem to pass).

RazrFalcon commented 3 months ago

Do you expect a resvg release as well?

drupol commented 3 months ago

Yes please.

RazrFalcon commented 3 months ago

Maybe on weekends.

drupol commented 3 months ago

Thank you in advance, I believe many projects are awaiting that release.

jcdickinson commented 3 months ago

Thanks for resolving this, your time is greatly appreciated.

May I suggest using cargo versioning to decrease the burden you are facing @RazrFalcon ? Cargo uses a slightly different versioning scheme to semantic: for major=0 releases everything is basically shifted to the right by one. Changes in 0.x indicate incompatible changes, changes in 0.*.x indicate compatible improvements. This unfortunate behavior is described here: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html

This compatibility convention is different from SemVer in the way it treats versions before 1.0.0. While SemVer says there is no compatibility before 1.0.0, Cargo considers 0.x.y to be compatible with 0.x.z, where y ≥ z and x > 0.

RazrFalcon commented 2 months ago

I don't see how it would help since we still have a Cargo.lock

laurmaedje commented 2 months ago

The Cargo.lock only applies to the project itself though, not to any projects that depend on it. So if there's a nested dependency like fontdb that made a patch release, it's easy to pull it in with cargo update -p fontdb, but if the nested dependency made a minor release, all the dependencies in between need to be updated.

RazrFalcon commented 2 months ago

Are you sure? The whole reason we have Cargo.lock is to make sure everyone are using tested dependencies.

laurmaedje commented 2 months ago

I'm sure, yeah. For binaries, Cargo.lock indeed gives tested dependencies, but for libraries not. The reason, from my understanding, is that if Cargo.lock files of dependencies were used, many projects would get incompatible and duplicate dependencies. E.g. if I work on project A and depend on B and C, and B's lockfile says time=0.3.41 while C's lockfile says time=0.3.42, then I would already have time twice in the binary and couldn't pass time types from B to C (would be very bad with serde for instance). So as long as the dependencies specify compatible versions (i.e. for 0.x patch level and for x.y with x>0 minor level), Rust will unify them.

RazrFalcon commented 2 months ago

Got it. Will think about it removing it then.

My biggest issue is that when I'm testing resvg on CI I want it to use the same dependency versions as I have locally, otherwise there would be too many random, unrelated breaks.

laurmaedje commented 2 months ago

My biggest issue is that when I'm testing resvg on CI I want it to use the same dependency versions as I have locally, otherwise there would be too many random, unrelated breaks.

Yeah, I think keeping it is generally advisable for this reason, both for libraries and binaries. (The cargo team shifted their opinion and cargo init's defaults from binary-only to both).