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

Rust: update Cargo.lock and attempt formatting-preserving Cargo.toml update #704

Closed
fasterthanlime opened this issue Jan 14, 2021 · 2 comments · Fixed by #705
Closed

Rust: update Cargo.lock and attempt formatting-preserving Cargo.toml update #704

fasterthanlime opened this issue Jan 14, 2021 · 2 comments · Fixed by #705
Assignees
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@fasterthanlime
Copy link
Contributor

I've been using the initial Rust support ( #684 ) for a week now and I want to come up with some improvements:

Cargo.toml

The editing of Cargo.toml that is not formatting-preserving is more annoying than anticipated.

The initial PR is noisy (as expected) and the subsequent PRs aren't as noisy (also as expected), which is what is is, but acceptable.

What's less ideal is that it also strips all TOML comments from the file, so comments like "revert to upstream once PR #ZZZ lands" get lost in translation.

Cargo.lock

I initially considered Cargo.lock too hard to update, since it would require running a command like cargo check, which requires a fully-functioning Rust toolchain, potentially with a C compiler, an alternative linker (like LLVM), etc.

I also didn't want to edit it "by hand" (without running cargo) since [[package]] entries contain checksums.

As it turns out, not updating Cargo.lock prevents cargo publish from working after a release has been created, since the first thing it does is updated the Cargo.lock. It then fails because there are "dirty" (uncommitted) files in the repository.

This is only an issue for binary crates that are not in a monorepo, because:

  • library crates have their Cargo.lock gitignored
  • monorepos have their Cargo.lock at the top level, not in crates/mycrate

Nevertheless, I want to try and address both of these - I'll open a relevant PR soon.

@bcoe bcoe added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Jan 14, 2021
@bcoe
Copy link
Contributor

bcoe commented Jan 14, 2021

@fasterthanlime 👍 thanks for keeping this moving forward...

One thought, could we potentially write a parser for Cargo.lock in JavaScript? How complex are the bits we need to edit?

@fasterthanlime
Copy link
Contributor Author

One thought, could we potentially write a parser for Cargo.lock in JavaScript? How complex are the bits we need to edit?

It's also "just TOML"! I managed to achieve formatting-preserving edits with @iarna/toml, drafting the PR as we speak, here's a screenshots of the proof of concept:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
2 participants