Update all Cargo.toml files to use workspace dependency versions #6501

alfiedotwtf · 2024-09-05T07:11:04Z

Description

This PR relates to #6179 which aims to simplify individual workspace member dependency versioning by deferring upstream to the workspace dependencies itself.

I've also extended #6179 as it currently only relates to external dependencies - I've noticed that whenever Sway has a version bump, the diffs manually update all of the many Cargo.toml files to the new globally shared version. We can also use workspaces here to defer each Cargo.toml package version to the workspace package version as well.

This PR is in 3 parts:

Update all Cargo.toml external dependencies to use the shared versions defined in the workspace dependencies
Update all Cargo.toml package version to defer to the workspace package version.
Update all external dependencies to their latest versions

If an individual workspace member needs to break away from the shared workspace package version, we can still do that:

Update the individual Cargo.toml to the new version (as normal)
Update all references within the whole repository to state this new version

Notes:

[dev-dependencies] was not factored out since [workspace.dev-dependencies] is currently not supported by Cargo.toml, meaning all dev dependencies would have to be brought into [workspace.dependencies] which I assume is not what we would want to do.
[target.'cfg(not(target_os = "macos"))'.dependencies] was not factored out since these crates should only be brought in if the condition is met, but again as the only way to do this would be to move it to [workspace.dependencies], I would advise to keep them separated.

Checklist

[✅] I have linked to any relevant issues.
[✅] I have commented my code, particularly in hard-to-understand areas.
[␆] I have updated the documentation where relevant (API docs, the reference, and the Sway book).
- [␆] If my change requires substantial documentation changes, I have requested support from the DevRel team
[␆] I have added tests that prove my fix is effective or that my feature works.
[␆] I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
[✅] I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
[✅] I have requested a review from the relevant team or maintainers.

codspeed-hq · 2024-09-05T07:20:19Z

CodSpeed Performance Report

Merging #6501 will degrade performances by 17.31%

_{Comparing alfie/6179-cargo-workspace-version (4957517) with master (a073a95)}

Summary

⚡ 1 improvements
❌ 1 regressions
✅ 20 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	`master`	`alfie/6179-cargo-workspace-version`	Change
⚡	`code_lens`	6.1 µs	5.5 µs	+11.09%
❌	`document_symbol`	4.3 ms	5.2 ms	-17.31%

.github/workflows/ci.yml

Voxelot · 2024-09-06T18:02:24Z

nice 😎

sdankel

Looks good, just a few comments.

Cargo.toml

sdankel · 2024-09-06T22:21:01Z

Closes #5845

JoshuaBatty · 2024-09-08T23:45:07Z

Awesome to see this. so much cleaner.

- Support `act` in ci.yml for local testing - Add documentation for `act` testing - Add `act` binary to .gitignore - Version bump differences - Test fixtures - Minor changes for errors and typos

JoshuaBatty · 2024-09-16T23:27:08Z

sway-core/src/semantic_analysis/type_check_context.rs

@@ -1201,6 +1201,7 @@ impl<'a> TypeCheckContext<'a> {
            }

            if !maybe_method_decl_refs.is_empty() {
+                #[allow(clippy::mutable_key_type)]


Maybe we shouldn't enforce this clippy lint since we need to disable it in quite a few places...

JoshuaBatty · 2024-09-16T23:31:14Z

sway-lsp/Cargo.toml

@@ -65,8 +68,8 @@ pretty_assertions = "1.4.0"
 rand = "0.8"
 regex = "^1.10.2"
 sway-lsp-test-utils = { path = "tests/utils" }
-tikv-jemallocator = "0.5"
-tower = { version = "0.4.12", default-features = false, features = ["util"] }
+tikv-jemallocator = "0.6.0"


I personally prefer using "x.y" instead of "x.y.z" for dependency versions, as it automatically incorporates bug fixes and security updates within the x.y.* range. What are your thoughts about adopting this approach?

JoshuaBatty

Nice work Alfie, just a couple of small nits.

sdankel · 2024-09-17T00:44:40Z

.github/workflows/ci.yml

@@ -423,32 +426,40 @@ jobs:
      contents: write
    steps:
      - uses: actions/checkout@v3
-      - name: Install toolchain
+      - name: Install Rust toolchain
+        if: ${{ !env.ACT }}


nit: I'd prefer to do the act-related stuff in a separate PR since it's not related to updating the dependencies

sway-utils/src/performance.rs

Cargo.toml

sdankel · 2024-09-17T01:02:06Z

Cargo.toml

+bytes = "1.3.0"
+chrono = { version = "0.4", default-features = false }
+cid = "0.11"
+clap = "4.5.4"


Cargo.toml

sdankel · 2024-09-17T01:03:41Z

Cargo.toml

+num-bigint = "0.4.3"
+num-traits = "0.2.16"
+object = "0.36.4"
+once_cell = "1.18.0"


sdankel · 2024-09-17T01:03:50Z

Cargo.toml

+object = "0.36.4"
+once_cell = "1.18.0"
+opener = "0.7.2"
+parking_lot = "0.12.1"


sdankel · 2024-09-17T01:04:28Z

Cargo.toml

+serde_ignored = "0.1.9"
+serde_json = "1.0.91"
+serde_with = "3.3.0"
+serde_yaml = "0.9.27"


latest is "0.9.34"

Yeah i think we should just be setting it to use 0.9 and leave the minor version part off. Same everywhere really.

sdankel · 2024-09-17T01:04:40Z

Cargo.toml

+tempfile = "3"
+term-table = "1.3"
+termion = "4.0.2"
+textwrap = "0.16.0"


Cargo.toml

sway-lsp/tests/lib.rs

sdankel

Looks good overall, just a few nits. I highlighted some other dependencies that have newer versions since it looks like you're aiming to upgrade everything in this PR now.

kayagokalp

Looks like i forgot to post my review 🤦 anyways it seems like the generated bytecode for the proxy contract changed after this one. I wan't to take a closer look but it is not nice (also not sure how this is possible) that dependency updates are causing bytecode changes in the compiler.

Do we know why?

kayagokalp · 2024-09-17T00:11:00Z

forc-plugins/forc-client/tests/deploy.rs

@@ -341,7 +341,7 @@ async fn test_simple_deploy() {
    node.kill().unwrap();
    let expected = vec![DeployedContract {
        id: ContractId::from_str(
-            "50fe882cbef5f3da6da82509a66b7e5e0a64a40d70164861c01c908a332198ae",


Do we know why these are changed?

I've turned this PR back into draft, and am pulling out all the changes so that we can see which crates can be updated with no code changes, and which ones end up doing changes like these.

Unfortunately it's a bit time consuming because you end up having to wait for the whole ci.yml to finish before the next 'edit -> test -> debug` cycle

I finally found the cause! Bumping rustc-hash from 1.1 to 2.0 changes the bytecode. I guess it makes sense given the name of the crate, but dang it's not reproducible.

Oh... from the 1.3.0 changelog:

This replaces the previous "fxhash" algorithm originating in Firefox with a custom hasher

I'm going to have a good guess that the old and new hashers don't produce the same output. Ah, yep... all the fixtures for test_hash!() changed between the algo switch.

## Description For #6179, PR #6501 kept bumping into errors as it was doing too many things, so I've split that PR into multiple PRs. This is the first, and the only thing it does is move all of the various `Cargo.toml` dependencies into the single workspace `Cargo.toml`. Future PRs will: - Update dependency versions - Update code that breaks from the version bumps ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.

## Description This is a continuation of #6179, and follows on from PR #6501. These are broken into separate PRs so that we can discuss each PR `HEAD` separately so that the PRs that can get merged, do so quickly. Otherwise, the further along they get stale the more work they all need in order to keep up to `master` without being hard to find which change introduced a possible broken build. This PR only does a version bump to every crate dependency that does not need any code changes to apply cleanly. Following this PR will be a new PR, where every commit updates a single crate along with the files that were needed to be changed in order to work again. The crates that will need code changes are: - tokio 1.12: -> 1.40 - rustc-hash: 1.1 -> 2.0 - miden-core: 0.3 -> 0.10 - notify: 5.0 -> 6.1 - notify-debouncer-mini: 0.2 -> 0.4 - revm: 2.3 -> 14.0 - syn: 1.0 -> 2.0 - tikv-jemallocator: 0.5 -> 0.6 - toml_edit: 0.21 -> 0.22 There should also be a commit to upgrade Rust itself to >= `1.81.0`. ... and for another day, there's 3 crates that will take a bit of time to get working, and so will go under a different issue #6536: - annotate-snippets - lsp-types - uint ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. Co-authored-by: Joshua Batty <joshpbatty@gmail.com>

alfiedotwtf force-pushed the alfie/6179-cargo-workspace-version branch 2 times, most recently from ba0e9d3 to ff3c935 Compare September 6, 2024 11:08

alfiedotwtf marked this pull request as ready for review September 6, 2024 11:35

alfiedotwtf requested review from a team as code owners September 6, 2024 11:35

sdankel reviewed Sep 6, 2024

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

sdankel reviewed Sep 6, 2024

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

Cargo.toml Outdated Show resolved Hide resolved

Cargo.toml Show resolved Hide resolved

Cargo.toml Show resolved Hide resolved

alfiedotwtf force-pushed the alfie/6179-cargo-workspace-version branch from ff3c935 to cb684bf Compare September 9, 2024 07:25

alfiedotwtf marked this pull request as draft September 10, 2024 08:42

alfiedotwtf force-pushed the alfie/6179-cargo-workspace-version branch 3 times, most recently from 771d4ad to 0df0880 Compare September 12, 2024 06:43

alfiedotwtf self-assigned this Sep 12, 2024

alfiedotwtf added compiler General compiler. Should eventually become more specific as the issue is triaged code quality dependencies labels Sep 12, 2024

alfiedotwtf mentioned this pull request Sep 12, 2024

Version bump workspace crates to newer versions #6536

Open

alfiedotwtf force-pushed the alfie/6179-cargo-workspace-version branch 8 times, most recently from 4692b0e to 52b4570 Compare September 13, 2024 19:06

Fix rebase issues and support act in CI (#6179)

8c73032

- Support `act` in ci.yml for local testing - Add documentation for `act` testing - Add `act` binary to .gitignore - Version bump differences - Test fixtures - Minor changes for errors and typos

alfiedotwtf force-pushed the alfie/6179-cargo-workspace-version branch from 6780e76 to 8c73032 Compare September 16, 2024 09:28

JoshuaBatty reviewed Sep 16, 2024

View reviewed changes

JoshuaBatty requested a review from a team September 16, 2024 23:32