summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2015-12-28 12:05:55 +0000
committerDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2015-12-28 12:05:55 +0000
commit08b4d8b6acf1698e0a5d1509efd1af2141bb9840 (patch)
treec100a18f00832e0e9a601901168791c4883f954e
parent2978be2e2224c7886494f0d4395bd1da5d4a0652 (diff)
parentff8cd116a04d187f63222b3b29ebb6818dfd65e5 (diff)
downloadgitlab-ce-08b4d8b6acf1698e0a5d1509efd1af2141bb9840.tar.gz
Merge branch 'disable-git-follow' into 'master'
Disable --follow in `git log` to avoid loading duplicate commit data in infinite scroll `git` doesn't work properly when `--follow` and `--skip` are specified together. We could even be **omitting commits in the Web log** as a result. Here are the gory details. Let's say you ran: ``` git log -n=5 --skip=2 README ``` This is the working case since it omits `--follow`. This is what happens: 1. `git` starts at `HEAD` and traverses down the tree until it finds the top-most commit relevant to README. 2. Once this is found, this commit is returned via `get_revision_1()`. 3. If the `skip_count` is positive, decrement and repeat step 2. Otherwise go onto step 4. 4. `show_log()` gets called with that commit. 5. Repeat step 1 until we have all five entries. That's exactly what we want. What happens when you use `--follow`? You have to understand how step 1 is performed: * When you specify a pathspec on the command-line (e.g. README), a flag `prune` [gets set here](https://github.com/git/git/blob/master/revision.c#L2351). * If the `prune` flag is active, `get_commit_action()` determines whether the commit should be [scanned for matching paths](https://github.com/git/git/blob/master/revision.c#L2989). * In the case of `--follow`, however, `prune` is [disabled here](https://github.com/git/git/blob/master/revision.c#L2350). * As a result, a commit is never scanned for matching paths and therefore never pruned. `HEAD` will always get returned as the first commit, even if it's not relevant to the README. * Making matters worse, the `--skip` in the example above would actually skip a every other entry after `HEAD` N times. If README were changed in these skipped commits, we would actually miss information! Since git uses a matching algorithm to determine whether a file was renamed, I believe `git` needs to generate a diff of each commit to do this and traverse each commit one-by-one to do this. I think that's the rationale for disabling the `prune` functionality since you can't just do a simple string comparison. Closes #4181, #4229, #3574, #2410 See merge request !2210
-rw-r--r--CHANGELOG1
-rw-r--r--app/models/repository.rb4
2 files changed, 4 insertions, 1 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 39545c4a15c..57f0b9f30d5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,7 @@ v 8.4.0 (unreleased)
- Revert back upvote and downvote button to the issue and MR pages
v 8.3.2 (unreleased)
+ - Disable --follow in `git log` to avoid loading duplicate commit data in infinite scroll (Stan Hu)
- Enable "Add key" button when user fills in a proper key
v 8.3.1
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 9f688e3b45b..a9bf4eb4033 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -76,7 +76,9 @@ class Repository
path: path,
limit: limit,
offset: offset,
- follow: path.present?
+ # --follow doesn't play well with --skip. See:
+ # https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520
+ follow: false
}
commits = Gitlab::Git::Commit.where(options)