Show changes in submodules in PR #11227

New Issue

GiteaMirror · 2025-11-02T09:31:24-06:00

GiteaMirror commented

2025-11-02 09:31:24 -06:00

Originally created by @mikaeleimantlab on GitHub (Jul 14, 2023).

Feature Description

It would be very useful to be able to see changes made in submodules when reviewing a PR, instead of just seeing the changed submodule commit hash.

This is supported by git using the git diff --submodule=diff main option, which I'm guessing means that the changes needed on the Gitea end would be relatively small: submodule contents would need to be fetched and the diff command tweaked.

I imagine this would be an option to enable (per repo, per org or globally?).

Example (cleaned for readability) output from git diff as it used by Gitea now:

README.md:
+
+And now we've added a sub repo, to see how that works.

sub-repo:
-Subproject commit 773e95df076c02e982bea7eb745866e92e0cfcfb
+Subproject commit 4326ed282c7f5bd49fd4a29c1ac0f414d25d71b9

Example output using git diff --submodule=diff:

README.md:
+
+And now we've added a sub repo, to see how that works.

sub-repo/README.md:
+# Sub repo
+
+Test av hur Gitea hanterar ändringar i sub-repon.

This would make it a lot easier to review all the changes for a PR of the parent repo in a single view, instead of manually having to navigate separate PRs for each submodule. Especially in projects using many submodules, this saves a lot of time and mental effort.

You would still need to create PRs for merging the changes for each submodule into their respective main branches, but at least the review can be performed efficiently for the overall work before performing those PRs.

Screenshots

No response

Originally created by @mikaeleimantlab on GitHub (Jul 14, 2023). ### Feature Description It would be very useful to be able to see changes made in submodules when reviewing a PR, instead of just seeing the changed submodule commit hash. This is supported by git using the `git diff --submodule=diff main` option, which I'm guessing means that the changes needed on the Gitea end would be relatively small: submodule contents would need to be fetched and the diff command tweaked. I imagine this would be an option to enable (per repo, per org or globally?). Example (cleaned for readability) output from `git diff` as it used by Gitea now: ``` README.md: + +And now we've added a sub repo, to see how that works. sub-repo: -Subproject commit 773e95df076c02e982bea7eb745866e92e0cfcfb +Subproject commit 4326ed282c7f5bd49fd4a29c1ac0f414d25d71b9 ``` Example output using `git diff --submodule=diff`: ``` README.md: + +And now we've added a sub repo, to see how that works. sub-repo/README.md: +# Sub repo + +Test av hur Gitea hanterar ändringar i sub-repon. ``` This would make it a lot easier to review all the changes for a PR of the parent repo in a single view, instead of manually having to navigate separate PRs for each submodule. Especially in projects using many submodules, this saves a lot of time and mental effort. You would still need to create PRs for merging the changes for each submodule into their respective main branches, but at least the review can be performed efficiently for the overall work before performing those PRs. ### Screenshots _No response_

GiteaMirror added the type/proposal issue/needs-feedback labels 2025-11-02 09:31:24 -06:00

GiteaMirror closed this issue

2025-11-02 09:31:24 -06:00

GiteaMirror commented

2025-11-02 09:31:25 -06:00

@AdamMajer commented on GitHub (Jul 3, 2024):

I don't believe we would need to fetch submodule content and change the command here. We can just coalesce the diff output between repositories directly.

If there is a submodule, issue a diff to child repository -- it either is on the server, which makes this trivial, or it's not, so we can't do submodule diff then.. We already have the two relevant commit ids from the parent repository. Then replace the paths and append the diffs.

Thing to keep in mind is that these submodules could be recursive and one could even craft an infinite recursion.

I think that having UI option in the diff/commit view to either include the submodule diff or not would be best here.

@AdamMajer commented on GitHub (Jul 3, 2024): I don't believe we would need to fetch submodule content and change the command here. We can just coalesce the diff output between repositories directly. If there is a submodule, issue a diff to child repository -- it either is on the server, which makes this trivial, or it's not, so we can't do submodule diff then.. We already have the two relevant commit ids from the parent repository. Then replace the paths and append the diffs. Thing to keep in mind is that these submodules could be recursive and one could even craft an infinite recursion. I think that having UI option in the diff/commit view to either include the submodule diff or not would be best here.

