Revert #1831 and #1812#1850
Merged
Merged
Conversation
This reverts commit c80fb1b.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reverts the previously-added behavior that allowed experimental_spec_shaking_v2 builds to silently fall back to spec shaking v1 when the build-system env var wasn’t set. It aligns the SDK with the spec-shaking-v2 rollout plan by making the env-var/tooling requirement enforced (on wasm builds) and removing the associated fallback tests/vectors.
Changes:
- Remove “no env var => fallback to v1” behavior (and its tests/vectors) for spec shaking v2.
- Gate spec-shaking-v2 codepaths directly on the
experimental_spec_shaking_v2feature (rather than a customcfg(spec_shaking_v2)set bybuild.rs). - Update CI/Makefile/test expansions to always provide
SOROBAN_SDK_BUILD_SYSTEM_SUPPORTS_SPEC_SHAKING_V2=1during wasm/test-vector builds.
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/spec_shaking_v2/src/test.rs | Removes the “no env var fallback to v1” WASM vector and test. |
| tests-expanded/test_workspace_lib_tests.rs | Updates expanded output to include SpecShakingMarker impls under v2 behavior. |
| tests-expanded/test_udt_tests.rs | Updates expanded output to include SpecShakingMarker impls / propagation. |
| tests-expanded/test_spec_shaking_v2_tests.rs | Updates expanded output and removes embedded “no env fallback” test/bytes. |
| tests-expanded/test_spec_shaking_v1_tests.rs | Updates expanded output (host-test expansion reflects v2-default dev-dep behavior). |
| tests-expanded/test_spec_lib_tests.rs | Updates expanded output to include marker impls for spec-bearing types. |
| tests-expanded/test_events_tests.rs | Updates expanded output to include marker impl + publish-time marker call. |
| tests-expanded/test_events_ref_tests.rs | Same as above for ref-event variant. |
| tests-expanded/test_errors_tests.rs | Updates expanded output to include marker impls for error/int-enum types. |
| tests-expanded/test_contracttrait_trait_tests.rs | Updates expanded output to include marker impls for contract trait types. |
| tests-expanded/test_constructor_tests.rs | Updates expanded output to include marker impls for constructor-related types. |
| tests-expanded/test_bn254_tests.rs | Updates expanded output to include marker propagation for BN254-related types. |
| tests-expanded/test_bls_tests.rs | Updates expanded output to include marker propagation for BLS-related types. |
| tests-expanded/test_auth_tests.rs | Updates expanded output to include marker impls for auth test error types. |
| tests-expanded/test_account_tests.rs | Updates expanded output to include marker impls for account test error types. |
| soroban-sdk/src/try_from_val_for_contract_fn.rs | Switches spec-shaking gating from cfg(spec_shaking_v2) to the feature flag. |
| soroban-sdk/src/lib.rs | Switches spec-shaking meta/module exports to feature gating; comment updated to mention stellar-cli. |
| soroban-sdk/src/into_val_for_contract_fn.rs | Switches spec-shaking gating from cfg(spec_shaking_v2) to the feature flag. |
| soroban-sdk/src/_features.rs | Updates feature docs to reflect enforced build requirements (no fallback). |
| soroban-sdk/build.rs | Enforces env-var requirement by panicking on wasm builds when the feature is enabled but env var is missing. |
| soroban-sdk-macros/src/lib.rs | Removes env-var-based enablement check; uses feature gating to decide export behavior. |
| soroban-sdk-macros/src/derive_struct_tuple.rs | Removes old helper usage; gates marker impl generation on the feature. |
| soroban-sdk-macros/src/derive_struct.rs | Same as above for named structs. |
| soroban-sdk-macros/src/derive_event.rs | Gates event marker impl/call generation on the feature (no env-var check). |
| soroban-sdk-macros/src/derive_error_enum_int.rs | Gates marker impl generation on the feature. |
| soroban-sdk-macros/src/derive_enum_int.rs | Gates marker impl generation on the feature. |
| soroban-sdk-macros/src/derive_enum.rs | Gates marker impl generation on the feature. |
| Makefile | Removes the extra “build without env var” step and keeps env var set for wasm test builds. |
| .github/workflows/rust.yml | Sets SOROBAN_SDK_BUILD_SYSTEM_SUPPORTS_SPEC_SHAKING_V2=1 globally for CI jobs. |
This reverts commit 440c453.
mootz12
approved these changes
Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Reverts #1831 and #1812.
This is a plain git revert, plus then rerunning the expand-tests make target.
Why
Close #1845