Optimize pulls merging

[typeless]

This is yet-another attempt to fix #601.

It passes the integration tests but not sure if it is sufficiently covered.

Benchmarking is not trivial to be automated within the existing testing framework because

1. Adding a large test repo inside Gitea's integration facility is not desirable.
2. Cloning an existing large repo from somewhere on the internet is very time-consuming and not appropriate to be run repeatedly.
3. Testing small repo seems to be unreliable and I keep getting interrupted by 'database table is locked' along the way when looping through the pull merging test.

I still published this patch in hopes that someone can find any flaws early.

[codecov-io]

Codecov Report

Merging #4921 into master will increase coverage by 0.03%.
The diff coverage is 47.82%.

@@            Coverage Diff             @@
##           master    #4921      +/-   ##
==========================================
+ Coverage   37.88%   37.92%   +0.03%     
==========================================
  Files         328      328              
  Lines       48084    48141      +57     
==========================================
+ Hits        18219    18256      +37     
- Misses      27246    27257      +11     
- Partials     2619     2628       +9

Impacted Files	Coverage Δ
models/pull.go	51.04% <47.82%> (+0.39%)	⬆️
models/repo_list.go	63.29% <0%> (-1.27%)	⬇️
routers/repo/view.go	47.3% <0%> (+1.19%)	⬆️
models/unit.go	14.28% <0%> (+14.28%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d43437...5c56a2a. Read the comment docs.

[lunny]

Maybe we could create a docker to contain some popluar git repos for testing with Gitea and publish it to dockhub.

[sapk]

@typeless You can have a look at https://github.com/go-gitea/gitea/blob/801843b0115e29ba2304fa6a5bea1ae169a58e02/integrations/benchmarks_test.go wich introduce a suite of repos and manage short flag for quick test. Benchmark are not currently run on CI so it does not have an impact on test time. You can use short flag when testing/dev and validate only at end by running all benchmarks.

[filipnavara]

I gave it a try, but "Rebase and merge" caused this:

2018/10/17 20:17:15 [...routers/repo/pull.go:579 MergePullRequest()] [E] Merge: git rebase [E:\Git\emclient\emclient.git -> E:/Tools/Gitea/data/tmp/local-repo/merge-387491500.git]: Cannot rebase: You have unstaged changes.
Please commit or stash them.

Adding the -s alone to git clone does help quite a bit though.

[typeless]

@filipnavara Is there a simple way to reproduce it?

[filipnavara]

Well, happened on very first attempt I tried. As far as I know git rebase works only when there is a checkout, so I would expect it to happen every time. However, if you think it may be more complicated than that I will be happy to share some test repo+configuration.

[typeless]

@filipnavara There is probably a bug. However, the code path following the git-fetch and git-read-tree does have done checkouts (see https://github.com/go-gitea/gitea/pull/4921/files#diff-ca95a2152012afcd9192d0ed961e6b9dL389).

Here is also a demo:

#!/bin/sh
set -e 
set -x
#rm -rf ./work
mkdir -p work && pushd work || exit 1

	mkdir -p repo1
	pushd repo1
		git init
		echo "Hello, " > README.md
		git add .
		git commit -m"Initial commit"
	popd
	
	git clone repo1 repo2
	
	pushd repo1
		echo "World" >> README.md
		git add -u
		git commit -m"Update"
	popd
	
	pushd repo2
		echo "Alice" >> README.md
		git add -u
		git commit -m"Add Alice"
	popd

	git clone -s --no-checkout -b master repo1 repo-tmp
	pushd repo-tmp
		git read-tree master
		git fetch ../repo2 master
		git checkout FETCH_HEAD

		## We don't care about conflits for this demo, so always choose 'ours'
		git rebase -s ours master
	popd
popd

This is a demo for merge, which does not require a checkout

#!/bin/sh
set -e 
set -x
#rm -rf ./work
mkdir -p work && pushd work || exit 1

	mkdir -p repo1
	pushd repo1
		git init
		echo "Hello, " > README.md
		git add .
		git commit -m"Initial commit"
	popd
	
	git clone repo1 repo2
	
	pushd repo1
		echo "World" >> README.md
		git add -u
		git commit -m"Update"
	popd
	
	pushd repo2
		echo "Alice" >> README.md
		git add -u
		git commit -m"Add Alice"
	popd

	git clone -s --no-checkout -b master repo1 repo-tmp
	pushd repo-tmp
		git read-tree master
		git fetch ../repo2 master

		## git-merge works without doing git-chekcout
		git merge --no-edit -X theirs FETCH_HEAD
	popd
popd

[typeless]

Fixed various bugs.

@filipnavara the problem you encountered should be fixed.

[typeless]

Can anyone help review this?

I tested it manually (not very systematically though), and it is capable of merging an 11GiB repo (6.2GiB bare) now.

[filipnavara]

I'll try to deploy it on our server and report back. Thanks

[typeless]

@filipnavara Thanks. I have deployed to our server too, and deliberately doing more commits 😄
If your repositories exploded, you would not be alone.

[zeripath]

@typeless - these changes look good to me, but see my comments in #601.

[filipnavara]

Yes

[typeless]

@filipnavara I just tested on our production server using the rebase merge-style for a 10+ GB repo.
The pull merge took less than 3 secs (not really measured it). But I did notice that when I pressed the "green" button to create the PR in the first place, it hung for a while in which it was doing git -c credential.helper= remote add -f 1547120927403349389 /home/git/git-repositories/my-org/my-huge-repo.git.

That was probably the culprit.

Edit: But it seems what you encountered was about merging the PR, not creation of it.

[filipnavara]

@typeless On further inspections the "timeouts" were caused by an unrelated issue in the code that sends out notifications. I don't have exact performance data, but this PR makes it good enough for our general usage (~ 6Gb repository at the moment).

[typeless]

Given that 1.7.0 is released, please consider merging this.

[typeless]

Need an LGTM to proceed.