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

Run cargo update before attempting to build Rust compute packages #179

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

phamann
Copy link
Member

@phamann phamann commented Dec 17, 2020

TL;DR

Invokes cargo update before attempting to build Rust based compute packages via fastly compute build. This is considered "safe" as cargo update will only update dependencies within their semver constraint ranges. However, does go against our philosophy to not mutate the users external state implicitly.

Why?

We currently use the semver version of the fastly-sys Rust crate to indicate major and minor changes to the underlying C@E ABI, rather than the typical meaning of Rust-breaking changes in the package itself. This has come back to bite us, as even if we keep the fastly-sys API itself stable, it depends on other packages that need to advance in major version increments, which requires a major version increment on fastly-sys in turn. We've seen when upgrading versions the fastly-sys gets stuck on an older version and causes rustc compilation error messages for the user, which can be confusing and misleading.

Therefore, we've decided to force an update of dependencies (and their transient deps) via cargo update, as a temporary solution, until we can externally manage the min/max versions of fastly-sys via a CLI configuration API.

@phamann phamann merged commit 8154752 into master Dec 17, 2020
@phamann phamann deleted the phamann/rust-cargo-update branch December 17, 2020 18:33
@phamann phamann added the bug Something isn't working label Dec 18, 2020
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.

2 participants