Skip to content

Renamed or deleted images do not render in diffs #18861

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.

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

Closed
cboylan opened this issue Feb 22, 2022 · 3 comments
Closed

Renamed or deleted images do not render in diffs #18861

cboylan opened this issue Feb 22, 2022 · 3 comments
Labels
Milestone

Comments

@cboylan
Copy link
Contributor

cboylan commented Feb 22, 2022

Gitea Version

1.16.1

Git Version

git version 2.30.2

Operating System

Debian Bullseye

How are you running Gitea?

We build our own Docker images using Debian Bullseye. This approach was adapted from the official upstream images that use Alpine. The Dockerfile used to build Gitea 1.16.1 is visible here: https://opendev.org/opendev/system-config/src/commit/f3b767cf60d58be979ba6bb61112907032912fb5/docker/gitea/Dockerfile. Note this is a Gitea 1.15.11 instance separate from the 1.16.1 instance that we are testing.

Database

MySQL

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Description

When viewing diffs for files that have been renamed viewing the raw data for the old file name is a 404. When viewing diffs for files that have been deleted the raw data for the file is a 404. In both cases this seems to trip the diff rendering of the commit for that file because one side of the diff 404s. In these situations we get a spinning circle rather than the diff as expected. Older gitea (1.15.11 for example) seems to handle this appropriately. In the case of deletions showing only the old side of the data and in the cases of renames finding both versions of the file.

If you click on view file for the renamed files the file loads fine directly outside of the diff view. The server logs indicate:

...s/context/context.go:290:PlainTextBytes() [E] PlainTextBytes: Not found.

I think the issue is that when the diff viewer is unable to find both sides of the diff it fails now. Previously it was able to find the old and new file content for renamed files and it ignored missing data for deleted files. It only showed the before side and not the after side for deleted files.

I believe that https://try.gitea.io/fx6904/gitea/commit/7cf23399a0b97cb8211e8e7b00020e9b1a5fdac1 shows this on the demo site for public/img/favicon.ico. This file appears to have been deleted so the new side of the diff is a 404 and the view just spins.

It is possible that this is also a problem for text files, but I haven't looked into that yet.

This issue #18811 is related in that it describes a new method of handling deleted files in diffs. I think showing that the file has been deleted instead of trying to diff it would be fine. However, that doesn't handle the renamed file case.

Screenshots

Here is Gitea 1.16.1 attempting to diff renamed and deleted files for us:

gitea1 16 1_spin_diff

And this shows the old Gitea 1.15.11 behavior:

gitea1 15 11_spin_diff

@cboylan
Copy link
Contributor Author

cboylan commented Feb 22, 2022

I've checked and renamed/deleted text files do not suffer from this issue. It seems related to more complex file types like the SVG and PNGs in my example.

@silverwind silverwind modified the milestone: 1.16.3 Feb 23, 2022
@silverwind
Copy link
Member

Yes, this is definitely a bug in the image diff.

@KN4CK3R
Copy link
Member

KN4CK3R commented Feb 24, 2022

Is fixed with #18799

@lunny lunny closed this as completed Feb 28, 2022
@lunny lunny added this to the 1.16.3 milestone Feb 28, 2022
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants