Fix sync fork for consistency #33147

changchaishi · 2025-01-08T06:03:43Z

An integration test could be added.

wxiaoguang · 2025-01-08T06:31:35Z

An integration test could be added.

I have some tests and will add them (see Improve "merge upstream" (sync fork) #33148 )

But I am not sure whether we should still use the "add remote" trick here, it seems fragile and I am not sure whether it is really helpful in daily usage.

changchaishi · 2025-01-08T06:57:22Z

But I am not sure whether we should still use the "add remote" trick here, it seems fragile and I am not sure whether it is really helpful in daily usage.

This is really what I am concerned about before submitting the PR.

One more bug I found on services/repository/merge_upstream.go:35, the current sync fork is not correct, should be base repo's defaultBranch.

https://github.com/orgs/community/discussions/11729

wxiaoguang · 2025-01-08T07:05:35Z

But I am not sure whether we should still use the "add remote" trick here, it seems fragile and I am not sure whether it is really helpful in daily usage.

This is really what I am concerned about before submitting the PR.

One more bug I found on services/repository/merge_upstream.go:35, the current sync fork is not correct, should be base repo's defaultBranch.

https://github.com/orgs/community/discussions/11729

If you have more ideas about the problem, feel free to take #33148 (there are more tests, I could close my PR if you'd like to use it in your PR)

wxiaoguang · 2025-01-08T07:15:18Z

One more bug I found on services/repository/merge_upstream.go:35, the current sync fork is not correct, should be base repo's defaultBranch.

https://github.com/orgs/community/discussions/11729

At the moment, there is something unclear in my mind.

There could be 2 behaviors:

"Sync fork" only sync the default branch (or name-matched branch), the current behavior:
- The typical workflow is like this: the user forks the repo, then base repo's default changes a lot, then the user sync the default branch into the fork, and start a new branch from the fork's default branch.
- In this case, if a user would like to update (sync) a PR, they could just go to the PR's page and click the "update" button there
"Sync fork" sync everything basing on the base repo's default branch (your suggestion?)
- There will be more edge cases to handle conflicts, and it duplicates with the "update" button on the PR page.

changchaishi · 2025-01-08T07:55:44Z

I see~

For 1. I did not notice this is the intended behavior of Gitea right now, as I am studying the code, I refer to GitHub a lot, Github has
the Number 2 behavior, so I thought this was a kind of bug.

I think we can simplify this PR without this issue first.

One more bug I found on services/repository/merge_upstream.go:35, the current sync fork is not correct, should be the base repo's defaultBranch.

https://github.com/orgs/community/discussions/11729

If you have more ideas about the problem, feel free to take #33148 (there are more tests, I could close my PR if you'd like to use it in your PR)

I reviewed it, and this is nice, how can I use the test? Must you close it so that I can use it, or I can just reference it to create something like repo_sync_fork_rest.py

wxiaoguang · 2025-01-08T08:02:42Z

Actually I am not sure which behavior is better for end users. Maybe following GitHub's behavior is better since it is widely used. 🤣

If you have more ideas about the problem, feel free to take #33148 (there are more tests, I could close my PR if you'd like to use it in your PR)

I reviewed it, and this is nice, how can I use the test? Must you close it so that I can use it, or I can just reference it to create something like repo_sync_fork_rest.py

Some possible choices:

You could merge my branch into yours https://github.com/wxiaoguang/gitea/tree/improve-merge-upstream
Or just copy the code into your PR
Or wait for WIP: Improve "merge upstream" (sync fork) #33148 to be merged
Or I could merge the code into this PR and work together here.

services/repository/merge_upstream.go

wxiaoguang · 2025-01-09T12:02:22Z

If you don't mind, I think I can merge #33148 here, then we do not need to use a separate PR to handle the similar tests.

wxiaoguang · 2025-01-10T03:30:00Z

Made some new changes:

merge WIP: Improve "merge upstream" (sync fork) #33148 and reuse the tests
- the new case is covered by BaseChangeAfterHeadChange
use base repo's default branch to sync (as you suggested and as GitHub does)

Fixes go-gitea#33145 An integration test could be added. --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

Backport #33147 by changchaishi Fixes #33145 An integration test could be added. --------- Co-authored-by: Chai-Shi <changchaishi@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

* giteaofficial/main: (21 commits) Support public code/issue access for private repositories (go-gitea#33127) Validate that the tag doesn't exist when creating a tag via the web (go-gitea#33241) [skip ci] Updated translations via Crowdin Switch back to `vue-tsc` (go-gitea#33248) Let API create and edit system webhooks, attempt 2 (go-gitea#33180) Fix incorrect ref "blob" (go-gitea#33240) Refactor RefName (go-gitea#33234) Refactor context RefName and RepoAssignment (go-gitea#33226) [skip ci] Updated translations via Crowdin Fix upload file form (go-gitea#33230) Fix mirror bug (go-gitea#33224) Remove unused CSS styles and move some styles to proper files (go-gitea#33217) Refactor context repository (go-gitea#33202) [skip ci] Updated translations via Crowdin Fix unpin hint on the pinned pull requests (go-gitea#33207) fix(cache): cache test triggered by non memory cache (go-gitea#33220) Update README.md (go-gitea#33149) Fix editor markdown not incrementing in a numbered list (go-gitea#33187) Some small refactors (go-gitea#33144) Fix sync fork for consistency (go-gitea#33147) ...

fix sync fork

957aed4

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 8, 2025

pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 8, 2025

github-actions bot added the modifies/go Pull requests that update Go code label Jan 8, 2025

lunny requested a review from wxiaoguang January 8, 2025 06:11

changchaishi marked this pull request as draft January 8, 2025 06:12

wxiaoguang reviewed Jan 8, 2025

View reviewed changes

services/repository/merge_upstream.go Outdated Show resolved Hide resolved

avoid using tmp remote for comparison to fork branch

04c2a5e

pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 9, 2025

changchaishi changed the title ~~[WIP] Fix sync fork for consistency~~ Fix sync fork for consistency Jan 9, 2025

changchaishi marked this pull request as ready for review January 9, 2025 10:15

add repo sync fork integration test

1ec4df8

changchaishi force-pushed the fix-sync-fork branch from 829ff2a to 1ec4df8 Compare January 10, 2025 01:52

pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 10, 2025

wxiaoguang added type/bug backport/v1.23 This PR should be backported to Gitea 1.23 labels Jan 10, 2025

merge

cc008dd

pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 10, 2025

github-actions bot added the modifies/api This PR adds API routes or modifies them label Jan 10, 2025

use base repo's default branch to sync

69fb07b

wxiaoguang added 2 commits January 10, 2025 11:45

fix git repo open

93d9efd

fix tests

d87dfbf

github-actions bot added the modifies/templates This PR modifies the template files label Jan 10, 2025

wxiaoguang approved these changes Jan 10, 2025

View reviewed changes

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 Jan 10, 2025

wxiaoguang added 3 commits January 10, 2025 12:12

fine tune

1ca671b

fix comment

491bc53

fix lint

dd35251

wxiaoguang added this to the 1.24.0 milestone Jan 10, 2025

lunny approved these changes Jan 10, 2025

View reviewed changes

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 Jan 10, 2025

lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 10, 2025

Merge branch 'main' into fix-sync-fork

7c7290c

lunny enabled auto-merge (squash) January 10, 2025 05:27

lunny merged commit e5f3c16 into go-gitea:main Jan 10, 2025
26 checks passed

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 10, 2025

changchaishi deleted the fix-sync-fork branch January 10, 2025 05:30

GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Jan 10, 2025

Fix sync fork for consistency (go-gitea#33147)

aba09da

Fixes go-gitea#33145 An integration test could be added. --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

GiteaBot mentioned this pull request Jan 10, 2025

Fix sync fork for consistency (#33147) #33192

Merged

GiteaBot added the backport/done All backports for this PR have been created label Jan 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sync fork for consistency #33147

Fix sync fork for consistency #33147

changchaishi commented Jan 8, 2025

wxiaoguang commented Jan 8, 2025 •

edited

Loading

changchaishi commented Jan 8, 2025

wxiaoguang commented Jan 8, 2025

wxiaoguang commented Jan 8, 2025 •

edited

Loading

changchaishi commented Jan 8, 2025

wxiaoguang commented Jan 8, 2025 •

edited

Loading

wxiaoguang commented Jan 9, 2025

wxiaoguang commented Jan 10, 2025 •

edited

Loading

Fix sync fork for consistency #33147

Fix sync fork for consistency #33147

Conversation

changchaishi commented Jan 8, 2025

wxiaoguang commented Jan 8, 2025 • edited Loading

changchaishi commented Jan 8, 2025

wxiaoguang commented Jan 8, 2025

wxiaoguang commented Jan 8, 2025 • edited Loading

changchaishi commented Jan 8, 2025

wxiaoguang commented Jan 8, 2025 • edited Loading

wxiaoguang commented Jan 9, 2025

wxiaoguang commented Jan 10, 2025 • edited Loading

wxiaoguang commented Jan 8, 2025 •

edited

Loading

wxiaoguang commented Jan 8, 2025 •

edited

Loading

wxiaoguang commented Jan 8, 2025 •

edited

Loading

wxiaoguang commented Jan 10, 2025 •

edited

Loading