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

Add --locked flag for rust action #349

Closed
wants to merge 1 commit into from

Conversation

chenrui333
Copy link
Contributor

Include --locked flag to the rust action to make it the build reproducible.

@ethomson
Copy link
Contributor

I'm not familiar enough with Rust to be able to weigh in on this. @rylev can you help here with a review?

@chenrui333
Copy link
Contributor Author

@ethomson @rylev homebrew-core side has a better well-written issue might give you more context and impact on this change :)
Homebrew/homebrew-core#46025

@rylev
Copy link

rylev commented Feb 19, 2020

This seems reasonable to me for builds on CI, but I'm wondering what @pietroalbini thinks about this.

@pietroalbini
Copy link

I don't think adding --locked to the default template is a good idea, as it would make the template useless for ~50.7% of the Rust repositories here on GitHub.

The --locked flag will not only error out if the lockfile is not up to date, but also if Cargo.lock is missing. According to the Rust project's scraping efforts, of the 117465 Rust repositories with a Cargo.toml we found across the website only 57819 of them also have a Cargo.lock.

This is because the project recommends to commit the lockfile only for binaries, not for libraries. This recommendation is also encoded in cargo new --lib, which puts Cargo.lock in the .gitignore.

@ethomson
Copy link
Contributor

Thanks for the information @pietroalbini!

@chenrui333 maybe instead of adding --locked we should add a comment in the workflow file that instead indicates that users could add --locked, when this would be beneficial, and what the benefits would be?

@chenrui333
Copy link
Contributor Author

@pietroalbini I am actually surprised that most of the rust projects do not have lock file, that sounds pretty interesting. But thanks for the info.

@ethomson Sounds good, I will modify my PR to add as comment to raise the awareness. Ideally the CI build should be reproducible anywhere. :)

@chenrui333
Copy link
Contributor Author

The locked file is like package-lock.json in npm (yarn.lock in yarn) world or go.sum in go modules.

@github-actions
Copy link

This pull request has become stale and will be closed automatically within a period of time. Sorry about that.

@chenrui333
Copy link
Contributor Author

looks like we indeed run into the --locked flag issue in the homebrew side, see discussions in Homebrew/brew#10205

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants