Skip to content

Commit

Permalink
migrate encoding v1 to sway-features (#6586)
Browse files Browse the repository at this point in the history
## Description

This PR starts the implementation of
https://github.com/FuelLabs/sway-rfcs/blob/master/rfcs/0013-changes-lifecycle.md.

## CLI flags

Now, all `CLI`s, including our tests, support two new arguments

```
...
      --experimental <EXPERIMENTAL>
          Comma separated list of all experimental features that will be enabled [possible values: new_encoding, storage_domains]
      --no-experimental <NO_EXPERIMENTAL>
          Comma separated list of all experimental features that will be disabled [possible values: new_encoding, storage_domains]
...
```

Experimental features can also be enabled inside `Forc.toml` for
workspaces and individual projects.

```
experimental = { encoding-v1 = true }
```

And can be enabled using environment variables:

```
FORC_NO_EXPERIMENTAL=feature_a,feature_b FORC_EXPERIMENTAL=feature_c forc build ...
```

These configurations are applied in a very specific order to allow them
to be overwritten. The order from the weakest to the strongest is:
1 - Workspace `TOML`
2 - Project `TOML`
3 - CLI
4 - Environment variables.

The rationale is:
1 - We want an easy way to enable and/or disable a feature for the whole
workspace;
2 - But we want to allow projects to overwrite these features, when
needed.

These two are the suggested approach to configure experimental features,
but we also want to allow someone to easily turn on or off features to
test something in particular. For that one can use the CLI flags; and in
the specific case one does not have access to the CLI, for example, when
one of the `SDK` is compiling the code, one can still completely
overwrite the `TOML` files using environment variables.

We are also applying the "no experimental" first. This is to allow the
use case of exactly controlling which features will be enabled and
disabled. In this case one can use the "meta-feature" `all` to disable
everything and enable only the desired features. For example:

```
FORC_NO_EXPERIMENTAL=all FORC_EXPERIMENTAL=new_encoding your_command....
```

## Test for "-h"

This PR also introduces snapshot tests for "-h" for all forc commands at
https://github.com/FuelLabs/sway/pull/6586/files#diff-20a5e677d2ae9266a2247739b6a473d2ad0c627955187d668822e7d194f8e940

## Multiple test.toml

This PR also enables the use of multiple `test.toml`. Now each
`test.toml` will be a different test, allowing the same code to be
compiled and asserted differently.

For example, `main_args_empty` has two files: the normal `test.toml` and
a new `test.encoding_v1.toml`. One has `new_encoding` enabled and the
other disabled.

When a `test.toml` file has the `experimental` field, we do not use
fields with `_new_encoding` suffix anymore, making these files simpler:

test.toml:

```toml
category = "run"

# (1336, 1)
script_data = "0000000000000538 0000000000000001"
expected_result = { action = "return", value = 1337 }

validate_abi = true

experimental = { encoding_v1 = false }
```

test.encoding_v1.toml
```
script_data = "0000000000000538 0000000000000001"
expected_result = { action = "return_data", value = "0000000000000539" }

experimental = { new_encoding = true }
```

Test tomls with a suffix use `test.toml` as a base. So we do not need to
repeat every field in them.

Now when we run a test, we will see two tests instead of just one:

```
...
Testing should_pass/language/main_args/main_args_empty (test.encoding_v1.toml) ... ok
Testing should_pass/language/main_args/main_args_empty (test.toml) ... ok
...
```

## Conditional compilation

It is also possible to use conditional compilation using these
experimental features. Examples can be found inside `sway-lib-std`.

```sway
#[cfg(experimental_new_encoding = true)]
pub fn assert_eq<T>(v1: T, v2: T) {
...
}
``` 

## Checklist

- [ ] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [ ] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: IGI-111 <[email protected]>
  • Loading branch information
xunilrj and IGI-111 authored Oct 28, 2024
1 parent 34b70f2 commit ddb505d
Show file tree
Hide file tree
Showing 81 changed files with 1,462 additions and 520 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ jobs:
with:
cache-provider: "buildjet"
- name: Cargo Run E2E Tests (EVM)
run: cargo run --locked --release --bin test -- --target evm --locked --no-encoding-v1
run: cargo run --locked --release --bin test -- --target evm --locked --no-experimental new_encoding

# TODO: Remove this upon merging std tests with the rest of the E2E tests.
cargo-test-lib-std:
Expand Down
18 changes: 18 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ members = [
"sway-ast",
"sway-core",
"sway-error",
"sway-features",
"sway-ir",
"sway-ir/sway-ir-macros",
"sway-lsp",
Expand Down Expand Up @@ -60,6 +61,7 @@ forc-tx = { path = "forc-plugins/forc-tx/", version = "0.66.2" }
sway-ast = { path = "sway-ast/", version = "0.66.2" }
sway-core = { path = "sway-core/", version = "0.66.2" }
sway-error = { path = "sway-error/", version = "0.66.2" }
sway-features = { path = "sway-features/", version = "0.66.2" }
sway-lsp = { path = "sway-lsp/", version = "0.66.2" }
sway-parse = { path = "sway-parse/", version = "0.66.2" }
sway-types = { path = "sway-types/", version = "0.66.2" }
Expand Down Expand Up @@ -164,6 +166,7 @@ libtest-mimic = "0.7"
lsp-types = "0.94"
mdbook = { version = "0.4", default-features = false }
minifier = "0.3"
normalize-path = "0.2.1"
notify = "6.1"
notify-debouncer-mini = "0.4"
num-bigint = "0.4"
Expand Down
6 changes: 2 additions & 4 deletions forc-pkg/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ forc-tracing.workspace = true
forc-util.workspace = true
fuel-abi-types.workspace = true
futures.workspace = true
git2 = { workspace = true, features = [
"vendored-libgit2",
"vendored-openssl",
] }
git2 = { workspace = true, features = ["vendored-libgit2", "vendored-openssl"] }
gix-url = { workspace = true, features = ["serde"] }
hex.workspace = true
ipfs-api-backend-hyper = { workspace = true, features = ["with-builder"] }
Expand All @@ -33,6 +30,7 @@ serde_json.workspace = true
serde_with.workspace = true
sway-core.workspace = true
sway-error.workspace = true
sway-features.workspace = true
sway-types.workspace = true
sway-utils.workspace = true
tar.workspace = true
Expand Down
18 changes: 1 addition & 17 deletions forc-pkg/src/manifest/build_profile.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
use serde::{Deserialize, Serialize};
use sway_core::{OptLevel, PrintAsm, PrintIr};

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, Default)]
#[serde(rename_all = "kebab-case")]
pub struct ExperimentalFlags {
pub new_encoding: bool,
}

/// Parameters to pass through to the `sway_core::BuildConfig` during compilation.
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
#[serde(rename_all = "kebab-case")]
Expand Down Expand Up @@ -39,8 +33,6 @@ pub struct BuildProfile {
pub reverse_results: bool,
#[serde(default)]
pub optimization_level: OptLevel,
#[serde(default)]
pub experimental: ExperimentalFlags,
}

impl BuildProfile {
Expand All @@ -65,9 +57,6 @@ impl BuildProfile {
error_on_warnings: false,
reverse_results: false,
optimization_level: OptLevel::Opt0,
experimental: ExperimentalFlags {
new_encoding: false,
},
}
}

Expand All @@ -88,9 +77,6 @@ impl BuildProfile {
error_on_warnings: false,
reverse_results: false,
optimization_level: OptLevel::Opt1,
experimental: ExperimentalFlags {
new_encoding: false,
},
}
}
}
Expand All @@ -103,10 +89,9 @@ impl Default for BuildProfile {

#[cfg(test)]
mod tests {
use crate::{BuildProfile, PackageManifest};
use sway_core::{OptLevel, PrintAsm, PrintIr};

use crate::{manifest::build_profile::ExperimentalFlags, BuildProfile, PackageManifest};

#[test]
fn test_build_profiles() {
let manifest = PackageManifest::from_dir("./tests/sections").expect("manifest");
Expand Down Expand Up @@ -160,7 +145,6 @@ mod tests {
error_on_warnings: true,
reverse_results: true,
optimization_level: OptLevel::Opt0,
experimental: ExperimentalFlags { new_encoding: true },
};
let profile = build_profiles.get("release").expect("release profile");
assert_eq!(*profile, expected);
Expand Down
2 changes: 2 additions & 0 deletions forc-pkg/src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ pub struct Project {
pub entry: String,
pub implicit_std: Option<bool>,
pub forc_version: Option<semver::Version>,
#[serde(default)]
pub experimental: HashMap<String, bool>,
}

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
Expand Down
Loading

0 comments on commit ddb505d

Please sign in to comment.