Skip to content

v2.2: move version pinning to binary crates for sdk deps (backport of #5594)#5783

Merged
t-nelson merged 1 commit intov2.2from
mergify/bp/v2.2/pr-5594
Apr 12, 2025
Merged

v2.2: move version pinning to binary crates for sdk deps (backport of #5594)#5783
t-nelson merged 1 commit intov2.2from
mergify/bp/v2.2/pr-5594

Conversation

@mergify
Copy link
Copy Markdown

@mergify mergify Bot commented Apr 12, 2025

Problem

version pinning in the monorepo restricts dependency options for the remaining public api crates that reside there

eg. try to use solana-client = "2.2.1" (monorepo) with solana-transaction = "2.2.2" (sdk)

Summary of Changes

move all version pinning from workspace to bin crates for sdk dependencies


This is an automatic backport of pull request #5594 done by Mergify.

@mergify mergify Bot requested a review from a team as a code owner April 12, 2025 00:41
@mergify mergify Bot added the conflicts label Apr 12, 2025
@mergify
Copy link
Copy Markdown
Author

mergify Bot commented Apr 12, 2025

Cherry-pick of 0a0cce2 has failed:

On branch mergify/bp/v2.2/pr-5594
Your branch is up to date with 'origin/v2.2'.

You are currently cherry-picking commit 0a0cce2ba.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   bench-tps/Cargo.toml
	modified:   cli/Cargo.toml
	modified:   dos/Cargo.toml
	modified:   faucet/Cargo.toml
	modified:   install/Cargo.toml
	modified:   keygen/Cargo.toml
	modified:   ledger-tool/Cargo.toml
	modified:   log-analyzer/Cargo.toml
	modified:   net-shaper/Cargo.toml
	modified:   net-utils/Cargo.toml
	modified:   platform-tools-sdk/cargo-build-sbf/Cargo.toml
	modified:   platform-tools-sdk/cargo-test-sbf/Cargo.toml
	modified:   stake-accounts/Cargo.toml
	modified:   test-validator/Cargo.toml
	modified:   tokens/Cargo.toml
	modified:   validator/Cargo.toml
	modified:   watchtower/Cargo.toml
	modified:   zk-keygen/Cargo.toml

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   Cargo.toml
	both modified:   genesis/Cargo.toml
	both modified:   gossip/Cargo.toml

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@t-nelson t-nelson force-pushed the mergify/bp/v2.2/pr-5594 branch from 81eac0c to 3e3af8a Compare April 12, 2025 02:10
@t-nelson
Copy link
Copy Markdown

@mergify rebase

@mergify
Copy link
Copy Markdown
Author

mergify Bot commented Apr 12, 2025

rebase

☑️ Nothing to do

Details
  • -conflict [📌 rebase requirement]
  • -closed [📌 rebase requirement]
  • queue-position = -1 [📌 rebase requirement]
  • any of:
    • #commits-behind > 0 [📌 rebase requirement]
    • #commits > 1 [📌 rebase requirement]
    • -linear-history [📌 rebase requirement]

* move version pinning to binary crates for sdk deps

* address review

(cherry picked from commit 0a0cce2)
@t-nelson t-nelson force-pushed the mergify/bp/v2.2/pr-5594 branch from 3e3af8a to f3a51e9 Compare April 12, 2025 02:53
@t-nelson t-nelson requested a review from joncinque April 12, 2025 02:54
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.4%. Comparing base (9b4adb4) to head (f3a51e9).

Additional details and impacted files
@@            Coverage Diff            @@
##             v2.2    #5783     +/-   ##
=========================================
- Coverage    83.4%    83.4%   -0.1%     
=========================================
  Files         806      806             
  Lines      373256   373256             
=========================================
- Hits       311351   311342      -9     
- Misses      61905    61914      +9     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread Cargo.toml
solana-ed25519-program = "=2.2.2"
solana-ed25519-program = "2.2.2"
solana-entry = { path = "entry", version = "=2.2.9" }
solana-feature-set-interface = "=4.0.0"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This should probably get relaxed too

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i skipped this one on purpose. was going to bp #5549 instead

Comment thread test-validator/Cargo.toml
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I didn't catch this last time, but this crate isn't actually a binary crate. the solana-test-validator binary is built from the validator crate, so we likely don't need to pin here

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ah ok. let's unwind that in master and bp separately

Comment thread install/Cargo.toml
solana-message = "=2.2.1"
solana-pubkey = { version = "=2.2.1", default-features = false }
solana-rpc-client = { workspace = true }
solana-sha256-hasher = { workspace = true }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I missed this last time, but this crate comes from the SDK and should be pinned

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

yeah, the wip robot noticed too 😅

These crates have the wrong version in '/scratch/code/solana/solana/install/Cargo.toml':
    solana-sha256-hasher: has 'workspace' but expected '=2.2.1'

will fix this one in master and bp separately as well

Copy link
Copy Markdown

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Sounds like everything I brought up will get addressed separately, so :shipit:

@t-nelson t-nelson merged commit 52fbd17 into v2.2 Apr 12, 2025
46 checks passed
@t-nelson t-nelson deleted the mergify/bp/v2.2/pr-5594 branch April 12, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants