Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build!: bump rust to 1.76 and add rust update script #1693

Open
wants to merge 5 commits into
base: v1.0-dev
Choose a base branch
from

Conversation

lklimek
Copy link
Contributor

@lklimek lklimek commented Feb 9, 2024

Issue being fixed or feature implemented

New Rust was released

What was done?

  • bumped version to 1.76 in github actions, rust-toolchain.toml, Dockerfile, Cargo.toml files and README.md
  • added script that makes such updates easy

How Has This Been Tested?

Run script locally + Github Actions

Breaking Changes

New Rust version 1.76 is required

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@lklimek lklimek changed the title build(deps)!: bump rust to 1.76 and add rust update script build!: bump rust to 1.76 and add rust update script Feb 9, 2024
# The same as in /README.md
default: "stable"
# Update using scripts/update-rust-toolchain.sh
default: "1.76"
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it's using toolchain file if not set?

@@ -17,7 +17,6 @@ inputs:
description: Enable Rust cache
required: false
default: "true"

Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove new lines? I see it more readable rather then on block of test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what yq tool did that I use to replace version in yaml.

I can try to refactor to use multi-line sed if you wish.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, you dynamically update it...

Copy link
Member

Choose a reason for hiding this comment

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

Maybe just read it from the toolchain file instead of hardcoding?

Suggestion from ChatGPT:

jobs:
  build:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2
      # Checkout your repository

    - name: Extract Rust toolchain version
      id: rust_toolchain
      run: |
        TOOLCHAIN_VERSION=$(grep channel rust-toolchain.toml | awk '{print $3}' | tr -d '"')
        echo "TOOLCHAIN_VERSION=$TOOLCHAIN_VERSION" >> $GITHUB_ENV
        echo "::set-output name=version::$TOOLCHAIN_VERSION"
      # This command reads the rust-toolchain.toml, extracts the version, and sets it as an environment variable and step output

    # Use the extracted version in subsequent steps
    - name: Use Rust toolchain
      uses: actions-rs/toolchain@v1
      with:
        toolchain: ${{ steps.rust_toolchain.outputs.version }}
        override: true

I would use $GITHUB_OUTPUT though

@@ -10,7 +10,8 @@ authors = [
"Ivan Shumkov <shumkov@dash.org>",
]
edition = "2021"
rust-version = "1.73"
# Update using scripts/update-rust-toolchain.sh
rust-version = "1.76"
Copy link
Member

Choose a reason for hiding this comment

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

Why we set it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand the question.

The rust-version field is an optional key that tells cargo what version of the Rust language and compiler your package can be compiled with.

As we only test on 1.76, we only support this version onwards.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I've never seen such an option, and since it's a minimal value then it's all good.

@@ -0,0 +1,20 @@
use std::fmt::Display;
Copy link
Member

Choose a reason for hiding this comment

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

Is this by mistake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, good point, missed this.

Will remove.

@@ -17,7 +17,6 @@ inputs:
description: Enable Rust cache
required: false
default: "true"

Copy link
Member

Choose a reason for hiding this comment

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

Maybe just read it from the toolchain file instead of hardcoding?

Suggestion from ChatGPT:

jobs:
  build:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2
      # Checkout your repository

    - name: Extract Rust toolchain version
      id: rust_toolchain
      run: |
        TOOLCHAIN_VERSION=$(grep channel rust-toolchain.toml | awk '{print $3}' | tr -d '"')
        echo "TOOLCHAIN_VERSION=$TOOLCHAIN_VERSION" >> $GITHUB_ENV
        echo "::set-output name=version::$TOOLCHAIN_VERSION"
      # This command reads the rust-toolchain.toml, extracts the version, and sets it as an environment variable and step output

    # Use the extracted version in subsequent steps
    - name: Use Rust toolchain
      uses: actions-rs/toolchain@v1
      with:
        toolchain: ${{ steps.rust_toolchain.outputs.version }}
        override: true

I would use $GITHUB_OUTPUT though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants