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.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 22, 2025
Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 22, 2025
@wxiaoguang wxiaoguang enabled auto-merge (squash) November 22, 2025 05:33
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 22, 2025
@wxiaoguang wxiaoguang merged commit a60a8c6 into go-gitea:main Nov 22, 2025
25 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 22, 2025
@lunny lunny deleted the lunny/allow_empty_squash_merge_pr branch November 22, 2025 06:27
lunny added a commit to lunny/gitea that referenced this pull request Nov 22, 2025
…tea#35989)

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

---------

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Zettat123 <zettat123@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@lunny lunny added the backport/done All backports for this PR have been created label Nov 22, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 22, 2025
* giteaofficial/main:
  Fix various permission & login related bugs (go-gitea#36002)
  Allow empty commit when merging pull request with squash style (go-gitea#35989)
  [skip ci] Updated translations via Crowdin
  Mention proc-receive in text for dashboard.resync_all_hooks func (go-gitea#35991)
  Update JS deps (go-gitea#35978)
  wiki: reuse selectable style for wiki (go-gitea#35990)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/done All backports for this PR have been created backport/v1.25 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code modifies/internal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants