search

go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
44.36k stars 5.43k forks source link

Branches with same content does not show empty diff #19666

Open jedi7 opened 2 years ago

jedi7 commented 2 years ago

Description

When user creates same content in separate branches. Then the diff shows differences even if the tree content is the same.

See repo https://try.gitea.io/jedi7/test_paralel_branches/src/branch/paralel

Also please see the screenshots

Gitea Version

1.16.7

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

Master branch with "test" content image

Paralel branch with "test" content (added in different commit, so conflicting with master) image

Invalid diff image

Invalid PR image

Git Version

web

Operating System

Archlinux

How are you running Gitea?

https://try.gitea.io/ also reproducible on released gitea 1.16.7

Database

SQLite

Gusted commented 2 years ago

Then the diff shows differences even if the tree content is the same.

The diff is generated based of the older base branch commit. This diff seems to be as intended.

jedi7 commented 2 years ago

The diff is generated based of the older base branch commit. This diff seems to be as intended.

But why it should be intended? When comparing two branches, then user is interested in actual state (not what was before X commits).

Gusted commented 2 years ago

When comparing two branches, then user is interested in actual state (not what was before X commits).

In such case they should update the branch to include the latest change from the base branch. I'm pretty sure this is the same behavior for GitHub+GitLab.

jedi7 commented 2 years ago

@Gusted true, github have the same behavior (just tested on the testing repo) But still not understand how it is useful. when comparing "master" and "paralel" branches.

master content: "test" paralel content: "test" github/gitea diff is "test"

but diff should be "" or at least say the content "test" is conflicting. when I do git diff master paralel , then diff is empty.

Attaching test repo. test-empty-merge-commit-paralel.tar.gz

Gusted commented 2 years ago

when I do git diff master paralel , then diff is empty.

Gitea doesn't use that diff, under-the-hood Gitea requests the merge-base of the paralel with master branch: https://github.com/go-gitea/gitea/blob/main/modules/git/repo_compare.go#L35 and that's the actual commit being used to diff against. In your case the command would return df1d83b5339dfe1155a12e0cc86085e3432addf1 which is the initial commit.

As of why we do this? I'm not entirely sure, but I assume if we would diff against the branch directly, any kind of conflict between the branches would lead to a inaccurate presentation of the diff.

jedi7 commented 2 years ago

Thank you for explanation of the mechanics in gitea, I think I understand now. Question is, if this behavior is ok, or is there space for improve.

From my point of view will be useful this behavior:

Actual behavior is for me little misleading, because for example I can think I'm fixing a bug, but somebody else did it already and I will not see it in diff.

Gusted commented 2 years ago

Question is, if this behavior is ok, or is there space for improve.

I'm not sure if there's space to improve, if it was possible to diff against the branch's HEAD by-default the reverse could happen, code added into the base-branch would now show up as "added" or "removed" in the PR's diff(which we avoid by using the merge-base). To actually have such feature/option we would to merge the branch to not be out-of-date and then do the diff against the branch's HEAD but is a bit misleading and the performance would be degraded especially for very out-of-date PR's where to merge the base branch can take some time.

Actual behavior is for me little misleading, because for example I can think I'm fixing a bug, but somebody else did it already and I will not see it in diff.

Well, Gitea already marks PR's as out-of-date and it's common practice to first merge the latest commit(s) of the base's branch into the PR to avoid such things. But to improve Gitea in such cases, to e.g. show a message if such thing has happpend shouldn't be accomplished by changing how the diff mechanism works, it has no direct benefit and doesn't actually show that it has happen(unless the author has good memory and notices that certain code is missing in the diff).

hramrach commented 11 months ago

It's common but not universal practice to only merge up-to-date branches.

The gitea UI clearly does not support the other option - merging branches that are not up-to-date.

Some forges provide a 'rebase' button that does automated rebase.

Many forges also show if the branch can be merged or if it conflicts in the PR status, and that's a useful feature to have.

To calculate if there is a conflict the merge needs to be done, and then diff of the merge against the branch head can be shown as well.

Eddcapone commented 10 months ago

I was confused too. It would be nice if the "Changed Files" Tab would only show the files, which actually changed. I suggest that you provide atleast another diff option so that only the actual changed files are showing.

You could run a foreach on all files and execute a git diff target feature $filename and if the output is empty, then simply don't show the file.