Skip to content

Conversation

@lunny
Copy link
Member

@lunny lunny commented Nov 20, 2025

Before this PR, when merging an empty PR with squash style will result in 500.

How to reproduce the issue:
1. Create a new branch dev from main.
2. Add some changes and commit them on the dev branch.
3. Open a pull request from dev to main.
4. Manually cherry-pick the commit from dev into main.
5. Open the pull request page and attempt a Squash Merge → a 500 error occurs.

This PR will allow the behavior.

141a51853afac0686fa213af5d7c1f66

@lunny lunny added this to the 1.26.0 milestone Nov 20, 2025
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 20, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Nov 20, 2025
@wxiaoguang
Copy link
Contributor

Why user can't provide a message? What's the use case for keeping the message empty?

@lunny
Copy link
Member Author

lunny commented Nov 20, 2025

Why user can't provide a message? What's the use case for keeping the message empty?

It's an empty commit, not an empty commit message.

@wxiaoguang
Copy link
Contributor

Why user can't provide a message? What's the use case for keeping the message empty?

It's an empty commit, not an empty commit message.

OK, then why an empty commit should be supported? What's the use case

@lunny
Copy link
Member Author

lunny commented Nov 20, 2025

Why user can't provide a message? What's the use case for keeping the message empty?

It's an empty commit, not an empty commit message.

OK, then why an empty commit should be supported? What's the use case

I updated the issue content. Looks at the image on that. The previous implementation in Gitea has a warning hint there but it said this will be an empty commit. So that to keep the behavior consistent, it's nature to allow commit the empty commit. I can also change the hint and disable the button but I think that should be break change?

@wxiaoguang
Copy link
Contributor

Without the AddArguments("--allow-empty"), your TestPullSquashMergeEmpty still passes.

If it tests nothing, you can delete the test to avoid wasting CI time.

@silverwind
Copy link
Member

I'm confused because the screenshot shows "1 files changed", but a empty commit would actually be "0 files changed". Are you sure it's targeting the correct mechanism?

@wxiaoguang
Copy link
Contributor

I'm confused because the screenshot shows "1 files changed", but a empty commit would actually be "0 files changed". Are you sure it's targeting the correct mechanism?

That's how git works.

  • On "main" branch: set "file.txt" content to "dummy"
  • Create a new "branch-1", edit "file.txt", change its content to "changed" and create a PR
  • On "main" branch: set "file.txt" content to "changed"
  • Then "branch-1" can be merged into "main" without conflict or change.

@silverwind
Copy link
Member

silverwind commented Nov 20, 2025

So basically a branch that once had changes, but was then made to match the target branch. I still think the UI should show "0 changed files" in such a case as I'm pretty sure git diff would also show empty in such a case. In any case, this discussion is offtopic :)

@wxiaoguang
Copy link
Contributor

So basically a branch that once had changes, but was then made to match the target branch. I still think the UI should show "0 changed files" in such a case as I'm pretty sure git diff would also show empty in such a case. In any case, this discussion is offtopic :)

No. The "diff" is from its merge base.

If you want to see 0, click that "Update branch" button.

@lunny
Copy link
Member Author

lunny commented Nov 20, 2025

How to reproduce the issue:
1. Create a new branch dev from main.
2. Add some changes and commit them on the dev branch.
3. Open a pull request from dev to main.
4. Manually cherry-pick the commit from dev into main.
5. Open the pull request page and attempt a Squash Merge → a 500 error occurs.

@wxiaoguang
Copy link
Contributor

Yep, it can be reproduced if there is nothing to merge. My comment #35989 (comment) can also easily reproduce.

But the problem is: your test code is not right

Without the AddArguments("--allow-empty"), your TestPullSquashMergeEmpty still passes.
If it tests nothing, you can delete the test to avoid wasting CI time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants