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

feat: upgrade packages #355

Merged
merged 16 commits into from
Mar 29, 2024
Merged

Conversation

nelson137
Copy link
Contributor

@nelson137 nelson137 commented Mar 24, 2024

Changes

  • Update deny.toml -- remove deprecated keys & opt-in to new behavior (see relevant cargo-deny change)
    • Add MPL-2.0 to allowed licenses
  • Upgrade packages:
    • async-channel: 1.7, 1.8, 1.9 to 2.2
    • async-io: 1.9 to 2.3
    • bevy: 0.11 to 0.12
    • bevy_egui: 0.22 to 0.24
    • bevy_prototype_lyon: 0.9 to 0.10
    • egui, egui_plot: 0.23 to 0.24
    • event-listener: 3.1 to 4.0
    • erased-serde: 0.3.31 to 0.4
    • futures-lite: 1.12, 1.13 to 2.3
    • ggrs: 0.9.4 to 0.10.1
    • mdns-sd: 0.7 to 0.10
    • mio: 0.8.10 to 0.8.11
    • noise: 0.8 to 0.9
    • rcgen: 0.10, 0.11 to 0.12
    • shlex: 1.2 to 1.3

I avoided making the same upgrade as #334 but these PRs cause a merge conflict in Cargo.lock. Whichever is merged second should delete the lock file and rebuild.

@zicklag
Copy link
Member

zicklag commented Mar 25, 2024

Thanks for this! ❤️ @MaxCWhitehead I'll let you merge this so you can make sure that it doesn't mess anything that you're doing up.

@MaxCWhitehead
Copy link
Collaborator

MaxCWhitehead commented Mar 26, 2024

it looks like we might need to add MPL-2.0 to our allowed licenses here (I haven't reviewed what this license is yet but I assume it's probably ok).

Also seeing some duplicate crates of different versions in dependency tree after running CI:
https://github.com/fishfolk/bones/actions/runs/8430987184/job/23127392844?pr=355#step:4:377

We might be able to fix this by looking at dep tree there, but not always easy / possible. May be ok, but one gotcha I've hit before with this is that if there are static / global variables (like a message channel or something) that we use in our code, we end up with two separate instances, which can lead to bugs.

I suspect we may need to do some code fixups to jumpy once we upgrade bones there after merging this - just something to be call out, not worried about it.

Thanks for the work here @nelson137!

@MaxCWhitehead MaxCWhitehead self-requested a review March 26, 2024 22:25
@zicklag
Copy link
Member

zicklag commented Mar 26, 2024

Yeah, MPL should be fine according to my past review of it. The rule is essentially that you share modifications to the MPL licensed code, which is fine, since we're not modifying the libraries that we use. I think Jumpy already has the MPL allowance in it's deny.toml, too.

@nelson137
Copy link
Contributor Author

I added MPL-2.0 and fixed the advisories that had solutions.. but that leaves 2 errors on the (now-deprecated) failure crate.

@MaxCWhitehead
Copy link
Collaborator

@nelson137 looks great - those failures are already existing.

It seems like bitfield-rle updated to remove failure, and ggrs updated to upgrade bitfield-rle, so a upgrade to ggrs should remove those. Happy to merge as is (give me the go ahead) otherwise if you want to try throwing in updating ggrs we might actually get green CI for first time in a while :)

@nelson137
Copy link
Contributor Author

Bitfield has switched to anyhow, but they have yet to publish a new version (and it doesn't look like they will soon). I updated ggrs anyway but I think we're stuck with these for now. It might be worth adding an ignore to the deny.toml:

[advisories]
ignore = ["RUSTSEC-2019-0036", "RUSTSEC-2020-0036"]

@nelson137
Copy link
Contributor Author

@MaxCWhitehead I've made all the upgrade I could, as suggested by dependabot. If you agree about the ignores then I'll push that up; otherwise it's ready to merge.

@MaxCWhitehead
Copy link
Collaborator

Oh sorry I did not catch it wasn't released. Sounds good!

auto-merge was automatically disabled March 28, 2024 13:44

Head branch was pushed to by a user without write access

@zicklag
Copy link
Member

zicklag commented Mar 28, 2024

Bitfield has switched to anyhow, but they have yet to publish a new version (datrs/bitfield-rle#31). I updated ggrs anyway but I think we're stuck with these for now. It might be worth adding an ignore to the deny.toml:

I'd rather leave the warning in there for now. I'm going to leave another comment on bitfield to see if we can prompt a response, but if that doesn't work, we may just want to publish a fork at some point. But I guess we'd have to ask ggrs to use our fork, so it'd really be easiest if they can publish it.

@nelson137
Copy link
Contributor Author

@zicklag I was looking at bitfield-rle, it's a very small library. ggrs only uses encode and decode (source), so it might even make sense for them to just absorb those functions and drop the dependency.

Anyway, I'll revert that change to deny.toml and then we can finally merge this 😅

@MaxCWhitehead
Copy link
Collaborator

MaxCWhitehead commented Mar 28, 2024

Sounds good re: prompting bitfield-rle. If that doesn't go anywhere, I think seeing if GGRS would absorb those is a great idea / maybe we could test those changes on a fork of GGRS + PR it upstream. I've moved multiple deps to a personal fork at times, sometimes upstream takes PR and sometimes we remain on fork indefinitely lol (not suggesting I think we're hit that with GGRS at all)

@zicklag you opened an issue asking bitfield-rle to release in November 2022, I don't know what the process is but wonder if we should push the envelope and see if can have the cargo deny registry or however that works evaluate if flagging bitfield-rle as unmaintained is appropriate.

@nelson137 kicked of CI again but I think on last run we saw a new clippy failure in wasm - haven't had a chance to look at details but perhaps a side effect of async-channel update (https://github.com/fishfolk/bones/actions/runs/8462694019/job/23184928326).

We're close haha updating deps is a bit of a whack-a-mole game it seems.

@MaxCWhitehead
Copy link
Collaborator

It looks like bitfield-rle actually did remove this crate and then bump version to 0.2.0, @nelson137 indicated that on crates.io the dep on failure is still there, it looks to me like perhaps the push to crates.io did not occur on right version?

0.2.0 on git looks good - but not on crates.io

@MaxCWhitehead MaxCWhitehead added this pull request to the merge queue Mar 29, 2024
Merged via the queue into fishfolk:main with commit a3ed353 Mar 29, 2024
9 of 10 checks passed
@nelson137 nelson137 deleted the feat/upgrade-packages branch March 29, 2024 03:02
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.

None yet

3 participants