Github Web UI shows weblate PRs strangely (unexpected commits are listed)

[jepler]

Describe the bug

In our project, we prefer to have weblate use the "merge" style to incorporate changes from our main development branch. Unfortunately, github ends up showing commits already present in the main branch within the pull request! Here's one such pull request in our project's history: adafruit/circuitpython#3792

To Reproduce the bug

Here's the configuration and sequence of events that seem to happen:

1. In Hosted Weblate, configure a github project to "Merge style: Merge"
2. Create some translations and create a commit, which will create a github PR
3. Make some changes on the main branch by merging other PRs
4. Look at the PR created in step 2

Expected behavior

Only commits created by weblate are shown, not commits and changes from the main branch.

** Actual behavior **

Commits (and their changes) created by merging other PRs are visible in Weblate's PR

Screenshots

See adafruit/circuitpython#3792

Server configuration and status

Weblate installation: Hosted weblate

Additional context

I first assumed this was a Github problem, but I manually created a PR adafruit/circuitpython#3827 that goes through the same steps and github displays the PR as I expect, without listing commits from the main branch and the changes in those commits. So the next hypothesis is that something is different about a weblate git merge than the one I did locally. However, I can't see anything specifically different in git show --format=fuller. Pulling down the PR branch locally and looking at git diff and git stat, everything looks right. So I'm puzzled, and still not 100% sure it's Weblate that is "in the wrong" here. But this is negatively affecting our ability to review & approve translation PRs so I wanted to raise the issue here.

[nijel]

It indeed looks wrong. Were there any force pushes to the repo? That would explain that GitHub doesn't match the commits which are already part of the upstream repo.

[github-actions]

This issue looks more like a support question than an issue. We strive to answer these reasonably fast, but purchasing the support subscription is not only more responsible and faster for your business but also makes Weblate stronger. In case your question is already answered, making a donation is the right way to say thank you!

[jepler]

No, we do not force-push to our main branch, and only weblate pushes to its PR branch.

Adafruit are paid users on hosted weblate, thank you very much, we're thrilled with the service.

[nijel]

Sorry for asking, the force pushes are usual suspects in this situation, so I had to ask about that.

Looking at the merge commit, the extra commits were not there:

$ git log --oneline d07629665..b810b384c9795ae1b514aef451e9952115cf2395
b810b384c Merge pull request #3792 from weblate/weblate-circuitpython-main
9fd652111 Update translation files
f267c327b Merge branch 'origin/main' into Weblate.
2c4b1f251 Merge branch 'origin/main' into Weblate.
1bacfacfb Merge branch 'origin/main' into Weblate.
b98da3d89 Merge branch 'origin/main' into Weblate.
9acdd91c9 Merge branch 'origin/main' into Weblate.
54fcf127c Merge branch 'origin/main' into Weblate.
4becc00a7 Merge branch 'origin/main' into Weblate.
8d9d53a07 Translated using Weblate (Portuguese (Brazil))
8b98867f0 Merge branch 'origin/main' into Weblate.
6af532f4f Merge branch 'origin/main' into Weblate.
cea6c3d8d Update translation files
fea25c1c5 Merge branch 'origin/main' into Weblate.
a3fc20df0 Merge branch 'origin/main' into Weblate.
0ff0bb9ec Translated using Weblate (French)
f26100408 Merge branch 'origin/main' into Weblate.
a31fc3b08 Merge branch 'origin/main' into Weblate.
3b5be929e Merge branch 'origin/main' into Weblate.
458f55714 Merge branch 'origin/main' into Weblate.
4e7668d69 Merge branch 'origin/main' into Weblate.
ac8a9625d Translated using Weblate (Swedish)
bb1b2bd78 Translated using Weblate (Swedish)
e58c3852f Merge branch 'origin/main' into Weblate.
11d829e20 Translated using Weblate (Czech)
db04582af Translated using Weblate (Portuguese (Brazil))

So, this really boils down to figuring out why GitHub interprets it the way it did. Maybe the merge commits in between did confuse it. We already have custom merge logic to avoid this, but maybe GitHub implementation has changed meanwhile. The merge logic is here:

weblate/weblate/vcs/git.py

Lines 158 to 180 in aeede8d

	# We don't do simple git merge origin/branch as that leads
	# to different merge order than expected and most GUI tools
	# then show confusing diff (not changes done by Weblate, but
	# changes merged into Weblate)
	remote = self.get_remote_branch_name()
	# Create local branch for upstream
	self.execute(["branch", tmp, remote])
	# Checkout upstream branch
	self.execute(["checkout", tmp])
	# Merge current Weblate changes, this can lead to conflict
	cmd = [
	"merge",
	"--message",
	message or f"Merge branch '{remote}' into Weblate",
	]
	cmd.extend(self.get_gpg_sign_args())
	cmd.append(self.branch)
	self.execute(cmd)
	# Checkout branch with Weblate changes
	self.execute(["checkout", self.branch])
	# Merge temporary branch (this is fast forward so does not create
	# merge commit)
	self.execute(["merge", tmp])

You might want to switch to using rebase instead of merge in Weblate, that usually works well with pull requests (we're using it on Weblate as well).

[github-actions]

This issue has been automatically marked as stale because it has not had any recent activity.

It will be closed soon if no further action occurs.

Thank you for your contributions!

[tannewt]

You might want to switch to using rebase instead of merge in Weblate, that usually works well with pull requests (we're using it on Weblate as well).

We want to move off of rebase because our CI is slow (>1 hour). Merge allows us to see the CI history of the branch and see if it passed previously.

Are you sure 14294e4 is still correct and not just a fix against a github bug?

Merging the weblate changes into the upstream branch seems incorrect to me. Usually updating a feature branch is done by merging the main branch into it. (The order matters because the first parent is treated differently than the second.)

[nijel]

No, I'm not certain, but I think there are two different things here. The above commit addressed merge commit happening inside Weblate to show that Weblate changes are being merged into master and that IMHO works correctly (see for example adafruit/circuitpython@e58c385).

The root cause probably is how does the final merging happen:

• In case Weblate changes are fast-forwarded, the outcome from 14294e4 is expected as it will show you Weblate changes while looking on the merge commit.
• In case Weblate changes are merged with a merge commit, the result merge commit is probably showing wrong diff similarly as it happens with your pull request.

Unfortunately, GitHub doesn't support fast-forward merges in the UI (GitLab does), so most people merging on GitHub will get it wrong. What we can do is using simple merge when GitHub integration is used...

[tannewt]

Oh interesting! I didn't realize it was the PR merge direction that factored into it. Yes, we always merge using GitHub in our repo so that could be the issue.

[github-actions]

Thank you for your report, the issue you have reported has just been fixed.

• In case you see a problem with the fix, please comment on this issue.
• In case you see a similar problem, please open a separate issue.
• If you are happy with the outcome, don’t hesitate to support Weblate by making a donation.

[tannewt]

Thanks again for fixing this! It's been much more pleasant to review the PRs from Weblate now.