diff options
Diffstat (limited to 'doc/development/diffs.md')
| -rw-r--r-- | doc/development/diffs.md | 57 |
1 files changed, 54 insertions, 3 deletions
diff --git a/doc/development/diffs.md b/doc/development/diffs.md index eb070cbf4d7..ece7bb9e9ee 100644 --- a/doc/development/diffs.md +++ b/doc/development/diffs.md @@ -1,3 +1,9 @@ +--- +stage: none +group: unassigned +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +--- + # Working with diffs Currently we rely on different sources to present diffs, these include: @@ -93,7 +99,6 @@ Gitlab::Git::DiffCollection.collection_limits[:max_bytes] = Gitlab::Git::DiffCol No more files will be rendered at all if 5 megabytes have already been rendered. -NOTE: **Note:** All collection limit parameters are currently sent and applied on Gitaly. That is, once the limit is surpassed, Gitaly will only return the safe amount of data to be persisted on `merge_request_diff_files`. @@ -108,7 +113,6 @@ That is, it's equivalent to 10kb if the maximum allowed value is 100kb. The diff will still be persisted and expandable if the patch size doesn't surpass `ApplicationSettings#diff_max_patch_bytes`. -NOTE: **Note:** Although this nomenclature (Collapsing) is also used on Gitaly, this limit is only used on GitLab (hardcoded - not sent to Gitaly). Gitaly will only return `Diff.Collapsed` (RPC) when surpassing collection limits. @@ -123,7 +127,6 @@ Commit::DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines File diff will be suppressed (technically different from collapsed, but behaves the same, and is expandable) if it has more than 5000 lines. -NOTE: **Note:** This limit is currently hardcoded and only applied on GitLab. ## Viewers @@ -132,3 +135,51 @@ Diff Viewers, which can be found on `models/diff_viewer/*` are classes used to m whether it's a binary, which partial should be used to render it or which File extensions this class accounts for. `DiffViewer::Base` validates _blobs_ (old and new versions) content, extension and file type in order to check if it can be rendered. + +## Merge request diffs against the `HEAD` of the target branch + +Historically, merge request diffs have been calculated by `git diff target...source` which compares the +`HEAD` of the source branch with the merge base (or a common ancestor) of the target branch and the source's. +This solution works well until the target branch starts containing some of the +changes introduced by the source branch: Consider the following case, in which the source branch +is `feature_a` and the target is `master`: + +1. Checkout a new branch `feature_a` from `master` and remove `file_a` and `file_b` in it. +1. Add a commit that removes `file_a` to `master`. + +The merge request diff still contains the `file_a` removal while the actual diff compared to +`master`'s `HEAD` has only the `file_b` removal. The diff with such redundant +changes is harder to review. + +In order to display an up-to-date diff, in GitLab 12.9 we +[introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/27008) merge request +diffs compared against `HEAD` of the target branch: the +target branch is artificially merged into the source branch, then the resulting +merge ref is compared to the source branch in order to calculate an accurate +diff. + +Until we complete the epics ["use merge refs for diffs"](https://gitlab.com/groups/gitlab-org/-/epics/854) +and ["merge conflicts in diffs"](https://gitlab.com/groups/gitlab-org/-/epics/4893), +both options `master (base)` and `master (HEAD)` are available to be displayed in merge requests: + + + +The `master (HEAD)` option is meant to replace `master (base)` in the future. + +In order to support comments for both options, diff note positions are stored for +both `master (base)` and `master (HEAD)` versions ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/198457) in 12.10). +The position for `master (base)` version is stored in `Note#position` and +`Note#original_position` columns, for `master (HEAD)` version `DiffNotePosition` +has been introduced. + +One of the key challenges to deal with when working on merge ref diffs are merge +conflicts. If the target and source branch contains a merge conflict, the branches +cannot be automatically merged. The [recording on +YouTube](https://www.youtube.com/watch?v=GFXIFA4ZuZw&feature=youtu.be&ab_channel=GitLabUnfiltered) +is a quick introduction to the problem and the motivation behind the [epic](https://gitlab.com/groups/gitlab-org/-/epics/854). + +In 13.5 a solution for both-modified merge +conflict has been +[introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/232484). However, +there are more classes of merge conflicts that are to be +[addressed](https://gitlab.com/groups/gitlab-org/-/epics/4893) in the future. |