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

@dependabot recreate does not actually recreate the commit #4652

Open
jeffwidman opened this issue Jan 20, 2022 · 6 comments
Open

@dependabot recreate does not actually recreate the commit #4652

jeffwidman opened this issue Jan 20, 2022 · 6 comments
Labels
service 💁 Relates to Dependabot features GitHub provides T: bug 🐞 Something isn't working

Comments

@jeffwidman
Copy link
Member

jeffwidman commented Jan 20, 2022

I had a dependabot PR that I was munging around with trying to see what additional changes were required to get CI to pass.

However, I accidentally fat-fingered some git commands and dropped the original commit from Dependabot, both locally and on the remote.

So I did a @dependabot recreate since the description of that command says:

@dependabot recreate will recreate this PR, overwriting any edits that have been made to it

However, when Dependabot recreated the PR, it didn't reset the commit title or description. It merely reset the code changes of the commit.

I think this is a bug because recreating the PR should recreate the commit metadata, title, description, etc. Honestly, that's a big part of the usefulness of Dependabot, is the detailed commit messages listing what changed and links to more info. Very useful for later git blame spelunking when trying to figure out what may have caused a problem.

Notes:

  • No Job ID that is publicly accessable since this is a recreate of a specific PR.
  • commit hash that was force-pushed by Dependabot is f8bc0a02b4678cae09505a27eb8805b6ca1739ed
  • It was on a go-based repo. However, I suspect this is generic dependabot behavior that would repro for any language.
@jeffwidman jeffwidman added the T: bug 🐞 Something isn't working label Jan 20, 2022
@mctofu
Copy link
Contributor

mctofu commented Jan 26, 2022

I think this happens because a recreate triggers an update of the PR and we currently expect that we can re-use the message from the previous commit: https://github.com/dependabot/dependabot-core/blob/main/common/lib/dependabot/pull_request_updater/github.rb#L186

In this case the original commit was lost so it probably used the description from another commit in the PR?

@jeffwidman
Copy link
Member Author

In this case the original commit was lost so it probably used the description from another commit in the PR?

Yep, that's exactly what happened.

And that code section is what needs updating. But before opening a PR to fix, wanted to make sure we were all in agreement on what the desired behavior here should be.

@mctofu
Copy link
Contributor

mctofu commented Jan 27, 2022

I do think the ideal behavior here would have been to have the proper message on the commit. However, I'm seeing that in the spot that we are calling the PR updater internally we don't have the metadata available... We'd need to track down why that is the case and adjust it first.

@solvaholic
Copy link

In this case the original commit was lost so it probably used the description from another commit in the PR?

Yep, that's exactly what happened.

And that code section is what needs updating. But before opening a PR to fix, wanted to make sure we were all in agreement on what the desired behavior here should be.

In what scenarios do y'all envision @dependabot would re-use a commit message from a different pull request?

My Ruby fu is weak. Unless Ruby does something surprising with this bit of code, tho, I think any commit commit_being_updated returns should be from the same pull request where @dependabot recreate was run:

            commit =
              commits.find { |c| c.sha == old_commit } ||
              commits.find { |c| c.commit.author.name.include?(author_name) } ||
              commits.first

@bdragon
Copy link
Member

bdragon commented Oct 20, 2022

As of #5913 we'll now fall back to using the PR title as the commit message if the old commit has been lost. This is an interim solution until we have the capability to recreate the commit message for PR updates (context)

@codecholeric
Copy link

I ran into something similar today. Had an action that force-pushed something onto the original Dependabot commit (a version update of the project). However, whenever I tried a @dependabot recreate afterwards it would be incapable of creating a proper commit description and just write Dependabot couldn't find the original pull request head commit, $SHA.
I think this is really unfortunate, because the description of @dependabot recreate really sounds like the Dependabot PR will be recreated from scratch. Having this rely on some old commit message that could vanish from the repo is quite surprising.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service 💁 Relates to Dependabot features GitHub provides T: bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants