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

Set minimum x11-dl version to include Z #484

Merged
merged 1 commit into from
Apr 26, 2018

Conversation

branan
Copy link
Contributor

@branan branan commented Apr 26, 2018

Without this pin, an existing cargo.lock for an older winit will not
update the x11-dl dependency, and thus will select a version that is
missing required new XIM features.

Without this pin, an existing cargo.lock for an older winit will not
update the x11-dl dependency, and thus will select a version that is
missing required new XIM features.
@francesca64
Copy link
Member

Thanks! This is definitely a good idea. However, shouldn't it be =2.17.5? It looks like cargo normally ignores the patch number completely: rust-lang/cargo#1418

When I tested this locally, it wouldn't pull in the new version. Constraining with = made it work. You should double check what I'm saying, though.

@daggerbot in the future, could you please follow semver more closely? Cargo seems to make assumptions wrt semver.

@branan
Copy link
Contributor Author

branan commented Apr 26, 2018

That issue is related to Cargo pulling in a newer version of a package when you specify an older one, which is the documented behavior. It doesn't totally ignore Zs, it just treats them as a minimum. I don't see any reason to hard-pin to a specific Z here - we just want to be sure that winit is on a new enough version to include the bits it needs.

In my local testing this behaved correctly. I just rolled back to my pre-winit-update commit and tried the process again, to be sure nothing else I did accidentally fixed it

  • Update my app to point to local winit with this patch
  • Update my local vulkano to point to winit with this patch
  • Build app - fails to compile due to the Closed/CloseRequested change
  • Check Cargo.toml - It does contain the updated x11-dl as expected \o/

@francesca64
Copy link
Member

Alright, I tried again, and it works as expected. I'm not sure what I was doing wrong the first time.

@francesca64 francesca64 merged commit 7510b95 into rust-windowing:master Apr 26, 2018
@branan
Copy link
Contributor Author

branan commented Apr 26, 2018

Thanks! Please let me know when this fix is released so I can update vulkano-rs/vulkano#953

@francesca64
Copy link
Member

Do you want a release now? While I'll feel slightly embarrassed to need to release 0.13.1 a day after 0.13.0, it seems like the best thing to do.

@branan
Copy link
Contributor Author

branan commented Apr 26, 2018

I'm not in any rush, personally. I've got my local environment sorted.

That said, I think it's good to keep the ecosystem as up-to-date as possible, and I think 0.13.1 is necessary for that. Adding 0.13.0 dependencies to other crates is just going to cause trouble for people. It's up to you how urgent that is for your release schedule, though ¯\_(ツ)_/¯

@francesca64 francesca64 mentioned this pull request Apr 26, 2018
@francesca64
Copy link
Member

Well, I want to get winit 0.13 in as many hands as possible, so here we go: #486

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants