Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

chore: set resolver v2 at the workspace level #1731

Conversation

bishopcheckmate
Copy link
Contributor

@bishopcheckmate bishopcheckmate commented Jan 13, 2024

Description

Cargo defaults to resolver = "2" in edition 2021 only for singular crates. Workspace tho defaults to the old resolver = "1" and will override this setting for all crates in the workspace. The guideline is to manually set the resolver = "2" on workspace level.
This was documented recently and since a few releases produces a warning on building. You don't see it due to the rust-toolchain setting.

Type of change

  • Bug fix (non-breaking change which fixes an issue) (not really a bugfix but that's the closest match)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Contents

  • specifies resolver = "2" on workspace level
  • removes resolver = "2" from light-client-poc/Cargo.toml because it was ignored anyway

This also fixes this warning:

warning: resolver for the non root package will be ignored, specify resolver at the workspace root:
package:   /data/bishop/zkevm-circuits/light-client-poc/Cargo.toml
workspace: /data/bishop/zkevm-circuits/Cargo.toml

@github-actions github-actions bot added the crate-integration-tests Issues related to the integration-tests workspace member label Jan 15, 2024
@bishopcheckmate
Copy link
Contributor Author

I pushed the commit which should fix the builds. With resolver = 2, selected features are not propagated through different dependency types and targets so ethers-solc had to be explicitly enabled in build deps

Copy link
Collaborator

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the fix.

Copy link
Collaborator

@han0110 han0110 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ChihChengLiang ChihChengLiang added this pull request to the merge queue Jan 17, 2024
Merged via the queue into privacy-scaling-explorations:main with commit d5fe917 Jan 17, 2024
13 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-integration-tests Issues related to the integration-tests workspace member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants