Add reparent option to Repository CreateFork API
#35430
Conversation
Normally, a fork becomes the `child` of the source repository. In this case, the fork becomes the new `parent` of the source repository. Closes: go-gitea#34848
GiteaBot
added
the
lgtm/need 2
github-actions
bot
added
modifies/api
|
I'm not certain about permissions here, but I think this option only makes sense for server admin or admin of both source and target organizations. |
models/repo/fork.go
Outdated
|
|
||
| // ReparentFork sets the fork to be an unforked repository and the forked repo becomes its fork | ||
| func ReparentFork(ctx context.Context, forkedRepoID, srcForkID int64) error { | ||
| if _, err := db.GetEngine(ctx).Table("repository").ID(srcForkID).Cols("fork_id", "is_fork").Update(&Repository{ForkID: forkedRepoID, IsFork: true}); err != nil { |
There was a problem hiding this comment.
Database transaction is necessary. And what happens for the other fork repositories of the original parent repository?
There was a problem hiding this comment.
I would expect nothing needs to happen to the other forks. This is about just one particular fork.
This was already called from within a transaction, but never know about future callers
|
This feature is unusual—it can only be accessed through the API, not the web interface. |
|
I agree this is not a usual feature that should be exposed via UI. Maybe in the admin interface? But then is it even worth it? We plan on using this via API-only. An additional feature that would make sense to expose via UI may be to change the fork_id of a repository to another repository with compatible git history. This one would only need admin access to the repository that is being changed. But I think that's a different PR. |
it's already set
| // ReparentFork sets the fork to be an unforked repository and the forked repo becomes its fork | ||
| func ReparentFork(ctx context.Context, forkedRepoID, srcForkID int64) error { | ||
| return db.WithTx(ctx, func(ctx context.Context) error { | ||
| if _, err := db.GetEngine(ctx).Table("repository").ID(srcForkID).Cols("fork_id", "is_fork").Update(&Repository{ForkID: forkedRepoID, IsFork: true}); err != nil { |
There was a problem hiding this comment.
The fork numbers should be updated as well.
There was a problem hiding this comment.
For the source repo, the number of forks remain as-is. Only its new parent is updated.
There was a problem hiding this comment.
The fork count of the source repository should be decreased, while the fork count of the new parent repository should be increased.
There was a problem hiding this comment.
The fork count in the source repository is not yet increased here and it never is because it's part of the else{} clause in the calling function.
The fork of the new parent is set as 1 already.
But I understand how this design is a little confusing. I'll split this into separate API call instead which will make this more straightforward.
| @@ -52,6 +52,7 @@ type ForkRepoOptions struct { | |||
| Name string | |||
| Description string | |||
| SingleBranch string | |||
| Reparent bool | |||
There was a problem hiding this comment.
I prefer the previous implementation to have a new API to do the reparent.
There was a problem hiding this comment.
Previous implementation? Or do you mean this should be a new API call, like Repository/CreateNewParent or similar name?
There was a problem hiding this comment.
If we do a new API call here, can we still use the repo_service.ForkRepoOptions{Reparent:true} or split off that logic on top of a fork?
There was a problem hiding this comment.
Yes, I think introducing a new API endpoint would be more appropriate. Also, the Reparent field in ForkRepoOptions feels a bit awkward.
There was a problem hiding this comment.
As long as we can use the internal Fork implementation already, I'm ok in introducing another API.
Normally, a fork becomes the
childof the source repository. In this case, the fork becomes the newparentof the source repository.Closes: #34848