Skip to content

Rust: re-enable dedicated CI job with toolchain 1.85.1#3417

Closed
santiagomed wants to merge 4 commits intoapache:masterfrom
santiagomed:fix/rust-ci-reenable
Closed

Rust: re-enable dedicated CI job with toolchain 1.85.1#3417
santiagomed wants to merge 4 commits intoapache:masterfrom
santiagomed:fix/rust-ci-reenable

Conversation

@santiagomed
Copy link
Copy Markdown
Contributor

@santiagomed santiagomed commented Apr 25, 2026

Summary

  • re-enable the dedicated Rust CI job
  • bump the Rust CI toolchain from 1.83.0 to 1.85.1
  • update the repo rust-toolchain pin to 1.85.1

Why

The dedicated Rust CI job was disabled, and the previous 1.83.0 toolchain no longer resolves the current dependency graph cleanly. This restores a working Rust validation path.

Follow-up work in #3414 depends on having dedicated Rust CI healthy again, but GitHub cannot stack that upstream PR directly on a fork-only base branch. Review these PRs together, with this one first.

Validation

  • cargo +1.85 test --manifest-path lib/rs/Cargo.toml --locked
  • cargo +1.85 test --manifest-path lib/rs/test/Cargo.toml
  • cargo +1.85 test --manifest-path test/rs/Cargo.toml

@mergeable mergeable Bot added the github_actions Pull requests that update GitHub Actions code label Apr 25, 2026
@santiagomed
Copy link
Copy Markdown
Contributor Author

Follow-up codegen change: #3414. Please review this Rust CI fix first.

@kpumuk
Copy link
Copy Markdown
Member

kpumuk commented Apr 25, 2026

Could you please check e4666f2 on how to enable cross tests as well for Rust, this change only enables unit tests

@santiagomed santiagomed requested a review from mhlakhani as a code owner April 25, 2026 19:29
@mergeable mergeable Bot added rust build and general CI cmake, automake and build system changes labels Apr 25, 2026
@santiagomed santiagomed force-pushed the fix/rust-ci-reenable branch from 69d0f5e to d91566b Compare April 25, 2026 19:30
@santiagomed
Copy link
Copy Markdown
Contributor Author

@kpumuk you're right I had missed the commit, fixed.

@santiagomed santiagomed requested a review from emmenlau as a code owner April 25, 2026 21:21
@mergeable mergeable Bot added nodejs java Pull requests that update Java code labels Apr 25, 2026
@kpumuk
Copy link
Copy Markdown
Member

kpumuk commented Apr 27, 2026

Last changes go in weird directions :-) Java formatting is fixed in master, not quite sure what Node.js are for...

@santiagomed santiagomed force-pushed the fix/rust-ci-reenable branch from 89db116 to 40c36c4 Compare April 28, 2026 01:07
@santiagomed
Copy link
Copy Markdown
Contributor Author

santiagomed commented Apr 28, 2026

@kpumuk Thanks, removed the Java file. The Node.js commits fix cross-test failures that show up when Rust CI is re-enabled: the Node.js binary/compact protocol decoders were rejecting raw UUID bytes from the Rust client, and the test harness needed nyc and a fix for expected server exits. Without those the Rust cross-tests fail against the Node.js server.

@santiagomed
Copy link
Copy Markdown
Contributor Author

Does anyone know any way I can avoid waiting for a workflow approval for this PR? It would help me iterate faster. At xAI we need #3414 to land ASAP since it is causing issues.

@kpumuk
Copy link
Copy Markdown
Member

kpumuk commented Apr 28, 2026

The easiest option to avoid waiting it to allow the pipeline to run on your fork and iterate there :)

@santiagomed santiagomed force-pushed the fix/rust-ci-reenable branch 3 times, most recently from 69167c3 to 6c64e0e Compare April 28, 2026 03:33
@mergeable mergeable Bot added the testsuite label Apr 28, 2026
@santiagomed santiagomed force-pushed the fix/rust-ci-reenable branch 2 times, most recently from 3f1d38a to 251fa92 Compare April 28, 2026 05:02
@santiagomed santiagomed force-pushed the fix/rust-ci-reenable branch from 251fa92 to 0b61008 Compare April 28, 2026 05:33
@santiagomed
Copy link
Copy Markdown
Contributor Author

Can I get a workflow approval? I think CI should pass now santiagomed#1

Copy link
Copy Markdown
Member

@kpumuk kpumuk left a comment

Choose a reason for hiding this comment

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

Could you please remove unrelated changes to node.js files (commit 671dd71) and squash commits?

The pull request is ready to go after that

Comment thread lib/nodejs/lib/thrift/compact_protocol.js Outdated
The uuid npm package v13 validates that stringified UUIDs conform
to RFC 4122 (version nibble in [1-8], variant nibble in [89ab]).
The previous test value 00010203-0405-0607-0809-0a0b0c0d0e0f has
version=0 and variant=0, causing stringify() to throw TypeError
on the Node.js server side. Use the same UUID as the Go cross-test
client (00112233-4455-6677-8899-aabbccddeeff) which passes validation.
Client: rs
@santiagomed santiagomed force-pushed the fix/rust-ci-reenable branch from 0b61008 to 61cf80f Compare April 28, 2026 19:42
@santiagomed
Copy link
Copy Markdown
Contributor Author

@Jens-G looks like I need another approval. Can you take a look please?

@kpumuk kpumuk closed this in 3ac576c Apr 28, 2026
@kpumuk kpumuk removed java Pull requests that update Java code nodejs labels Apr 28, 2026
sveneld pushed a commit to sveneld/thrift that referenced this pull request May 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build and general CI cmake, automake and build system changes github_actions Pull requests that update GitHub Actions code rust testsuite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants