Skip to content

RUST-1268 Correctly report rustc version used for compilation in handshake #622

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

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

kmahar
Copy link
Contributor

@kmahar kmahar commented Apr 13, 2022

RUST-1268

Switches the driver to use rustc_version_runtime to retrieve the rustc version used for compilation for inclusion in the driver handshake metadata.

Previously, on a machine where rustc was available the platforms field looked like this:

"rustc 1.60.0 stable (2022-04-04) with tokio" // stable release
"rustc 1.62.0 nightly (2022-04-11) with tokio" // nightly release

When rustc is not available, this field was null.

Now, it looks like this (slightly different due to the formatting of VersionMeta.short_version_string, but the relevant info is still there and in a similar format):

"rustc 1.60.0 (7737e0b5c 2022-04-04) with tokio" // stable release
"rustc 1.62.0-nightly (90ca44752 2022-04-11) with tokio" // nightly release

It doesn't seem like we test this at all right now, but I did test manually to get the results I pasted above. I don't think a test would have caught the bug this fixes anyway though, since we always have rustc available when we are running tests.

Feel free to check yourselves as well. Note one quirk is that if you switch Rust versions and recompile, it seems rustc_version_runtime doesn't get rebuilt, so you have to cargo clean --package rustc_version_runtime and rebuild to get that updated.

@@ -35,7 +35,7 @@ struct ClientMetadata {
application: Option<AppMetadata>,
driver: DriverMetadata,
os: OsMetadata,
platform: Option<String>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this only seemed to be an option to allow it to be initially set to None in BASE_CLIENT_METADATA below, and then conditionally set if version_check::triple returned a value.
since we can unconditionally set that value now in one line, and the fact that we allowed this value to be None is kind of how this bug existed in the first place, it seems preferable to make it non-optional.

@kmahar kmahar marked this pull request as ready for review April 13, 2022 17:38
@kmahar
Copy link
Contributor Author

kmahar commented Apr 13, 2022

just realized I made the base branch here main rather than 2.2.x, but I think these changes can be cherry-picked cleanly onto that branch, so I'll do that after this gets merged

Copy link
Contributor

@abr-egn abr-egn left a comment

Choose a reason for hiding this comment

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

LGTM!

@kmahar kmahar merged commit 81d9d5e into mongodb:main Apr 13, 2022
@kmahar kmahar deleted the RUST-1268/metadata-platforms-field branch April 13, 2022 18:57
kmahar added a commit to kmahar/mongo-rust-driver that referenced this pull request Apr 13, 2022
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.

4 participants