Skip to content

Bump serde from 1.0.126 to 1.0.130 #156

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

Merged
merged 1 commit into from
Oct 20, 2021
Merged

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Oct 18, 2021

Bumps serde from 1.0.126 to 1.0.130.

Release notes

Sourced from serde's releases.

v1.0.130

  • Provide MapAccess and SeqAccess impl for reference to a dynamically sized existing impl (#2081)

v1.0.129

  • Support deserialization of remote structs that used packed repr (#2078, #2079, #2080)

v1.0.128

v1.0.127

  • Resolve warning in rustc nightly-2021-07-31+ compiling serde_test
Commits
  • 65e1a50 Release 1.0.130
  • 87d41b5 Merge pull request #2081 from dtolnay/accessunsized
  • 3f120fb Enable unsized Map/SeqAccess types to use the impl for &mut
  • 2b92c80 Release 1.0.129
  • c1c0ede Merge pull request #2080 from dtolnay/packeddrop
  • 4a66c5f Support packed remote struct without destructuring
  • 714c8a5 Add test of packed struct that cannot be destructured
  • dc0c0dc Merge pull request #2079 from dtolnay/packedremote
  • 54102ee Avoid generating ref patterns for fields of packed remote struct
  • 14accf7 Add test of remote with a packed struct
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Dependabot will merge this PR once CI passes on it, as requested by @davepacheco.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot added the dependencies Pull requests that update a dependency file label Oct 18, 2021
@davepacheco
Copy link
Collaborator

This change seems fine, but it's disappointing that it's updating Cargo.toml and Cargo.lock, given that the new version is compatible with the specification in Cargo.toml. I don't understand why it did this here and for serde_json (#154) but not with hyper (#157).

@davepacheco
Copy link
Collaborator

davepacheco commented Oct 20, 2021

I went back to the notes I wrote after experimenting with dependabot configs. To summarize what's relevant here:

  • Dependabot looks for the presence of a Cargo.lock file to determine if it's looking at an app or a library.
  • If it decides it's looking at a library, then by default it uses a versioning strategy called "increase-if-necessary". This means that if the new crate version is semver-compatible with what's in your Cargo.toml, then Dependabot won't update Cargo.toml. In this case, by definition there's no Cargo.lock, so it won't update that either.
  • If it decides it's looking at an app, then by default it uses a versioning strategy called "increase", which will always update Cargo.toml to point to the new version. In this case, by definition, there's a Cargo.lock file, and it updates that too.
  • You can override the versioning strategy, but the only ones you can specify for Cargo are "lockfile-only" (which only updates Cargo.lock) and "auto" (which decides what to do based on whether it's an app or library -- see above).

I'm sure my understanding remains incomplete. For example, oxidecomputer/omicron#285 was a PR to omicron that updated Cargo.lock and not any Cargo.toml files. Why would that ever happen? In that case, we had two crates within the workspace that depended on tokio-postgres "0.7" and Dependabot updated Cargo.lock from 0.7.2 to 0.7.3. Why didn't it update the Cargo.toml files too? You might guess that it's because there were two crates in the workspace that would be affected (although I'm not sure that's a good reason) -- but oxidecomputer/omicron#283 was a different PR that also updated a dep used in multiple crates, and in that one, it updated all the Cargo.toml files too, from expectorate 1.0.1 to 1.0.4. That wasn't strictly necessary, since these are implicitly semver caret dependencies. Does it depend on the precision of the version string? There's also #157 in this repo, where it also updated just Cargo.lock. That one also depended on only "0.14". Maybe this does depend on the precision of the version string?

Anyway, what I think we really want for Dropshot is:

  • Update Cargo.toml only when a new dep version comes out that's semver-incompatible with what we've got in Cargo.toml. (This is the main thrust of using dependabot: we want to avoid getting stuck on old dependencies just because of laziness.) Don't update it when it's semver-compatible -- that would unnecessarily constrain consumers.
  • Update Cargo.lock with every new dep version that comes out. If it was semver-incompatible with Cargo.toml, we'll update that too. If it's compatible, we still want to test the latest versions to identify breakage quickly.

This would correspond to treating Dropshot like an app, but with versioning strategy "increase-if-necessary". It really seems like this should be possible, but I don't think it is. Dependabot obviously has the ability to implement increase-if-necessary for Cargo, since it does that for libraries. But I'm pretty sure I tried requesting this versioning strategy and it reported that it's not supported for Cargo.

So I think our options with Dependabot are:

  1. Treat it like an app but only update the lockfile. This largely defeats the point.
  2. Treat it like an app. This is what we're doing today. This constrains consumers unnecessarily.
  3. Treat it like a library. This will cause Cargo.lock to get out of date with every dependabot update, which defeats the point of automation around this.
  4. Treat it like a library and stop checking in Cargo.lock. This makes it harder to tell what we've tested and makes it easy to have silent breakage for developers that's caused by external sources (i.e., someone might clone Dropshot and build and run tests and finds it's broken, even if everything worked on the last commit). This has already happened under Pin toolchain in actions #139 and was annoying.

(2) is the only useful option and it's what we're currently doing so I'm going to go ahead and land this PR and other Dependabot PRs that update Cargo.toml unnecessarily. If the consumer constraints become a problem, we can either dig deeper into the Dependabot implementation to better understand the behavior and see if there are other options, try to get it fixed, or use some other automation for this.

@davepacheco
Copy link
Collaborator

@dependabot squash and merge

Bumps [serde](https://github.com/serde-rs/serde) from 1.0.126 to 1.0.130.
- [Release notes](https://github.com/serde-rs/serde/releases)
- [Commits](serde-rs/serde@v1.0.126...v1.0.130)

---
updated-dependencies:
- dependency-name: serde
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot force-pushed the dependabot/cargo/serde-1.0.130 branch from 17e912e to ccd6734 Compare October 20, 2021 23:51
@dependabot dependabot bot merged commit 5165fc6 into main Oct 20, 2021
@dependabot dependabot bot deleted the dependabot/cargo/serde-1.0.130 branch October 20, 2021 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant