v2.3: Remove conditional feature as it is not supported (backport of #6577)#6607
v2.3: Remove conditional feature as it is not supported (backport of #6577)#6607mergify[bot] wants to merge 2 commits intov2.3from
Conversation
* remove condition as it is not supported * remove useless whitespace to fix sort (cherry picked from commit 51c33c2) # Conflicts: # svm/Cargo.toml
|
Cherry-pick of 51c33c2 has failed: 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 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v2.3 #6607 +/- ##
=========================================
- Coverage 82.8% 82.8% -0.1%
=========================================
Files 848 848
Lines 379542 379542
=========================================
- Hits 314575 314524 -51
- Misses 64967 65018 +51 🚀 New features to boost your workflow:
|
| solana-sysvar-id = { workspace = true } | ||
| solana-timings = { workspace = true } | ||
| solana-transaction-context = { workspace = true } | ||
| solana-transaction-context = { workspace = true, features = ["debug-signature"] } |
There was a problem hiding this comment.
the dependency should be redeclared with the feature flag in [dev-dependencies], not added to prod [dependencies]
There was a problem hiding this comment.
Crate publishing fails if the feature is not added to [dependencies]
There was a problem hiding this comment.
something else is wrong then. this should never be used in prod
There was a problem hiding this comment.
p sure this should be gated by the cargo feature, not debug_assertions.
agave/svm/src/transaction_processor.rs
Lines 863 to 864 in 3281f61
don't see any other usage of the tx-ctx symbols behind that feature
There was a problem hiding this comment.
honestly deleting the Signature bs might be the way to go? don't see it touched anywhere
There was a problem hiding this comment.
But yes, can be deleted if it causes too much trouble with dependencies.
There was a problem hiding this comment.
sure. saw that. was more concerned that it's not actually read anywhere in the codebase other than implicitly through PartialEq, but don't actually see an instance of (a sanely named instantiation of) TransactionContext equality comparison in a debug_assert
But yes, can be deleted if it causes too much trouble with dependencies.
|
Closing this as the code has been removed by #6647 |
Problem
Adding dependencies based on configuration values is currently not supported. Cargo generates an error when trying to build this crate (see #6440 (comment))
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies
The same applies to cfg(debug_assertions), cfg(test) and cfg(proc_macro). These values will not work as expected and will always have the default value returned by rustc --print=cfg. There is currently no way to add dependencies based on these configuration values.Summary of Changes
Remove condition and add feature directly to dependency.
Fixes #
This is an automatic backport of pull request #6577 done by [Mergify](https://mergify.com).