diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-12-28 12:05:55 +0000 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2015-12-29 01:34:03 -0500 |
commit | 795a29aee541089a14650e9aed1bdbf718e6e003 (patch) | |
tree | 3a159b4e7aa8f6b4c8d98cfbb68d7ab88df06f38 | |
parent | 92e0722f0f23f8683b8127cd86595d32427cb4e1 (diff) | |
download | gitlab-ce-795a29aee541089a14650e9aed1bdbf718e6e003.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-- | CHANGELOG | 17 | ||||
-rw-r--r-- | app/models/repository.rb | 4 |
2 files changed, 20 insertions, 1 deletions
diff --git a/CHANGELOG b/CHANGELOG index dca688f514e..6ba3bdb2012 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,22 @@ Please view this file on the master branch, on stable branches it's out of date. +v 8.4.0 (unreleased) + - Add support for Google reCAPTCHA in user registration to prevent spammers (Stan Hu) + - Implement new UI for group page + - Implement search inside emoji picker + - Add API support for looking up a user by username (Stan Hu) + - Add project permissions to all project API endpoints (Stan Hu) + - Only allow group/project members to mention `@all` + - Expose Git's version in the admin area + - Add "Frequently used" category to emoji picker + - Add CAS support (tduehr) + - Add link to merge request on build detail page. + - 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 - Fix Error 500 when global milestones have slashes (Stan Hu) - Fix Error 500 when doing a search in dashboard before visiting any project (Stan Hu) diff --git a/app/models/repository.rb b/app/models/repository.rb index 2c25f4ce451..1f4651f14ed 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) |