Skip to content

feat(ci): Add tasks for checking and updating Rust lock file (Cargo.lock); Add check to GH workflow. #1448

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.

Sign up for GitHub

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

Merged
LinZhihao-723 merged 21 commits into from
Oct 21, 2025
Merged

feat(ci): Add tasks for checking and updating Rust lock file (Cargo.lock); Add check to GH workflow. #1448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
306bbea
Initial commit.
LinZhihao-723 Sep 30, 2025
15cd106
Update dependency versions to the latest.
LinZhihao-723 Sep 30, 2025
7fa8871
Follow Rust 2018 format.
LinZhihao-723 Sep 30, 2025
77f5bc0
Fix client.
LinZhihao-723 Oct 19, 2025
e7be8c2
Minor fix.
LinZhihao-723 Oct 19, 2025
438ed6e
Merge main.
LinZhihao-723 Oct 19, 2025
317bad8
Merge branch 'main' into s3-client-fix
LinZhihao-723 Oct 19, 2025
48fef4e
Update cargo lock.
LinZhihao-723 Oct 20, 2025
71f4948
Add Rust lock check.
LinZhihao-723 Oct 20, 2025
bd79171
Merge branch 's3-client-fix' into rust-deps-check
LinZhihao-723 Oct 20, 2025
8754cf9
Update workflow.
LinZhihao-723 Oct 20, 2025
723ec56
Fix yaml indentation.
LinZhihao-723 Oct 20, 2025
ff1a813
Apply code review comments.
LinZhihao-723 Oct 20, 2025
ee8050f
Apply code review comments.
LinZhihao-723 Oct 20, 2025
df1a529
Merge branch 'main' into rust-deps-check
LinZhihao-723 Oct 20, 2025
9bb5948
Merge branch 'main' into rust-deps-check
LinZhihao-723 Oct 21, 2025
3c8d9e8
Update Cargo.lock.
LinZhihao-723 Oct 21, 2025
e47b216
Update .github/workflows/clp-rust-checks.yaml
LinZhihao-723 Oct 21, 2025
3674742
Update .github/workflows/clp-rust-checks.yaml
LinZhihao-723 Oct 21, 2025
c6785a5
Merge branch 'main' into rust-deps-check
LinZhihao-723 Oct 21, 2025
d1519fd
Merge branch 'main' into rust-deps-check
LinZhihao-723 Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/clp-rust-checks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ jobs:
shell: "bash"
run: "npm install -g @go-task/cli@3.44.0"

- name: "Validate lock files"
shell: "bash"
run: "task deps:lock:check-rust"

- name: "Lint"
shell: "bash"
run: "task lint:check-rust"
Expand Down
4 changes: 2 additions & 2 deletions Cargo.lock
View file
Open in desktop

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

30 changes: 30 additions & 0 deletions taskfiles/deps/lock.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
version: "3"

includes:
toolchains: "../toolchains.yaml"

tasks:
check-rust:
cmds:
- task: "cargo-workspace-update"
vars:
CARGO_UPDATE_FLAGS: "--locked"

update-rust:
cmds:
- task: "cargo-workspace-update"
vars:
CARGO_UPDATE_FLAGS: ""

# Runs `cargo update` in the root Rust workspace directory with the specified flags.
#
# @param {string} CARGO_UPDATE_FLAGS The flags to pass to the `cargo update` command.
cargo-workspace-update:
internal: true
requires:
vars: ["CARGO_UPDATE_FLAGS"]
dir: "{{.ROOT_DIR}}"
deps: ["toolchains:rust"]
cmd: |-
Comment thread
LinZhihao-723 marked this conversation as resolved.
. "$HOME/.cargo/env"

@junhaoliao junhaoliao Oct 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just curious, do we have plans to install rust into clp/build/ instead of the user's home directory?

@LinZhihao-723 LinZhihao-723 Oct 21, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'd prefer to install to the user's home directory:

  • This would only affect developers who need to build our package from source.
  • Cargo is designed to be per-user installation. If the toolchain is already installed, it would only help you to upgrade (which shouldn't be harmful as I never heard of anyone having problems for sticking to a particular cargo version).
  • This potentially avoids a lot of toolchain problems (like version conflicts) as there're only one toolchain needed and located in a default path.

@kirkrodrigues kirkrodrigues Oct 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, we discussed this when @LinZhihao-723 was initially adding the task. I do think it's worth documenting somewhere though. Did we add it to the package build docs?

@LinZhihao-723 LinZhihao-723 Oct 21, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No we haven't. I will do it in a separate PR.

@LinZhihao-723 LinZhihao-723 Oct 21, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@coderabbitai Please create an issue for us to document new Rust components in CLP package. In this doc we should mention our installation approach for cargo toolchain.

@coderabbitai coderabbitai Bot Oct 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create GitHub issue for documenting Rust components in CLP package

gh issue create \
  --repo y-scope/clp \
  --title "Document Rust components and cargo toolchain installation approach" \
  --body "## Background

As part of adding Rust components to the CLP package, we need to document the new Rust components and our installation approach for the cargo toolchain.

## Documentation Requirements

- Document the new Rust components in the CLP package
- Explain the installation approach for the cargo toolchain
  - Specifically, that cargo is installed to the user's home directory rather than \`clp/build/\`
  - Rationale for this approach (per-user installation, default path, avoiding version conflicts)

## References

- PR: https://github.com/y-scope/clp/pull/1448
- Discussion: https://github.com/y-scope/clp/pull/1448#discussion_r2449502274

Requested by: @LinZhihao-723" \
  --assignee LinZhihao-723

Length of output: 42

@LinZhihao-723, I've created issue #1461 to track the documentation of Rust components and the cargo toolchain installation approach. The issue has been assigned to you and includes references to this PR and discussion.

🔗 #1461

cargo update {{.CARGO_UPDATE_FLAGS}}
1 change: 1 addition & 0 deletions taskfiles/deps/main.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
version: "3"

includes:
lock: "./lock.yaml"
utils: "./utils.yaml"
yscope-dev-utils: "../../tools/yscope-dev-utils/exports/taskfiles/utils/utils.yaml"

Expand Down
Loading