Skip to content

chore: git-include Cargo.lock#39

Open
JohnTitor wants to merge 1 commit intorust-netlink:mainfrom
JohnTitor:cargo-lock
Open

chore: git-include Cargo.lock#39
JohnTitor wants to merge 1 commit intorust-netlink:mainfrom
JohnTitor:cargo-lock

Conversation

@JohnTitor
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes Cargo.lock from the .gitignore file and commits the generated lock file to the repository. The reviewer advises against this change, explaining that library crates should generally ignore the lock file to ensure compatibility testing against the latest dependency versions in continuous integration, as recommended by the Cargo Book.

Comment thread .gitignore
@@ -1,4 +1,3 @@
Cargo.lock
target
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Committing Cargo.lock is generally discouraged for library crates. According to the Cargo Book, libraries should not check in their lock file. This ensures that the library is tested against the latest compatible versions of its dependencies in CI, which better reflects the environment of downstream users who will resolve dependencies based on their own lock files. Since netlink-proto is a library, it is recommended to keep Cargo.lock in .gitignore.

Cargo.lock
target

@cathay4t
Copy link
Copy Markdown
Member

Any reason doing this? I have the impression that public crate does not include lock file.

@JohnTitor
Copy link
Copy Markdown
Member Author

@JohnTitor JohnTitor force-pushed the cargo-lock branch 2 times, most recently from eb207a0 to 693684f Compare March 31, 2026 08:35
@cathay4t
Copy link
Copy Markdown
Member

I know the reason of doing that for a project exposing binary.

If we include Cargo.lock for crate like this, it is possible for consumer to holds multiple different version of same crate.

In fedora rust build guideline, they suggest all rust tool depend on latest compatible crate instead of this pinned one.

Above two links does not explain why a crate-only rust project should ship Cargo.lock.

@cathay4t
Copy link
Copy Markdown
Member

IMHO, maintaining this Cargo.lock is time consuming with zero benefit? So I hope you could provide example on why it cause problem when lacking Cargo.lock?

@cathay4t
Copy link
Copy Markdown
Member

Any explanation beside emoji?

@JohnTitor
Copy link
Copy Markdown
Member Author

I don't agree with you at all.

If we include Cargo.lock for crate like this, it is possible for consumer to holds multiple different version of same crate.

I'm not sure what you mean, users can still choose via --locked or not.

In fedora rust build guideline, they suggest all rust tool depend on latest compatible crate instead of this pinned one.

It's totally unrelated to this topic.

maintaining this Cargo.lock is time consuming with zero benefit?

Explain why it's zero-benefit. Otherwise read the docs once again. You can ensure deterministic builds.

@cathay4t
Copy link
Copy Markdown
Member

cathay4t commented Mar 31, 2026

The https://github.com/rust-lang/log/blob/master/.gitignore indicate even rust-lang/log crate does not ship so.

And https://github.com/tokio-rs/tokio/blob/master/.gitignore neither.

I am aware of deterministic build, just don't know why it impact this rust-netlink fundamental crate.

I am no-issue-no-change dev. So I need proof on why make this change.

@cathay4t
Copy link
Copy Markdown
Member

For MSRV stuff mentioned by FAQ doc. We have CI ensure we are good without this lock file.

@cathay4t
Copy link
Copy Markdown
Member

cathay4t commented Mar 31, 2026

If you claim it as old custom, please provide example library only crate who is shipping this lock file.

If you want this repo ship it, please explains which use case we are solving. And also explain how this file should be updated in the future release and CI tests.

Both rust-lang.org doc has zero explicitly suggestion on whether library only crate should ship it or not.

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