GiteaMirror commented

2025-11-02 09:31:25 -06:00

@delvh commented on GitHub (Jul 3, 2024):

Yes, your approach sounds like a good idea.
However, we then need the following adjustments:

there is either a timeout, a recursion detection mechanism, or both. The timeout would be the dirty solution. We probably need both.
files that changed in a submodule must be visually distinct from all other files to make it immediately clear that you didn't change them yourself
the same detection would also be needed when viewing any commit, or any subrange of a PR

@delvh commented on GitHub (Jul 3, 2024): Yes, your approach sounds like a good idea. **However**, we then need the following adjustments: - there is either a timeout, a recursion detection mechanism, or both. The timeout would be the dirty solution. We probably need both. - files that changed in a submodule **must** be visually distinct from all other files to make it immediately clear that you didn't change them yourself - the same detection would also be needed when viewing any commit, or any subrange of a PR

GiteaMirror commented

2025-11-02 09:31:26 -06:00

@AdamMajer commented on GitHub (Jul 4, 2024):

Well, you did change them yourself, through updating the commit id for the submodule. But I agree it would be a good idea to have this visually distinct.

If submodule is detected in the diff, we can add a diff submodule menu. For example,

no submodule expansion
1-level submodule expansion
recursive expansion

Similar to the one we have for whitespace, etc.

@AdamMajer commented on GitHub (Jul 4, 2024): Well, you did change them yourself, through updating the commit id for the submodule. But I agree it would be a good idea to have this visually distinct. *If* submodule is detected in the diff, we can add a diff submodule menu. For example, - no submodule expansion - 1-level submodule expansion - recursive expansion Similar to the one we have for whitespace, etc.

GiteaMirror commented

2025-11-02 09:31:26 -06:00

@lunny commented on GitHub (Jan 29, 2025):

After #33097 was merged, reviewing changes has become easier due to submodule reversion improvements. However, since submodules can reference a remote Git server, retrieving the changed files efficiently remains a challenge, as it may introduce network latency and performance issues.

@lunny commented on GitHub (Jan 29, 2025): After #33097 was merged, reviewing changes has become easier due to submodule reversion improvements. However, since submodules can reference a remote Git server, retrieving the changed files efficiently remains a challenge, as it may introduce network latency and performance issues.

GiteaMirror commented

2025-11-02 09:31:26 -06:00

@AdamMajer commented on GitHub (Jan 30, 2025):

Remote fetching should be out of scope here. It can even introduce vectors for vulnerabilities.

@AdamMajer commented on GitHub (Jan 30, 2025): Remote fetching should be out of scope here. It can even introduce vectors for vulnerabilities.

GiteaMirror commented

2025-11-02 09:31:26 -06:00

@wxiaoguang commented on GitHub (Jan 30, 2025):

I think we can mark this issue as "completed" because the review UI provides a "compare/...." link for submodules in #33097 .

I also agree that we shouldn't spend time on fetching remote submodules to compare, it would cause many problems including performance regressions.

@wxiaoguang commented on GitHub (Jan 30, 2025): I think we can mark this issue as "completed" because the review UI provides a "compare/...." link for submodules in #33097 . I also agree that we shouldn't spend time on fetching remote submodules to compare, it would cause many problems including performance regressions. ![image](https://github.com/user-attachments/assets/5fdc8753-ad3e-4363-911f-9b12889b9ab8)

GiteaMirror commented

2025-11-02 09:31:26 -06:00

@AdamMajer commented on GitHub (Jan 30, 2025):

I agree, the minimum requirements looks to be there, so this can be closed as completed. It would be nice to have ability to show these diffs inline without reloading, but that can be done at a later point.

@AdamMajer commented on GitHub (Jan 30, 2025): I agree, the minimum requirements looks to be there, so this can be closed as completed. It would be nice to have ability to show these diffs inline without reloading, but that can be done at a later point.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#11227