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

Consider less intrusive cargo manifest updating #7

Open
justinrubek opened this issue Mar 18, 2023 · 4 comments
Open

Consider less intrusive cargo manifest updating #7

justinrubek opened this issue Mar 18, 2023 · 4 comments

Comments

@justinrubek
Copy link
Owner

#3 brought in the ability to edit cargo manifest files directly. However, extra fields are written to Cargo.lock and existing fields get reordered (see 12572ae#diff-13ee4b2252c9e516a0547f2891aa2105c3ca71c6d7a1e682c69be97998dfc87e).

I'm not sure how much work it would be to prevent this from happening, but I anticipate that it could be a non-trivial change. In the meantime it should be sufficient to either

  1. Use by_file replacement (just like how was done in 0.5.0)
  2. Accept the modified files. It doesn't break them but just makes them more explicit

I'm not entirely against leaving things as they are but it is something worth considering

@justinrubek
Copy link
Owner Author

I just ran into this as an issue. I had a proc macro crate that had like this:

[lib]
proc-macro = true

When running bomper, it added crate-type = ["rlib"] which is not accurate. That wasn't the only change I found annoying.

Some goals I think should be accomplished as a result of this:

  1. Provide for the ability to not add any fields that were not set previously
  2. Ensure fields that were set are not changed. For example, is the package entry has version = { workspace = true }, it should not be overridden with an explicit value, but instead it should keep inheriting it from the workspace

@justinrubek
Copy link
Owner Author

I should add that it likely isn't enough to try to ensure that a field such as crate-type is already specified. For the proc-macro, setting crate-type = ["proc-macro"] will trigger a clippy lint.

@justinrubek
Copy link
Owner Author

It seems that this is due to calling from_path which calls complete_from_path to fill in missing implicit data. Instead, from_slice can be used to retrieve only the included data.

@justinrubek justinrubek linked a pull request Apr 2, 2023 that will close this issue
@justinrubek
Copy link
Owner Author

justinrubek commented Apr 2, 2023

I'm leaving this open despite #9. It seems that comments may be removed [from Cargo.toml files] and existing data is reformatted. I think stopping implicit data from being added is a big win, but further efforts may be a good idea. I don't particularly mind the reformatting but comments might be nice to have. I prefer to keep my files without them, but if someone with a use case for them comes by it could be fixed.

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

No branches or pull requests

1 participant