Show changes in submodules in PR

[mikaeleimantlab]

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

[AdamMajer]

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.

[delvh]

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

[AdamMajer]

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.

[lunny]

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.

[AdamMajer]

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

[wxiaoguang]

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.

[AdamMajer]

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.