Skip to content

Fix missing reference prefix of commits when sync mirror repository #24868

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Fix missing reference prefix of commits when sync mirror repository #24868

wants to merge 4 commits into from

Conversation

sillyguodong
Copy link
Contributor

Fix #24824
The reference name of commits when synchronizing should also has prefix like refs/heads/<branch-name>.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 23, 2023
@sillyguodong sillyguodong marked this pull request as ready for review May 23, 2023 02:25
@wxiaoguang
Copy link
Contributor

Related to Allow for matching short-hand references in sync pulls #23070

That PR has more comments

@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 May 23, 2023
@wxiaoguang
Copy link
Contributor

I think it needs some tests, to show how parseRemoteUpdateOutput works.

wxiaoguang
wxiaoguang reviewed May 24, 2023
View reviewed changes
services/mirror/mirror_pull_test.go
input: "From https://xxx.com/xxx/xxx\n - [deleted] (none) -> test/t2\n",
results: []*mirrorSyncResult{
{
refName: "test/t2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it need prefix?

Copy link
Contributor Author

@sillyguodong sillyguodong May 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, but it seems impossible to parse out whether the current reference is a tag or a branch from the following text:

From https://xxx.com/xxx/xxx\n - [deleted] (none) -> v1.0.1\n
From https://xxx.com/xxx/xxx\n - [deleted] (none) -> test/t2\n

maybe we can use command git show-ref --tags/heads?
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I think it needs to be commented.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the comment // delete branch is not right, the example contains tags.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I did similar things in my refname PR

@wxiaoguang wxiaoguang mentioned this pull request May 25, 2023
Closed
@dboreham
Copy link

It seems like this PR could be merged already, but if there is any work needed in furtherance of that let me know.
I have a reproduction environment setup and could easily run manual tests on that.

@sillyguodong
Copy link
Contributor Author

sillyguodong commented May 26, 2023 •
edited
Loading

#24824 should be fixed by #24634, so close this PR.
Maybe this PR can be used as a patch for 1.19.4.

@lunny lunny mentioned this pull request May 29, 2023
Merged
@wxiaoguang
Copy link
Contributor

According to #23070 (comment)

Let's take this PR for 1.19.4 ?

@wxiaoguang wxiaoguang reopened this May 29, 2023
@wxiaoguang wxiaoguang added this to the 1.19.4 milestone May 29, 2023
@sillyguodong sillyguodong changed the base branch from main to release/v1.19 May 30, 2023 01:46
@sillyguodong sillyguodong changed the base branch from release/v1.19 to main May 30, 2023 01:47
@sillyguodong sillyguodong mentioned this pull request May 30, 2023
Merged
@GiteaBot GiteaBot removed this from the 1.19.4 milestone May 30, 2023
lunny pushed a commit that referenced this pull request May 30, 2023
@sillyguodong
Fix missing reference prefix of commits when sync mirror repository (#…
826b7b9 
…24994)

replace #24868
just a patch to fix #24824 in v1.19.4
The reference name of commits when synchronizing should also has prefix
like refs/heads/<branch-name>.
Codeberg-org pushed a commit to Codeberg-org/gitea that referenced this pull request Jun 3, 2023
@sillyguodong @earl-warren
Fix missing reference prefix of commits when sync mirror repository (g…
58bdcae 
…o-gitea#24994)

replace go-gitea#24868
just a patch to fix go-gitea#24824 in v1.19.4
The reference name of commits when synchronizing should also has prefix
like refs/heads/<branch-name>.

(cherry picked from commit 826b7b9)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 28, 2023
@sillyguodong sillyguodong deleted the bugfix/issue_24824 branch February 29, 2024 03:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@wxiaoguang wxiaoguang wxiaoguang left review comments

Assignees
No one assigned
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Action cannot be triggered after the mirror repository is synchronized
6 participants
@sillyguodong @wxiaoguang @dboreham @lunny @techknowlogick @GiteaBot