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

Merge failed: Commit message is too long (maximum is 16383 characters) #8972

Closed
1 task done
OmarTawfik opened this issue Feb 3, 2024 · 7 comments
Closed
1 task done
Assignees
Labels
good first issue L: git:submodules Git submodules L: go:modules Golang modules L: rust:cargo Rust crates via cargo T: bug 🐞 Something isn't working

Comments

@OmarTawfik
Copy link

OmarTawfik commented Feb 3, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Package ecosystem

cargo

Package manager version

No response

Language version

No response

Manifest location and content before the Dependabot update

/Cargo.toml

dependabot.yml content

https://github.com/NomicFoundation/slang/blob/main/.github/dependabot.yml

Updated dependency

NomicFoundation/slang#778

What you expected to see, versus what you actually saw

Expected

A valid commit that can be merged.

Actual

Enabling auto-merge failed.
Commit message is too long (maximum is 16383 characters).

Maybe for such extreme cases, we can just truncate the message?

Native package manager behavior

No response

Images of the diff or a link to the PR, issue, or logs

NomicFoundation/slang@7cf7241

image

Smallest manifest that reproduces the issue

No response

@OmarTawfik OmarTawfik added the T: bug 🐞 Something isn't working label Feb 3, 2024
@sachin-sandhu sachin-sandhu self-assigned this May 8, 2024
@github-actions github-actions bot added L: git:submodules Git submodules L: go:modules Golang modules labels May 8, 2024
@thavaahariharangit
Copy link
Collaborator

thavaahariharangit commented May 10, 2024

@OmarTawfik

I am unable recreate above problem

Step I followed are below

  1. git clone the repo to local https://github.com/NomicFoundation/slang.git
git clone https://github.com/NomicFoundation/slang.git
  1. Ran cargo build against it ensure repo is working as expected
$ cargo install "slang_solidity"
    Updating crates.io index

# build ran without any problem. 
  1. Create a new repo with same code to test against dependabot
https://github.com/thavaahariharangit/slang
  1. Ran dependabot check updates against https://github.com/thavaahariharangit/slang
insight -> dependency graph -> dependabot -> check for updates
# it generated 5 PRs
  1. merged 4 PR without any error.

Only error I saw while running check for updates is

 10:34:17 ERROR <job_826520924> Error processing url (RuntimeError)
updater | 2024/05/10 10:34:17 ERROR <job_826520924> Failed to update url!
updater | 2024/05/10 10:34:17 ERROR <job_826520924> /home/dependabot/cargo/lib/dependabot/cargo/file_updater/lockfile_updater.rb:45:in `block in updated_lockfile_content'
updater | 2024/05/10 10:34:17 ERROR <job_826520924> /home/dependabot/common/lib/dependabot/shared_helpers.rb:81:in `block in in_a_temporary_directory'

But it is not related to this ticket.

Could you please let me know the steps to reproduce and let me know why you think this issue is related to dependabot pls

FYI @abdulapopoola and @jurre
@sachin-sandhu

@thavaahariharangit
Copy link
Collaborator

thavaahariharangit commented May 10, 2024

@OmarTawfik

Reg: Analyzing on NomicFoundation/slang#930

And this PR is not generated by dependabot, I couldn't find the job related to this PR

@thavaahariharangit thavaahariharangit self-assigned this May 10, 2024
@OmarTawfik
Copy link
Author

OmarTawfik commented May 11, 2024

Hi @thavaahariharangit

I see in your fork, you are testing the slang repo from the latest main commits, not when this issue was created. The bug happened when dependabot ran on February 3rd. You can see the commit created by dependabot here: NomicFoundation/slang@7cf7241

You can possibly try to reproduce the same update commit by running against its parent commit, not the latest main: NomicFoundation/slang@307c0b8

But I don't think you necessarily need to reproduce it. You can just check the commit message locally if you want. This update commit message has 40841 characters, because there were many dependencies updated. But the maximum allowed by GitHub to merge a PR is 16383 characters, so it can never be merged, resulting the error I posted above. To unblock ourselves, I had to check out the PR locally, edit the commit message to truncate it, then force push and try to merge again.

I believe dependeabot should enforce the same length on the commits it creates, to match GitHub restrictions, truncating the message if it was too long.

As for the other url error, it is indeed a second unrelated issue, which is still happening today. I filed #8989 for it separately to be fixed.

@sachin-sandhu
Copy link
Contributor

sachin-sandhu commented May 14, 2024

Hi @OmarTawfik ,

I went through the dependabot generated PR commit message (9feb42d8) and found the length to be 65849 ( git show -s --format=%B 9feb42d8 | wc -c ) . GitHub has a max limit of 65536 chars for a commit message. Hence the PR generated an issue. We are pushing a fix that should fix this issue.

sachin-sandhu added a commit that referenced this issue May 14, 2024
#8972 - Fixes issue when Dependabot generates a PR commit message that is longer than 65536 chars.
sachin-sandhu added a commit that referenced this issue May 14, 2024
(Amend)

#8972 - Fixes issue when Dependabot generates a PR commit message that is longer than 65536 chars.
@sachin-sandhu
Copy link
Contributor

@sachin-sandhu
Copy link
Contributor

Closing the issue as fix is deployed.

@jurre
Copy link
Member

jurre commented May 29, 2024

@jurre jurre closed this as completed May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue L: git:submodules Git submodules L: go:modules Golang modules L: rust:cargo Rust crates via cargo T: bug 🐞 Something isn't working
Projects
Status: Done
6 participants