feat(ci): make rust version in ci configurable

[nimrod-starkware]

This change is 

[nimrod-starkware]

I couldn't find an easy way to set this variable per CI run (i.e for all workflows)..
If this is the right way to do that, we can add it to the other workflows (committer ci for example)

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware)

---

.github/workflows/main.yml line 58 at r1 (raw file):

        with:
          components: rustfmt
          toolchain: nightly-2024-04-29

@dorimedini-starkware, do you know why it is fixed to that version? Is that on purpose?

Code quote:

nightly-2024-04-29

[dorimedini-starkware]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @dan-starkware, @nimrod-starkware, and @TzahiTaub)

a discussion (no related file):

1. share the env-variable logic between CI jobs
2. this env var should select the rust version in all CIs, not just main

---

a discussion (no related file):
please rebase over main-v0.13.2 (this is a CI fix)

---

---

.github/workflows/main.yml line 18 at r1 (raw file):

      - auto_merge_enabled
      - edited
jobs:

newline

Suggestion:

      - edited

jobs:

---

.github/workflows/main.yml line 24 at r1 (raw file):

    - uses: actions/checkout@v4
    - id: version
      run: echo "rust_version=$(cat .github/workflows/rust_version.txt)" >> $GITHUB_OUTPUT

env vars in all caps by convension

Suggestion:

RUST_VERSION

---

.github/workflows/main.yml line 58 at r1 (raw file):

Previously, nimrod-starkware wrote…

@dorimedini-starkware, do you know why it is fixed to that version? Is that on purpose?

no. @dan-starkware any idea? not sure this is from papyrus CI, just from your experience maybe you know

[nimrod-starkware]

Reviewable status: 1 of 2 files reviewed, 5 unresolved discussions (waiting on @dan-starkware, @dorimedini-starkware, and @TzahiTaub)

---

.github/workflows/main.yml line 18 at r1 (raw file):

Previously, dorimedini-starkware wrote…

newline

Done.

---

.github/workflows/main.yml line 24 at r1 (raw file):

Previously, dorimedini-starkware wrote…

env vars in all caps by convension

Done.

[nimrod-starkware]

Reviewable status: 1 of 2 files reviewed, 5 unresolved discussions (waiting on @dan-starkware, @dorimedini-starkware, and @TzahiTaub)

a discussion (no related file):

Previously, dorimedini-starkware wrote…

please rebase over main-v0.13.2 (this is a CI fix)

Done.

---

[dorimedini-starkware]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware, @nimrod-starkware, and @TzahiTaub)

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.214 ms 34.289 ms 34.380 ms]
change: [-4.3133% -2.8383% -1.5049%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
3 (3.00%) high mild
4 (4.00%) high severe

[dorimedini-starkware]

Reviewed 9 of 9 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dan-starkware, @nimrod-starkware, and @TzahiTaub)

---

.github/workflows/papyrus_nightly-tests-call.yml line 36 at r4 (raw file):

      - uses: dtolnay/rust-toolchain@master
        with:
          toolchain: ${{ needs.set_rust_version.outputs.rust_version }}

non-blocking, but if this works - consider it... you have code duplication anyway, maybe cating each time isn't so bad

Suggestion:

jobs:
  GW-integration-test-call:
    runs-on: ${{ inputs.os }}
    steps:
      - uses: actions/checkout@v4
      - uses: dtolnay/rust-toolchain@master
        with:
          toolchain: $(cat .github/workflows/rust_version.txt)

---

.github/workflows/verify-deps.yml line 32 at r4 (raw file):

        run: cargo build --verbose
      - name: Test
        run: cargo test --verbose

looking at this particular job, I believe this one should actually stay stable.
not sure who wrote it and what the intention was, but it looks like it tests that our code works with latest packages - so makes sense to also test if our code works with latest release of rust, even if the rest of the jobs have fixed rust versions

Code quote:

jobs:
  set_rust_version:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v4
    - id: version
      run: echo "RUST_VERSION=$(cat .github/workflows/rust_version.txt)" >> $GITHUB_OUTPUT
    outputs:
      rust_version: ${{ steps.version.outputs.RUST_VERSION }}

  latest_deps:
    needs: set_rust_version
    name: Latest Dependencies
    runs-on: ubuntu-20.04
    continue-on-error: true
    steps:
      - uses: actions/checkout@v4
      - uses: dtolnay/rust-toolchain@master
        with:
          toolchain: ${{ needs.set_rust_version.outputs.rust_version }}
      - name: Update Dependencies
        run: cargo update --verbose
      - name: Build
        run: cargo build --verbose
      - name: Test
        run: cargo test --verbose

[dan-starkware]

.github/workflows/main.yml line 58 at r1 (raw file):

Previously, dorimedini-starkware wrote…

no. @dan-starkware any idea? not sure this is from papyrus CI, just from your experience maybe you know

We didn't want it to break between changes. AFAIR we agreed on default to latest with an easy way to change globally if needed

[dorimedini-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dan-starkware, @nimrod-starkware, and @TzahiTaub)

---

.github/workflows/main.yml line 58 at r1 (raw file):

Previously, dan-starkware wrote…

We didn't want it to break between changes. AFAIR we agreed on default to latest with an easy way to change globally if needed

@nimrod-starkware try changing this to stable and see if CIs pass? this may cause many format issues; but if it works then I think you can and should replace the toolchain here to the one in your rust_version.txt

[TzahiTaub]

Reviewed all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dan-starkware, @dorimedini-starkware, and @nimrod-starkware)

a discussion (no related file):
What exactly is the desired behavior? Do we want the rust version to be determined per commit across all of the CI? Or do we just want the ability to change it globally (i.e., not

---

---

.github/workflows/papyrus_nightly-tests-call.yml line 36 at r4 (raw file):

Previously, dorimedini-starkware wrote…

non-blocking, but if this works - consider it... you have code duplication anyway, maybe cating each time isn't so bad

AFAIU this is not possible - you can't run a bash command out of a run step

[TzahiTaub]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dan-starkware, @dorimedini-starkware, and @nimrod-starkware)

a discussion (no related file):

Previously, dorimedini-starkware wrote…

1. share the env-variable logic between CI jobs
2. this env var should select the rust version in all CIs, not just main

I'm not sure I understand 2. What exactly do we want? If we want the ability to change the version globally for all branches - we can set an env var via the repo settings and read it in all workflows - this is the better option if we don't want the version to be per commit (the limitation is that only the repo owners can set the version). Can this work?

---

[nimrod-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dan-starkware and @dorimedini-starkware)

a discussion (no related file):

Previously, TzahiTaub (Tzahi) wrote…

I'm not sure I understand 2. What exactly do we want? If we want the ability to change the version globally for all branches - we can set an env var via the repo settings and read it in all workflows - this is the better option if we don't want the version to be per commit (the limitation is that only the repo owners can set the version). Can this work?

We do want a version per commit. For example, we would like to fix a rust version for release branches

---

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.390 ms 65.555 ms 65.829 ms]
change: [-10.024% -6.5905% -3.4965%] (p = 0.00 < 0.05)
Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
4 (4.00%) high mild
4 (4.00%) high severe

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [64.987 ms 65.074 ms 65.178 ms]
change: [-10.027% -6.7756% -3.9853%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
1 (1.00%) high mild
6 (6.00%) high severe

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.187 ms 65.294 ms 65.412 ms]
change: [-9.9636% -6.4755% -3.2777%] (p = 0.00 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
4 (4.00%) high mild
2 (2.00%) high severe

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.070 ms 65.128 ms 65.195 ms]
change: [-8.8403% -5.2540% -2.1487%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.012 ms 65.095 ms 65.189 ms]
change: [-3.4296% -2.3763% -1.4691%] (p = 0.00 < 0.05)
Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [64.576 ms 64.654 ms 64.738 ms]
change: [-11.459% -7.4679% -3.7746%] (p = 0.00 < 0.05)
Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
7 (7.00%) high mild
1 (1.00%) high severe

[nimrod-starkware]

Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @dan-starkware and @TzahiTaub)

---

.github/actions/install_rust/action.yml line 15 at r14 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

A suggestion - define an env var here with the toolchain version - to have a single location to change and to avoid the edit lines 15 & 22 guidelines - as changing this file can change the line numbers but it's easy to miss updating the comment.

I tried to do that, but I couldn't find an easy way.

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [64.759 ms 64.839 ms 64.930 ms]
change: [-6.2794% -3.9496% -2.0873%] (p = 0.00 < 0.05)
Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
2 (2.00%) high mild
2 (2.00%) high severe

[dorimedini-starkware]

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dan-starkware and @TzahiTaub)

[dorimedini-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dan-starkware and @TzahiTaub)

[TzahiTaub]

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dan-starkware and @nimrod-starkware)

---

.github/actions/install_rust/action.yml line 15 at r14 (raw file):

Previously, nimrod-starkware wrote…

I tried to do that, but I couldn't find an easy way.

I see, I thought it would be possible to define an env scope but I see it's not possible for composite actions. You can try to define the toolchain in a step above the current two:

steps:
  - name: Define toolchain
    run: echo "DEFAULT_TOOLCHAIN=stable"  >> $GITHUB_ENV  # Will change the env of a caller workflow (overriding this var if it exists)
 - name: install ...
   ...
   with:
     toolchain: ${{ env.DEFAULT_TOOLCHAIN}}

[nimrod-starkware]

Reviewable status: 15 of 16 files reviewed, all discussions resolved (waiting on @dan-starkware, @dorimedini-starkware, and @TzahiTaub)

---

.github/actions/install_rust/action.yml line 15 at r14 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

I see, I thought it would be possible to define an env scope but I see it's not possible for composite actions. You can try to define the toolchain in a step above the current two:

steps:
  - name: Define toolchain
    run: echo "DEFAULT_TOOLCHAIN=stable"  >> $GITHUB_ENV  # Will change the env of a caller workflow (overriding this var if it exists)
 - name: install ...
   ...
   with:
     toolchain: ${{ env.DEFAULT_TOOLCHAIN}}

Done

[TzahiTaub]

Reviewed 1 of 1 files at r16, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dan-starkware)

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [64.427 ms 64.522 ms 64.661 ms]
change: [-7.3721% -4.0514% -1.2582%] (p = 0.00 < 0.05)
Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
3 (3.00%) high mild
2 (2.00%) high severe

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [64.584 ms 64.674 ms 64.777 ms]
change: [-7.0233% -3.7722% -1.0608%] (p = 0.01 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
4 (4.00%) high mild
2 (2.00%) high severe