fix: remove unnecessary lockfile header comparison #8622
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Eh2406 (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
On one hand we made the change intentionally, knowing that it may break some workflows. On the other hand making the diffing algoram ignore comment lines, feels small and reasonable. I put it on the Cargo Teams agenda for the next meeting. |
|
We talked about this in the Cargo Team meeting. Generally there are plans in the works to make the way we upgrade the lockfile more consistent. With the larger plan to be more consistently updating to newer formats, we don't want to take this incremental step in the other direction. Especially as #8554 will mean that changes to the lockfile will update to v2, and this small patch will not be enough to fix your workflow. Thank you for the contribution, and sorry for the inconvenience. |
|
Totally understand your points. It is reasonable to update the format steadily. Just another tiny question, should I send another PR to remove aforementioned unnecessary allocations? Sometimes Cargo.lock goes too large to ignore the cloning cost |
|
I have not seen the clones show up in profiling, but I am happy to see them go. Open to a new PR. |
fix: remove unnecessary allocations Remove unnecessary `str::to_string` and `str::replace` allocations by using iterators. This PR is almost identical to #8622 except it does not skip the generated header. Sorry that I did not profile the changes by myself. Seems that valgrind does not support macOS 10.15.6, I have not idea how to profile cargo subcommand. It would be great for an instruction to run a memory profiling for Rust program on macOS.
Closes #8610
What
#6548 introduced generated headers, which makes
fn are_equal_lockfiles()always return false if a lockfile contains the header. This PR let the equality check 1) skip lines of header and 2) remove unnecessarystr::to_stringandString::replaceallocations by using iterators to solve the minor regression bug.