Skip to content

Comments

Allow empty commit when merging pull request with squash style#35989

Merged
wxiaoguang merged 6 commits intogo-gitea:mainfrom
lunny:lunny/allow_empty_squash_merge_pr
Nov 22, 2025
Merged

Allow empty commit when merging pull request with squash style#35989
wxiaoguang merged 6 commits intogo-gitea:mainfrom
lunny:lunny/allow_empty_squash_merge_pr

Conversation

@lunny
Copy link
Member

@lunny lunny commented Nov 20, 2025 •
edited
Loading

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

wxiaoguang commented Nov 20, 2025

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

wxiaoguang 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

@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

wxiaoguang commented Nov 20, 2025

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

silverwind commented Nov 20, 2025

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

wxiaoguang commented Nov 20, 2025

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 •
edited
Loading

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

wxiaoguang 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 :)

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

wxiaoguang commented Nov 20, 2025

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
wxiaoguang
wxiaoguang reviewed Nov 22, 2025
View reviewed changes
@wxiaoguang
Update .editorconfig
7167a5e 
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
@lunny @Zettat123 @wxiaoguang
Allow empty commit when merging pull request with squash style (go-gi…
2d58cbf 
…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 mentioned this pull request Nov 22, 2025
Merged
@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
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
4030b8b 
* 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)
lunny added a commit that referenced this pull request Nov 22, 2025
@lunny
Allow empty commit when merging pull request with squash style (#35989)…
91901c2 
… (#36003)
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 24, 2025
@zjjhot
Merge remote-tracking branch 'giteaofficial/release/v1.25'
0a09b13 
* giteaofficial/release/v1.25: (77 commits)
  Add "site admin" back to profile menu (go-gitea#36010) (go-gitea#36013)
  release notes for 1.25.2 (go-gitea#35986)
  Allow empty commit when merging pull request with squash style (go-gitea#35989) (go-gitea#36003)
  Fix various permission & login related bugs (go-gitea#36002) (go-gitea#36004)
  upgrade golang.org/x/crypto to 0.45.0 (go-gitea#35988)
  Change project default column icon to 'star' (go-gitea#35967) (go-gitea#35979)
  Misc CSS fixes (go-gitea#35888) (go-gitea#35981)
  Fix container push tag overwriting (go-gitea#35936) (go-gitea#35954)
  Fix corrupted external render content (go-gitea#35946) (go-gitea#35950)
  Don't show unnecessary error message to end users for DeleteBranchAfterMerge (go-gitea#35937) (go-gitea#35941)
  Limit read bytes instead of ReadAll (go-gitea#35928) (go-gitea#35934)
  Load jQuery as early as possible to support custom scripts (go-gitea#35926) (go-gitea#35929)
  Allow to display embed images/pdfs when SERVE_DIRECT was enabled on MinIO storage (go-gitea#35882) (go-gitea#35917)
  Use correct form field for allowed force push users in branch protection API (go-gitea#35894) (go-gitea#35908)
  Make OAuth2 issuer configurable (go-gitea#35915) (go-gitea#35916)
  Fix go-gitea#35763: Add proper page title for project pages (go-gitea#35773) (go-gitea#35909)
  Display source code downloads last for release attachments (go-gitea#35897) (go-gitea#35903)
  Fix team member access check (go-gitea#35899) (go-gitea#35905)
  Fix conda null depend issue (go-gitea#35900) (go-gitea#35902)
  Fix avatar upload error handling (go-gitea#35887) (go-gitea#35890)
  ...

# Conflicts:
#	go.mod
#	go.sum
#	models/actions/run_test.go
#	models/fixtures/action_run.yml
#	models/fixtures/action_run_job.yml
#	models/fixtures/action_task.yml
#	models/fixtures/branch.yml
#	models/fixtures/repo_unit.yml
#	modules/git/tree_entry_gogit.go
#	modules/git/tree_gogit.go
#	routers/web/repo/actions/view.go
#	routers/web/repo/issue_comment.go
#	services/actions/workflow.go
#	services/doctor/actions_test.go
#	services/pull/comment.go
#	services/pull/pull.go
#	services/pull/temp_repo.go
#	templates/base/head_navbar.tmpl
#	templates/swagger/v1_json.tmpl
#	tests/integration/actions_schedule_test.go
#	tests/integration/git_lfs_ssh_test.go
#	tests/integration/pull_create_test.go
#	tests/integration/pull_merge_test.go
#	tests/sqlite.ini.tmpl
#	web_src/js/components/ContextPopup.vue
@xnox xnox mentioned this pull request Dec 7, 2025
Closed
pimpale pushed a commit to hud-evals/gitea that referenced this pull request Dec 14, 2025
@lunny @Zettat123
@wxiaoguang @pimpale
Allow empty commit when merging pull request with squash style (go-gi…
038251f 
…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>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

+1 more reviewer

@Zettat123 Zettat123 Zettat123 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

1.26.0

Development

Successfully merging this pull request may close these issues.

5 participants

@lunny @wxiaoguang @silverwind @Zettat123 @GiteaBot