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

enable GitHub Dependabot #457

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rursprung
Copy link

this ensures that the dependencies are kept up to date. see the docs for further information.

note that cortex-m-rt/macros isn't listed in the main workspace and also not in cortex-m-rt. thus it has been added here explicitly.

this ensures that the dependencies are kept up to date. see [the docs][]
for further information.

note that `cortex-m-rt/macros` isn't listed in the main `workspace` and
also not in `cortex-m-rt`. thus it has been added here explicitly.

[the docs]: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates
@rursprung rursprung requested a review from a team as a code owner December 28, 2022 18:14
Copy link
Member

@newAM newAM left a comment

Choose a reason for hiding this comment

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

I'm ok with this, I will wait for other maintainers to comment because I know there are some disadvantages with dependabot.

@adamgreig
Copy link
Member

I'm afk right now, but what are the advantages of dependabot for Rust projects? My third-hand experience has been a lot of email and PR noise, but it's not clear to me that always having the latest patch release in Cargo.toml really makes any difference. We don't currently use it anywhere in the rust-embedded org afaik.

@thejpster
Copy link
Contributor

I would expect any dependencies' security updates to be non breaking changes, which users of this library will just pick up automatically, right? I wonder if dependabot is more useful for binary crates with a Cargo.lock file.

@rursprung
Copy link
Author

my personal experience is that it helps with keeping track of updates: this way you'll get a PR whenever there's a new version of a dependency (where dependabot can do the update - it keeps track of its PRs and if too many of them fail it'll stop offering it to other repos; the docs go into more details on this).
when i run cargo tree on an embedded rust project i see various crates (libraries) depending on various versions of other crates. if all of them had dependabot enabled that tree would look flatter, potentially even leading to smaller binaries at the end (code de-duplication). here the main problem is probably that a lot of the rust (embedded) ecosystem is still on 0.x version numbers where each minor step is already considered a (potentially) breaking change.
presuming that the crates follow semver the updates done by dependabot are non-breaking (unless it's a major release in which case manual interaction is usually required - and there you can also tell dependabot to close the PR and ignore the major release so that you can work on it yourself in another, bigger, PR at your own leisure) and it's smooth sailing to get them in.

after enabling it for the first time it can indeed be that you get a bunch of PRs, but after that the rate should be pretty low - of course depending on how many dependencies you have and how often these release.

most big projects i know (not necessarily rust projects) have dependabot enabled and are actively using it (presuming they're using a language which is supported by dependabot).

I wonder if dependabot is more useful for binary crates with a Cargo.lock file.

indeed you'll notice the biggest impact with binaries due to the lock file, but that doesn't mean that updating the Cargo.toml in libraries isn't already helpful either. it just helps automating things which you'd otherwise have to do tediously manually (and thus often don't do it regularly - at least that's my experience).

@thejpster
Copy link
Contributor

If the change is non-breaking, then cargo should pick it up automatically when the user does a build.

@rursprung
Copy link
Author

If the change is non-breaking, then cargo should pick it up automatically when the user does a build.

non-breaking here also means that even if according to semver it might be breaking it isn't. e.g. a crate going from 0.x to 0.y is often (but not always) non-breaking even though according to semver it can be breaking. dependabot will attempt this upgrade even if semver says that it might not work.

@adamgreig
Copy link
Member

when i run cargo tree on an embedded rust project i see various crates (libraries) depending on various versions of other crates. if all of them had dependabot enabled that tree would look flatte

For semver-compatible updates, this isn't true - Cargo will automatically use the newest compatible version when built, for all projects, so you shouldn't ever get duplicate versions that could have been automatically updated. Perhaps this is one way that Cargo means dependabot is less useful for Rust libraries than other projects, though I don't know about the version control for specific other languages.

non-breaking here also means that even if according to semver it might be breaking it isn't. e.g. a crate going from 0.x to 0.y is often (but not always) non-breaking even though according to semver it can be breaking. dependabot will attempt this upgrade

Conversely, this doesn't seem good - those updates are breaking in some way or they wouldn't use a non-compatible version number, so it just means that either a) the break doesn't happen to affect us, or b) dependabot hasn't discovered how the break affects us. I'm not fully convinced of the utility here vs doing these rarer updates manually, but perhaps if dependabot is configured to only notify about semver-incompatible updates (which Cargo wouldn't automatically apply), it would be useful to just get notified of them, and should be much less noisy.

@therealprof
Copy link
Contributor

FWIW I'm also not a fan. The noise/usefulness balance for Rust in general just isn't there. The only partial utility I can see is for Rust project with -sys crates since they tend to have more frequent vulnerabilities which require immediate action.

@rursprung
Copy link
Author

Conversely, this doesn't seem good - those updates are breaking in some way or they wouldn't use a non-compatible version number

what i've seen (and personally done) with 0.x releases is that you often bump the minor if you just add large new features - even if it's a non-breaking change (i.e. you treat it like a 1.x -> 1.y release even though it's a 0.x -> 0.y release and could thus also be 0.x.a -> 0.x.b). so these are non-breaking in reality but considered breaking by semver (and thus Cargo doesn't automatically pick these up). with proper CI infrastructure in place (which seems to be the case here) you'll notice immediately if such a bump was really breaking.

but i'm also ok with not having dependabot here if you don't want it - i wanted to launch the discussion to see whether you'd be ok with it :)

@rursprung
Copy link
Author

just as an addendum to maybe clarify something: for library crates dependabot doesn't create unnecessary bumps, e.g. my r2d2-oracle crate depends on oracle = "0.5.1" (source) and has dependabot enabled but dependabot doesn't create a PR even though oracle = "0.5.6" would be out.
but once oracle 0.6 comes out it will create a PR which may or may not work (in my specific case it'll most likely work as i'm only using 1-2 small APIs from the crate).

so there's very little noise there and it helps with keeping track of dependency updates - i might not notice it otherwise (ok, as i'm the sole developer on that crate i can also watch the oracle and r2d2 repos on GH to get notified of new releases, but for organisations this is harder unless you have somebody dedicated for that task).

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.

None yet

5 participants