restore-repo: issue comments are no longer imported #10147

Closed
opened 2025-11-02 08:59:31 -06:00 by GiteaMirror · 6 comments
Owner

Originally created by @drsybren on GitHub (Jan 23, 2023).

Description

Since 71ca3067bc issue comments are no longer imported with gitea restore-repo.

See func (r *RepositoryRestorer) GetComments(...) in services/migrations/restore.go , it still uses commentable.GetForeignIndex() (link) but as far as I can see that foreign index isn't loaded any more since that commit.

This patches things up for me locally, but I don't know if this is the right thing to do:

diff --git a/services/migrations/restore.go b/services/migrations/restore.go
index fd337b22c..6ee878a64 100644
--- a/services/migrations/restore.go
+++ b/services/migrations/restore.go
@@ -198,7 +198,7 @@ func (r *RepositoryRestorer) GetIssues(page, perPage int) ([]*base.Issue, bool,
 // GetComments returns comments according issueNumber
 func (r *RepositoryRestorer) GetComments(commentable base.Commentable) ([]*base.Comment, bool, error) {
        comments := make([]*base.Comment, 0, 10)
-       p := filepath.Join(r.commentDir(), fmt.Sprintf("%d.yml", commentable.GetForeignIndex()))
+       p := filepath.Join(r.commentDir(), fmt.Sprintf("%d.yml", commentable.GetLocalIndex()))
        _, err := os.Stat(p)
        if err != nil {
                if os.IsNotExist(err) {

To reproduce the issue, unzip demo-repo-22581.zip, then run gitea restore-repo -r /path/to/demo-repo-22581 --owner_name youruser --repo_name demo-repo-22581

When visiting the repo in Gitea's web interface, you'll notice that there is one imported issue, but no comments, even though there is a corresponding file in demo-repo-22581/comment/104010.yml.

There's nothing about this in the logs.

Gitea Version

29b78bce33

How are you running Gitea?

Self-built from the main branch.

Database

PostgreSQL

Originally created by @drsybren on GitHub (Jan 23, 2023). ### Description Since https://github.com/go-gitea/gitea/commit/71ca3067bcc6c7c7772d38fc7590505c8c7148ed issue comments are no longer imported with `gitea restore-repo`. See `func (r *RepositoryRestorer) GetComments(...)` in `services/migrations/restore.go` , it still uses `commentable.GetForeignIndex()` ([link](https://github.com/go-gitea/gitea/blob/29b78bce3336fcedc08d17b71a52e593d65cd951/services/migrations/restore.go#L201)) but as far as I can see that foreign index isn't loaded any more since that commit. This patches things up for me locally, but I don't know if this is the right thing to do: ```diff diff --git a/services/migrations/restore.go b/services/migrations/restore.go index fd337b22c..6ee878a64 100644 --- a/services/migrations/restore.go +++ b/services/migrations/restore.go @@ -198,7 +198,7 @@ func (r *RepositoryRestorer) GetIssues(page, perPage int) ([]*base.Issue, bool, // GetComments returns comments according issueNumber func (r *RepositoryRestorer) GetComments(commentable base.Commentable) ([]*base.Comment, bool, error) { comments := make([]*base.Comment, 0, 10) - p := filepath.Join(r.commentDir(), fmt.Sprintf("%d.yml", commentable.GetForeignIndex())) + p := filepath.Join(r.commentDir(), fmt.Sprintf("%d.yml", commentable.GetLocalIndex())) _, err := os.Stat(p) if err != nil { if os.IsNotExist(err) { ``` To reproduce the issue, unzip [demo-repo-22581.zip](https://github.com/go-gitea/gitea/files/10511505/demo-repo-22581.zip), then run `gitea restore-repo -r /path/to/demo-repo-22581 --owner_name youruser --repo_name demo-repo-22581` When visiting the repo in Gitea's web interface, you'll notice that there is one imported issue, but no comments, even though there is a corresponding file in `demo-repo-22581/comment/104010.yml`. There's nothing about this in the logs. ### Gitea Version 29b78bce3336fcedc08d17b71a52e593d65cd951 ### How are you running Gitea? Self-built from the `main` branch. ### Database PostgreSQL
GiteaMirror added the type/bug label 2025-11-02 08:59:31 -06:00
Author
Owner

@zeripath commented on GitHub (Jan 24, 2023):

Hmm... My original suggestion was to simply add a primary key to that table and IIRC correctly my code to do that was even removed from that PR and therefore I should not have been left on the commit as an author.

IIRC it was later decided that the foreign reference wasn't used anywhere and it was simpler to drop the table completely. I don't know this code very well so I don't understand why and wherefore this was added.

@wxiaoguang IIRC you said to remove this table?

@zeripath commented on GitHub (Jan 24, 2023): Hmm... My original suggestion was to simply add a primary key to that table and IIRC correctly my code to do that was even removed from that PR and therefore I should not have been left on the commit as an author. IIRC it was later decided that the foreign reference wasn't used anywhere and it was simpler to drop the table completely. I don't know this code very well so I don't understand why and wherefore this was added. @wxiaoguang IIRC you said to remove this table?
Author
Owner

@zeripath commented on GitHub (Jan 24, 2023):

Now looking at #18446. ForeignReference was added as a way to keep a match between a migrated repo issue id and pull id and its original repo. The idea presumably being that it would help in future to auto add incremental migrations and federation etc.

Of course the problem is whether the ForeignReference is actually helpful for that - possibly not as it may need to have the actual url for the entity that is referencing, but I'm not certain.


Now I don't fully understand why the code in #21721 would break things. @drsybren could you please give clear instructions on how to reproduce your issue and report any logs?

@zeripath commented on GitHub (Jan 24, 2023): Now looking at #18446. ForeignReference was added as a way to keep a match between a migrated repo issue id and pull id and its original repo. The idea presumably being that it would help in future to auto add incremental migrations and federation etc. Of course the problem is whether the ForeignReference is actually helpful for that - possibly not as it may need to have the actual url for the entity that is referencing, but I'm not certain. --- Now I don't fully understand why the code in #21721 would break things. @drsybren could you please give clear instructions on how to reproduce your issue and report any logs?
Author
Owner

@wxiaoguang commented on GitHub (Jan 25, 2023):

Maybe the PR https://github.com/go-gitea/gitea/pull/21721 missed something (hmm, it's caused by an old bug in history). If @wolfogre doesn't have time, I can take it.


ps: I think the patch in the first comment could be right. We can take it, but need to clarify all details (see the comment below)

@wxiaoguang commented on GitHub (Jan 25, 2023): Maybe the PR https://github.com/go-gitea/gitea/pull/21721 missed something (hmm, it's caused by an old bug in history). If @wolfogre doesn't have time, I can take it. ---- ps: I think the patch in the first comment could be right. We can take it, but need to clarify all details (see the comment below)
Author
Owner

@wxiaoguang commented on GitHub (Jan 25, 2023):

About why:

Now I don't fully understand why the code in 21721 would break things.

Actually, I think it's caused by some PRs before 18446. If I understand correctly, there was an IssueContext, which has ForeignID() and LocalID(), they all returned the same result at that time. However, that design also seems to be flawed, and using ForeignID for issue yaml files seems to be incorrect: The exported files are named by the original repo's issue ID -- which is likely to be LocalID.

The root problem is: the ForeignID and LocalID were already ambiguity. For example, if site-a exports a dataset, then site-b imports it:

  • site-a exports files by using its LocalID
  • from site-a's view, LocalID is site-a's ID while ForeignID is site-b's ID
  • but from site-b's view, LocalID is site-b's ID while ForeignID is site-a's ID

Maybe I misunderstood something , but indeed the design really seemed misleading for me.


In history, that's all correct: fmt.Sprintf("%d.yml", opts.IssueNumber))

2d1935acc7/modules/migrations/restore.go

But #16356 introduced IssueContext and started using ForeignID. At that time ForeignID was LocalID, so the design bug didn't cause problems. But now, the bug finally causes problems.

@wxiaoguang commented on GitHub (Jan 25, 2023): About why: > Now I don't fully understand why the code in 21721 would break things. Actually, I think it's caused by some PRs before 18446. If I understand correctly, there was an `IssueContext`, which has `ForeignID()` and `LocalID()`, they all returned the same result at that time. However, that design also seems to be flawed, and using `ForeignID` for issue yaml files seems to be incorrect: The exported files are named by the original repo's issue ID -- which is likely to be `LocalID`. The root problem is: the `ForeignID` and `LocalID` were already ambiguity. For example, if site-a exports a dataset, then site-b imports it: * site-a exports files by using its LocalID * from site-a's view, LocalID is site-a's ID while ForeignID is site-b's ID * but from site-b's view, LocalID is site-b's ID while ForeignID is site-a's ID Maybe I misunderstood something , but indeed the design really seemed misleading for me. ---- In history, that's all correct: `fmt.Sprintf("%d.yml", opts.IssueNumber))` https://github.com/realaravinth/gitea/blame/2d1935acc7b2a6ecc797346625b80caa7e0b5787/modules/migrations/restore.go But #16356 introduced `IssueContext` and started using `ForeignID`. At that time ForeignID was LocalID, so the design bug didn't cause problems. But now, the bug finally causes problems.
Author
Owner

@drsybren commented on GitHub (Jan 26, 2023):

To reproduce the issue, unzip demo-repo-22581.zip, then run gitea restore-repo -r /path/to/demo-repo-22581 --owner_name youruser --repo_name demo-repo-22581

When visiting the repo in Gitea's web interface, you'll notice that there is one imported issue, but no comments, even though there is a corresponding file in demo-repo-22581/comment/104010.yml.

There's nothing about this in the logs.

@drsybren commented on GitHub (Jan 26, 2023): To reproduce the issue, unzip [demo-repo-22581.zip](https://github.com/go-gitea/gitea/files/10511505/demo-repo-22581.zip), then run `gitea restore-repo -r /path/to/demo-repo-22581 --owner_name youruser --repo_name demo-repo-22581` When visiting the repo in Gitea's web interface, you'll notice that there is one imported issue, but no comments, even though there is a corresponding file in `demo-repo-22581/comment/104010.yml`. There's nothing about this in the logs.
Author
Owner

@wxiaoguang commented on GitHub (Feb 6, 2023):

Fix restore repo bug, clarify the problem of ForeignIndex #22776

@wxiaoguang commented on GitHub (Feb 6, 2023): Fix restore repo bug, clarify the problem of ForeignIndex #22776
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#10147