Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update glam dependency to allow 0.24.1 #124

Closed
wants to merge 1 commit into from

Conversation

TomiS
Copy link

@TomiS TomiS commented Aug 18, 2023

Trying to allow using latest version of glam to make it possible to use opencascade-rs with, for example, the latest [email protected]

At least examples seem to run correctly, but it's quite possible simple-game also needs updating to avoid duplicate dependency.

changes

changes

changes

revert some changes
@bschwind
Copy link
Owner

bschwind commented Aug 19, 2023

@strohel I'm curious about your opinion on this one as well. glam is a pretty fundamental library to everything I'm using, but its API mostly doesn't change (we're basically dealing with floats, after all).

I also don't want to re-export glam because that will just lead to more conflicts when other projects use other versions of glam for other reasons.

Should I go back to simple-game and all the crates here and specify an even older allowed glam? I'm tempted to say "anything greater than 0.20" or something to that effect, but it's discouraged in the cargo recommendations:

Avoid overly broad version requirements. For example, >=2.0.0 can pull in any SemVer-incompatible version, like version 5.0.0, which can result in broken builds in the future.

From what I can tell, the bet is determining which of the two happen more often:

  • glam publishes a new, SemVer-incompatible version which breaks the build for opencascade-rs

or

  • glam publishes a new, SemVer-incompatible version which doesn't break the build for opencascade-rs, but breaks it for projects depending on opencascade-rs, due to too-strict dependency rules.

Which do we think will happen more often in the future? And have I described the overall scenarios correctly?

@TomiS
Copy link
Author

TomiS commented Aug 20, 2023

Yeah, deciding how to notate the version requirement was a bit problematic. 0.x indicates there are no stable releases and things might still change unexpectedly. That's why I ended using the range which admittedly will be a pain to maintain.

If the glam API is stable in practice, ideally they'd release 1.0.0 and people could then rely on ^1.0.0 notation.

One approach is to just allow anything and then adjust if there are problems. Or go with something like >=0.2, <2.0, which relies on assumption that a new major (2.0) after first major (1.0) must have a breaking change.

Just my 2 cents.

@bschwind
Copy link
Owner

I don't think it will be too much of a pain to maintain this as it is, I just wanted to explore all the options out there.

In practice this should be simple to update as I don't imagine we'll be using the more advanced features of glam.

@TomiS do you think I should apply the same version range to simple-game?

@TomiS
Copy link
Author

TomiS commented Aug 20, 2023

@bschwind Probably. But you might want to validate somehow that everything works as expected.

@bschwind
Copy link
Owner

I played around with this a bit:

  • I updated Cargo.toml in simple-game to be:
    glam = { version = ">=0.20, <=0.24.1", features = ["bytemuck"] }
  • I updated all other glam dependencies in this repo to the same exact version requirements, except the examples crate, which has
    glam = { version = "0.20", features = ["bytemuck"] }

I end up getting type errors as you would see with multiple versions of the same crate in the graph. I thought it would select 0.20 as the singular version in the lockfile, but that wasn't the case. If I specify exactly 0.24 in the examples crate then it works.

I think this explanation describes what is happening.

Here's another related issue

I'll try to play around with this more to find the most sensible version range we should set.

@bschwind
Copy link
Owner

I have updated glam in #154, along with wgpu and other crates.

@TomiS I'm not sure if this resolves your original reason for opening this PR, but I'll close it for now and you can re-open if you want to add a more lenient version range for glam.

@bschwind bschwind closed this Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants