solana
solana copied to clipboard
Update clap to v3.1.5 for all the repository
Problem
Most of the targets use clap v2.33, we want to upgrade to v3.1.5
Proposed Solution
Prepare several PRs to address the problem:
- https://github.com/solana-labs/solana/pull/23555
- https://github.com/solana-labs/solana/pull/23556
- https://github.com/solana-labs/solana/pull/24096
- https://github.com/solana-labs/solana/pull/24479
- https://github.com/solana-labs/solana/pull/24594
- Other changes are in the https://github.com/solana-labs/solana/pull/23864. The idea is to split this PR into several smaller PR (see discussion below)
Problem with (2):
- When building all the downstream projects (
./scripts/build-downstream-projects.sh), fails with errors inside serum/dex related to clap. - This happens because serum uses unstable version
3.0.0-beta.1. Hence, a greater minor version picked up (3.1.6). This happens because assumption is that API is stable within one major version, this assumption is broken in this case because serum uses unstable beta version - Just changing cargo file on serum side leads to problem with conflicting packages version (with
quote).
Action items:
- Open issue on the serum side with explanation: https://github.com/project-serum/serum-dex/issues/221
- Try to fix it quickly
- if this attempts fails, propose a PR with clap-related changes and ask serum to do the rest cargo work
Vim commands to update some clap calls:
%s/Arg::with_name/Arg::new/g
%s/value_t!(matches, /matches.value_of_t(/g
%s/value_t_or_exit!(matches, /matches.value_of_t_or_exit(/g
%s/value_t!(matches, /matches.value_of_t(/g
%s/short("\([^']*\)")/short('\1')/g
%s/.validator(\([^']*\))/.validator(|s| \1(s))/g
%s/.multiple(true)/.multiple_occurrences(true).multiple_values(true)/g
%s/.setting(AppSettings::DisableVersion)/.disable_version_flag(true)/g
%s/.hidden(true)/.hide(true)/g
Hey have you considered how to handle the helper functions in the clap-utils crate? Examples include input_validators::is_parsable() and input_parsers::pubkey_of(). We can't simply flip these over to use clap 3 as that would break existing downstream users of the clap 2 functions.
One possible approach is to create a solana_clap_utils::v3 module for the v3 versions and leave the v2 versions as is
Hey have you considered how to handle the helper functions in the clap-utils crate? Examples include
input_validators::is_parsable()andinput_parsers::pubkey_of(). We can't simply flip these over to use clap 3 as that would break existing downstream users of the clap 2 functions.One possible approach is to create a
solana_clap_utils::v3module for the v3 versions and leave the v2 versions as is
By downstream users, do you mean those mentioned in build-downstream-projects.sh or there are some which are probably not known to us and we cannot break them?
With clap-utils it is hard by itself to do the upgrade because many other tools within our repo depends on it and some clap's API changes are very particular. It mean that it might be a rather big PR.
Having two versions of clap in one repo is probably possible but I found it difficult due to usage of attributes. Haven't figured out yet how to overcome these difficulties.
By downstream users, do you mean those mentioned in build-downstream-projects.sh or there are some which are probably not known to us and we cannot break them?
That's a start but there are other cli users of solana-clap-utils that aren't in that minimal ci script. I played with it a little and I think a new solana-clap-utils-v3 crate (clap-utils-v3/ in the monorepo) that's literally just a duplicate of what's in clap-utils/ but modified for clapv3 is the way to go. This allows downstream users to opt into clapv3 at the time of their choosing, and we're under no obligation to build more features into clap-utils/ as we add them into clap-utils-v3/ going forward
By downstream users, do you mean those mentioned in build-downstream-projects.sh or there are some which are probably not known to us and we cannot break them?
That's a start but there are other cli users of
solana-clap-utilsthat aren't in that minimal ci script. I played with it a little and I think a newsolana-clap-utils-v3crate (clap-utils-v3/in the monorepo) that's literally just a duplicate of what's inclap-utils/but modified for clapv3 is the way to go. This allows downstream users to opt into clapv3 at the time of their choosing, and we're under no obligation to build more features intoclap-utils/as we add them intoclap-utils-v3/going forward
To verify that I understood it correctly:
- add
clap-utils-v3which will coexist withclap-utils(clap v2) - all the crates inside solana repo will eventually depend on
clap-utils-v3 - external crates might continue using
clap-utilsand migrate toclap-utils-v3when they are ready to do so. Probably, a good idea is to add a notice about this in the crate.
On the operational side I like this idea because this will also allow to have smaller PRs
This PR which migrates the heaviest part of the code base was automatically closed: https://github.com/solana-labs/solana/pull/24594#issuecomment-1179810171 For the case we want to come back to this migration later
clap v2.34.0 ├── solana-clap-utils v1.14.5 () ├── solana-client v1.14.5 () └── solana-faucet v1.14.5 (*)
clap v3.2.22 └── solana-net-utils v1.14.5 (*)
clap v2.34.0 ├── solana-clap-utils v1.14.5 () ├── solana-client v1.14.5 () └── solana-faucet v1.14.5 (*)
clap v3.2.22 └── solana-net-utils v1.14.5 (*)
Sorry, but I don't quite understand what do you want to say by this message
clap v2.34.0 ├── solana-clap-utils v1.14.5 () ├── solana-client v1.14.5 () └── solana-faucet v1.14.5 () clap v3.2.22 └── solana-net-utils v1.14.5 ()
Sorry, but I don't quite understand what do you want to say by this message
I wanted to point out that there still clap v2 dependencies which leads to several clap versions in the dependency tree if one relies on those Solana packages.
@gitmalong that's a known thing, migration from clap v2 to v3 turned out to be too risky and was postponed
Check: can we avoid panicking by switching to try_get_matches?
another relevant suggestion: https://discord.com/channels/428295358100013066/439194979856809985/1085599192732401746
I think it is still active, @samkim-crypto is doing this migration
This repository is no longer in use. Please re-open this issue in the agave repo: https://github.com/anza-xyz/agave