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

fix(compute/rust) handling of 'cargo version' output #1197

Merged
merged 1 commit into from
May 8, 2024

Conversation

kpfleming
Copy link
Contributor

'cargo version' emits the version string to stdout, so the code should capture only stdout, and parse it for checking the ToolchainConstraint. Capturing stderr and including it in the parsing process can result in bogus version numbers being found and reported (as is happening right now when Rust 1.78 is used in a project directory where .cargo/config exists).

'cargo version' emits the version string to stdout, so the code should
capture only stdout, and parse it for checking the
ToolchainConstraint. Capturing stderr and including it in the parsing
process can result in bogus version numbers being found and reported
(as is happening right now when Rust 1.78 is used in a project
directory where .cargo/config exists).
@Integralist Integralist changed the title CDTOOL-890: Improve handling of 'cargo version' output Improve handling of 'cargo version' output May 7, 2024
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

LGTM. But can we manually test this change with different Rust/cargo versions to be sure there are no unexpected issues.

@kpfleming
Copy link
Contributor Author

I'll be testing it with 1.78 (the version causing the problem now) and 1.56 (the oldest version we claim to support).

@kpfleming
Copy link
Contributor Author

Tests complete:

  1. With Rust 1.78.0, the bogus warning about "version 1.38 didn't meet the constraint" is gone (that version number only appears on stderr, not stdout).
  2. With Rust 1.56.0, the constraint-checking code correctly reports that 1.56.0 doesn't meet the constraint.

@kpfleming kpfleming added the bug Something isn't working label May 7, 2024
@Integralist Integralist changed the title Improve handling of 'cargo version' output fix(compute/rust) handling of 'cargo version' output May 8, 2024
@Integralist Integralist merged commit da16dfa into fastly:main May 8, 2024
6 checks passed
@kpfleming kpfleming deleted the CDTOOL-890 branch May 8, 2024 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants