Skip to content

Set installed toolchain as default and bump MSRV to 1.77 #294

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wiktor-k
Copy link
Collaborator

The docs for the action indicate that unless specified the toolchain will not be used by default.

@wiktor-k
Copy link
Collaborator Author

Hmm... okay, fixing this uncovered another issue: the bindings are using std::mem::offset_of which, according to docs was introduced in Rust 1.77.

Do you think we should upgrade our MSRV or rather find a way to generate sources without offset_of or something else entirely? 🤔

Apparently it is possible to set Rust target when generating bindings: rust-lang/rust-bindgen#2960 (comment)

CC @Jakuje @hug-dev

@Jakuje
Copy link
Collaborator

Jakuje commented Jun 30, 2025

Hmm... okay, fixing this uncovered another issue: the bindings are using std::mem::offset_of which, according to docs was introduced in Rust 1.77.

Do you think we should upgrade our MSRV or rather find a way to generate sources without offset_of or something else entirely? 🤔

Apparently it is possible to set Rust target when generating bindings: rust-lang/rust-bindgen#2960 (comment)

CC @Jakuje @hug-dev

I do not have much experience or overview what all implications of bumping MSRV are, but given that this was here for quite some time and only now somebody noticed, I would say lets bump it. Sounds like there is really not a huge interest in old rust versions in there.

From technical point of view, do we know what is the alternative generated with bindgen set with rust_target < 1.77? Does it just use the experimental naming of the offset_of? Or does it use some different approach? Does it have some performance/readability/usabliity implications?

The docs for the action indicate that unless specified the toolchain will not be used by default.

Signed-off-by: Wiktor Kwapisiewicz <[email protected]>
@wiktor-k wiktor-k force-pushed the wiktor-k-patch-1 branch from 925468e to d73f78d Compare July 3, 2025 11:29
Signed-off-by: Wiktor Kwapisiewicz <[email protected]>
@wiktor-k wiktor-k force-pushed the wiktor-k-patch-1 branch from d73f78d to 8b1a132 Compare July 3, 2025 11:32
Copy link
Collaborator

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

no concerns here

@wiktor-k
Copy link
Collaborator Author

wiktor-k commented Jul 3, 2025

Thanks 👋 . Let's wait for @hug-dev since he touched these parts previously and may have valuable input.

@wiktor-k wiktor-k changed the title Set installed toolchain as default Set installed toolchain as default and bump MSRV to 1.77 Jul 3, 2025
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.

2 participants