Side by side git-range-diff

published (revision history)

git range-diff compares two commit ranges (two versions of a branch, e.g. before and after a rebase). Its output is difficult to comprehend, though. It’s a diff between diffs, presented in one dimension using two columns of pluses/​minuses/​spaces. Wouldn’t it be better if we used two dimensions and some nice colors?

Table of Contents

Motivation/Example

To illustrate my point, let’s take a work-in-progress1 pull request from xmonad-contrib, git rebase it to autosquash the fixup commits, and then use git range-diff to see what the rebase actually did:

$ gh pr checkout 836
$ git checkout -b wx-partial-rebase
$ git rebase -i --keep-base --autosquash origin/master
$ git range-diff wx-partial...wx-partial-rebase
git range-diff
1:  05a5291e ! 1:  d5c45f98 X.Prelude: Add infinite stream type
    @@ XMonad/Prelude.hs: multimediaKeys = filter ((/= noSymbol) . snd) . map (id &&& s
     +  type (Item (Stream a)) = a
     +
     +  fromList :: [a] -> Stream a
    -+  fromList []       = errorWithoutStackTrace "TODO"
     +  fromList (x : xs) = x :~ fromList xs
    ++  fromList []       = errorWithoutStackTrace "XMonad.Prelude.Stream.fromList: Can't create stream out of finite list."
     +
     +  toList :: Stream a -> [a]
     +  toList (x :~ xs) = x : toList xs
2:  9dfe5594 = 2:  a11625d9 X.L.Groups: Rewrite gen using infinite streams
3:  c8a08103 = 3:  06855306 Import X.Prelude unqualified if necessary
4:  90f16434 = 4:  2de422fe Reduce head usage
5:  00a457fd < -:  -------- fixup! X.Prelude: Add infinite stream type

This is a simple example—neither the old (05a5291e) nor new (d5c45f98) commits remove any lines, so the inner column is always +. Some of you can therefore figure out that the difference is that the old commit adds a line with “TODO” in it, while the new adds a line (somewhere else!) with a meaningful error message instead.

Now let’s look at it side by side (in 2 dimensions):

$ git si-range-diff wx-partial...wx-partial-rebase

(note: si-range-diff is my alias; don’t try this at home—just yet)

git si-range-diff
────────────────────────────────────────────────────────────────────────────────────────────────────────────
1:  05a5291e ! 1:  d5c45f98 X.Prelude: Add infinite stream type

──────────────────────────────────────────────────────────────────────────────────
 XMonad/Prelude.hs: multimediaKeys = filter ((/= noSymbol) . snd) . map (id &&& s 
──────────────────────────────────────────────────────────────────────────────────
+  type (Item (Stream a)) = a                         +  type (Item (Stream a)) = a
+                                                     +
+  fromList :: [a] -> Stream a                        +  fromList :: [a] -> Stream a
+  fromList []       = errorWithoutStackTrace "TODO"  
+  fromList (x : xs) = x :~ fromList xs               +  fromList (x : xs) = x :~ fromList xs
                                                      +  fromList []       = errorWithoutStackTrace "XMon
                                                      ad.Prelude.Stream.fromList: Can't create stream out
                                                       of finite list."
+                                                     +
+  toList :: Stream a -> [a]                          +  toList :: Stream a -> [a]
+  toList (x :~ xs) = x : toList xs                   +  toList (x :~ xs) = x : toList xs
────────────────────────────────────────────────────────────────────────────────────────────────────────────
2:  9dfe5594 = 2:  a11625d9 X.L.Groups: Rewrite gen using infinite streams

────────────────────────────────────────────────────────────────────────────────────────────────────────────
3:  c8a08103 = 3:  06855306 Import X.Prelude unqualified if necessary

────────────────────────────────────────────────────────────────────────────────────────────────────────────
4:  90f16434 = 4:  2de422fe Reduce head usage

────────────────────────────────────────────────────────────────────────────────────────────────────────────
5:  00a457fd < -:  -------- fixup! X.Prelude: Add infinite stream type

Much better. It’s easier to see what’s going on here. Left side is the old diff, right side is the new one. Both are syntax-highlighted using foreground (text) colours. The inter-diff diff is syntax-highlighted using background colours—the removed line on the left side is dark red, the added one on the right side is darkish green.

Here’s a longer example from the Linux kernel, which I believe requires superhuman abilities to understand (made more difficult by the ## and @@ lines appearing in the context of neighbouring diff hunks):

Expand/Collapse
git range-diff
1:  81f8e8008c9a ! 1:  05fce90455a2 mm: abstract the vma_merge()/split_vma() pattern for mprotect() et al.
    @@ Commit message
         parameters, create inline wrapper functions for each of the modify
         operations, parameterised only by what is required to perform the action.
     
    +    We can also significantly simplify the logic - by returning the VMA if we
    +    split (or merged VMA if we do not) we no longer need specific handling for
    +    merge/split cases in any of the call sites.
    +
         Note that the userfaultfd_release() case works even though it does not
         split VMAs - since start is set to vma->vm_start and end is set to
         vma->vm_end, the split logic does not trigger.
    @@ Commit message
         - vma->vm_start) >> PAGE_SHIFT, and start - vma->vm_start will be 0 in this
         instance, this invocation will remain unchanged.
     
    -    Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
    +    We eliminate a VM_WARN_ON() in mprotect_fixup() as this simply asserts that
    +    vma_merge() correctly ensures that flags remain the same, something that is
    +    already checked in is_mergeable_vma() and elsewhere, and in any case is not
    +    specific to mprotect().
    +
         Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
    +    Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
     
      ## fs/userfaultfd.c ##
     @@ fs/userfaultfd.c: static int userfaultfd_release(struct inode *inode, struct file *file)
    @@ fs/userfaultfd.c: static int userfaultfd_release(struct inode *inode, struct fil
     -				 vma->vm_file, vma->vm_pgoff,
     -				 vma_policy(vma),
     -				 NULL_VM_UFFD_CTX, anon_vma_name(vma));
    -+		prev = vma_modify_flags_uffd(&vmi, prev, vma, vma->vm_start,
    -+					     vma->vm_end, new_flags,
    -+					     NULL_VM_UFFD_CTX);
    +-		if (prev) {
    +-			vma = prev;
    +-		} else {
    +-			prev = vma;
    +-		}
    ++		vma = vma_modify_flags_uffd(&vmi, prev, vma, vma->vm_start,
    ++					    vma->vm_end, new_flags,
    ++					    NULL_VM_UFFD_CTX);
    + 
    + 		vma_start_write(vma);
    + 		userfaultfd_set_vm_flags(vma, new_flags);
    + 		vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
     +
    - 		if (prev) {
    - 			vma = prev;
    - 		} else {
    ++		prev = vma;
    + 	}
    + 	mmap_write_unlock(mm);
    + 	mmput(mm);
     @@ fs/userfaultfd.c: static int userfaultfd_register(struct userfaultfd_ctx *ctx,
      	unsigned long start, end, vma_end;
      	struct vma_iterator vmi;
    @@ fs/userfaultfd.c: static int userfaultfd_register(struct userfaultfd_ctx *ctx,
     -			/* vma_merge() invalidated the mas */
     -			vma = prev;
     -			goto next;
    -+		prev = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end,
    -+					     new_flags,
    -+					     (struct vm_userfaultfd_ctx){ctx});
    -+		if (IS_ERR(prev)) {
    -+			ret = PTR_ERR(prev);
    ++
    ++		vma = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end,
    ++					    new_flags,
    ++					    (struct vm_userfaultfd_ctx){ctx});
    ++		if (IS_ERR(vma)) {
    ++			ret = PTR_ERR(vma);
     +			break;
      		}
     -		if (vma->vm_start < start) {
    @@ fs/userfaultfd.c: static int userfaultfd_register(struct userfaultfd_ctx *ctx,
     -				break;
     -		}
     -	next:
    -+
    -+		if (prev)
    -+			vma = prev; /* vma_merge() invalidated the mas */
     +
      		/*
      		 * In the vma_merge() successful mprotect-like case 8:
    @@ fs/userfaultfd.c: static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
     -				 vma_policy(vma),
     -				 NULL_VM_UFFD_CTX, anon_vma_name(vma));
     -		if (prev) {
    -+		prev = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end,
    -+					     new_flags, NULL_VM_UFFD_CTX);
    -+		if (IS_ERR(prev)) {
    +-			vma = prev;
    +-			goto next;
    ++		vma = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end,
    ++					    new_flags, NULL_VM_UFFD_CTX);
    ++		if (IS_ERR(vma)) {
     +			ret = PTR_ERR(prev);
     +			break;
    -+		}
    -+
    -+		if (prev)
    - 			vma = prev;
    --			goto next;
    --		}
    + 		}
     -		if (vma->vm_start < start) {
     -			ret = split_vma(&vmi, vma, start, 1);
     -			if (ret)
    @@ fs/userfaultfd.c: static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
     -				break;
     -		}
     -	next:
    ++
      		/*
      		 * In the vma_merge() successful mprotect-like case 8:
      		 * the next vma was merged into the current one and
    @@ mm/madvise.c: static int madvise_update_vma(struct vm_area_struct *vma,
      	struct mm_struct *mm = vma->vm_mm;
      	int error;
     -	pgoff_t pgoff;
    -+	struct vm_area_struct *merged;
      	VMA_ITERATOR(vmi, mm, start);
      
      	if (new_flags == vma->vm_flags && anon_vma_name_eq(anon_vma_name(vma), anon_name)) {
    @@ mm/madvise.c: static int madvise_update_vma(struct vm_area_struct *vma,
     -		vma = *prev;
     -		goto success;
     -	}
    -+	merged = vma_modify_flags_name(&vmi, *prev, vma, start, end, new_flags,
    -+				       anon_name);
    -+	if (IS_ERR(merged))
    -+		return PTR_ERR(merged);
    ++	vma = vma_modify_flags_name(&vmi, *prev, vma, start, end, new_flags,
    ++				    anon_name);
    ++	if (IS_ERR(vma))
    ++		return PTR_ERR(vma);
      
    --	*prev = vma;
    -+	if (merged)
    -+		vma = *prev = merged;
    -+	else
    -+		*prev = vma;
    + 	*prev = vma;
      
     -	if (start != vma->vm_start) {
     -		error = split_vma(&vmi, vma, start, 1);
    @@ mm/madvise.c: static int madvise_update_vma(struct vm_area_struct *vma,
     
      ## mm/mempolicy.c ##
     @@ mm/mempolicy.c: static int mbind_range(struct vma_iterator *vmi, struct vm_area_struct *vma,
    + 		struct vm_area_struct **prev, unsigned long start,
    + 		unsigned long end, struct mempolicy *new_pol)
      {
    - 	struct vm_area_struct *merged;
    +-	struct vm_area_struct *merged;
      	unsigned long vmstart, vmend;
     -	pgoff_t pgoff;
     -	int err;
    @@ mm/mempolicy.c: static int mbind_range(struct vma_iterator *vmi, struct vm_area_
     -	merged = vma_merge(vmi, vma->vm_mm, *prev, vmstart, vmend, vma->vm_flags,
     -			 vma->anon_vma, vma->vm_file, pgoff, new_pol,
     -			 vma->vm_userfaultfd_ctx, anon_vma_name(vma));
    -+	merged =  vma_modify_policy(vmi, *prev, vma, vmstart, vmend, new_pol);
    -+	if (IS_ERR(merged))
    -+		return PTR_ERR(merged);
    -+
    - 	if (merged) {
    - 		*prev = merged;
    - 		return vma_replace_policy(merged, new_pol);
    - 	}
    - 
    +-	if (merged) {
    +-		*prev = merged;
    +-		return vma_replace_policy(merged, new_pol);
    +-	}
    +-
     -	if (vma->vm_start != vmstart) {
     -		err = split_vma(vmi, vma, vmstart, 1);
     -		if (err)
    @@ mm/mempolicy.c: static int mbind_range(struct vma_iterator *vmi, struct vm_area_
     -		if (err)
     -			return err;
     -	}
    --
    ++	vma =  vma_modify_policy(vmi, *prev, vma, vmstart, vmend, new_pol);
    ++	if (IS_ERR(vma))
    ++		return PTR_ERR(vma);
    + 
      	*prev = vma;
      	return vma_replace_policy(vma, new_pol);
    - }
     
      ## mm/mlock.c ##
     @@ mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,
    @@ mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_stru
      	int nr_pages;
      	int ret = 0;
      	vm_flags_t oldflags = vma->vm_flags;
    -+	struct vm_area_struct *merged;
    - 
    - 	if (newflags == oldflags || (oldflags & VM_SPECIAL) ||
    - 	    is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm) ||
     @@ mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,
      		/* don't set VM_LOCKED or VM_LOCKONFAULT and don't count */
      		goto out;
    @@ mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_stru
     -	if (*prev) {
     -		vma = *prev;
     -		goto success;
    -+	merged = vma_modify_flags(vmi, *prev, vma, start, end, newflags);
    -+	if (IS_ERR(merged)) {
    -+		ret = PTR_ERR(merged);
    ++	vma = vma_modify_flags(vmi, *prev, vma, start, end, newflags);
    ++	if (IS_ERR(vma)) {
    ++		ret = PTR_ERR(vma);
     +		goto out;
      	}
      
    @@ mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_stru
     -		if (ret)
     -			goto out;
     -	}
    -+	if (merged)
    -+		vma = *prev = merged;
    - 
    +-
     -	if (end != vma->vm_end) {
     -		ret = split_vma(vmi, vma, end, 0);
     -		if (ret)
    @@ mm/mmap.c: int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
     + *
     + * If no merge is possible and the range does not span the entirety of the VMA,
     + * we then need to split the VMA to accommodate the change.
    ++ *
    ++ * The function returns either the merged VMA, the original VMA if a split was
    ++ * required instead, or an error if the split failed.
     + */
     +struct vm_area_struct *vma_modify(struct vma_iterator *vmi,
     +				  struct vm_area_struct *prev,
    @@ mm/mmap.c: int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
     +			return ERR_PTR(err);
     +	}
     +
    -+	return NULL;
    ++	return vma;
     +}
     +
      /*
    @@ mm/mprotect.c: mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
      	unsigned int mm_cp_flags = 0;
      	unsigned long charged = 0;
     -	pgoff_t pgoff;
    -+	struct vm_area_struct *merged;
      	int error;
      
      	if (newflags == oldflags) {
    @@ mm/mprotect.c: mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
     -			   vma->vm_userfaultfd_ctx, anon_vma_name(vma));
     -	if (*pprev) {
     -		vma = *pprev;
    -+	merged = vma_modify_flags(vmi, *pprev, vma, start, end, newflags);
    -+	if (IS_ERR(merged)) {
    -+		error = PTR_ERR(merged);
    -+		goto fail;
    -+	}
    -+
    -+	if (merged) {
    -+		vma = *pprev = merged;
    - 		VM_WARN_ON((vma->vm_flags ^ newflags) & ~VM_SOFTDIRTY);
    +-		VM_WARN_ON((vma->vm_flags ^ newflags) & ~VM_SOFTDIRTY);
     -		goto success;
    -+	} else {
    -+		*pprev = vma;
    ++	vma = vma_modify_flags(vmi, *pprev, vma, start, end, newflags);
    ++	if (IS_ERR(vma)) {
    ++		error = PTR_ERR(vma);
    ++		goto fail;
      	}
      
    --	*pprev = vma;
    --
    + 	*pprev = vma;
    + 
     -	if (start != vma->vm_start) {
     -		error = split_vma(vmi, vma, start, 1);
     -		if (error)
git si-range-diff
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
1:  81f8e8008c9a ! 1:  05fce90455a2 mm: abstract the vma_merge()/split_vma() pattern for mprotect() et al.

────────────────
 Commit message 
────────────────
    parameters, create inline wrapper functions for each of the modify                  parameters, create inline wrapper functions for each of the modify
    operations, parameterised only by what is required to perform the action.           operations, parameterised only by what is required to perform the action.
                                                                                    
                                                                                        We can also significantly simplify the logic - by returning the VMA if we
                                                                                        split (or merged VMA if we do not) we no longer need specific handling for
                                                                                        merge/split cases in any of the call sites.
                                                                                    
    Note that the userfaultfd_release() case works even though it does not              Note that the userfaultfd_release() case works even though it does not
    split VMAs - since start is set to vma->vm_start and end is set to                  split VMAs - since start is set to vma->vm_start and end is set to
    vma->vm_end, the split logic does not trigger.                                      vma->vm_end, the split logic does not trigger.

────────────────
 Commit message 
────────────────
    - vma->vm_start) >> PAGE_SHIFT, and start - vma->vm_start will be 0 in this         - vma->vm_start) >> PAGE_SHIFT, and start - vma->vm_start will be 0 in this
    instance, this invocation will remain unchanged.                                    instance, this invocation will remain unchanged.
                                                                                    
    Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>                             
                                                                                        We eliminate a VM_WARN_ON() in mprotect_fixup() as this simply asserts that
                                                                                        vma_merge() correctly ensures that flags remain the same, something that is
                                                                                        already checked in is_mergeable_vma() and elsewhere, and in any case is not
                                                                                        specific to mprotect().
                                                                                    
    Reviewed-by: Vlastimil Babka <vbabka@suse.cz>                                       Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
                                                                                        Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
                                                                                    
 ## fs/userfaultfd.c ##                                                              ## fs/userfaultfd.c ##
@@ fs/userfaultfd.c: static int userfaultfd_release(struct inode *inode, struct f  @@ fs/userfaultfd.c: static int userfaultfd_release(struct inode *inode, struct f
                                                                       ile *file)                                                                         ile *file)

──────────────────────────────────────────────────────────────────────────────────
 fs/userfaultfd.c: static int userfaultfd_release(struct inode *inode, struct fil 
──────────────────────────────────────────────────────────────────────────────────
-                 vma->vm_file, vma->vm_pgoff,                                      -                 vma->vm_file, vma->vm_pgoff,
-                 vma_policy(vma),                                                  -                 vma_policy(vma),
-                 NULL_VM_UFFD_CTX, anon_vma_name(vma));                            -                 NULL_VM_UFFD_CTX, anon_vma_name(vma));
                                                                                    -        if (prev) {
                                                                                    -            vma = prev;
                                                                                    -        } else {
                                                                                    -            prev = vma;
                                                                                    -        }
+        prev = vma_modify_flags_uffd(&vmi, prev, vma, vma->vm_start,               +        vma = vma_modify_flags_uffd(&vmi, prev, vma, vma->vm_start,
+                         vma->vm_end, new_flags,                                   +                        vma->vm_end, new_flags,
+                         NULL_VM_UFFD_CTX);                                        +                        NULL_VM_UFFD_CTX);
                                                                                     
                                                                                             vma_start_write(vma);
                                                                                             userfaultfd_set_vm_flags(vma, new_flags);
                                                                                             vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
+                                                                                   +
         if (prev) {                                                                
             vma = prev;                                                            
         } else {                                                                   
                                                                                    +        prev = vma;
                                                                                         }
                                                                                         mmap_write_unlock(mm);
                                                                                         mmput(mm);
@@ fs/userfaultfd.c: static int userfaultfd_register(struct userfaultfd_ctx *ctx,   @@ fs/userfaultfd.c: static int userfaultfd_register(struct userfaultfd_ctx *ctx,
     unsigned long start, end, vma_end;                                                  unsigned long start, end, vma_end;
     struct vma_iterator vmi;                                                            struct vma_iterator vmi;

────────────────────────────────────────────────────────────────────────────────
 fs/userfaultfd.c: static int userfaultfd_register(struct userfaultfd_ctx *ctx, 
────────────────────────────────────────────────────────────────────────────────
-            /* vma_merge() invalidated the mas */                                  -            /* vma_merge() invalidated the mas */
-            vma = prev;                                                            -            vma = prev;
-            goto next;                                                             -            goto next;
                                                                                    +
+        prev = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end,              +        vma = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end,
+                         new_flags,                                                +                        new_flags,
+                         (struct vm_userfaultfd_ctx){ctx});                        +                        (struct vm_userfaultfd_ctx){ctx});
+        if (IS_ERR(prev)) {                                                        +        if (IS_ERR(vma)) {
+            ret = PTR_ERR(prev);                                                   +            ret = PTR_ERR(vma);
+            break;                                                                 +            break;
         }                                                                                   }
-        if (vma->vm_start < start) {                                               -        if (vma->vm_start < start) {

────────────────────────────────────────────────────────────────────────────────
 fs/userfaultfd.c: static int userfaultfd_register(struct userfaultfd_ctx *ctx, 
────────────────────────────────────────────────────────────────────────────────
-                break;                                                             -                break;
-        }                                                                          -        }
-    next:                                                                          -    next:
+                                                                                   
+        if (prev)                                                                  
+            vma = prev; /* vma_merge() invalidated the mas */                      
+                                                                                   +
         /*                                                                                  /*
          * In the vma_merge() successful mprotect-like case 8:                               * In the vma_merge() successful mprotect-like case 8:

──────────────────────────────────────────────────────────────────────────────────
 fs/userfaultfd.c: static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, 
──────────────────────────────────────────────────────────────────────────────────
-                 vma_policy(vma),                                                  -                 vma_policy(vma),
-                 NULL_VM_UFFD_CTX, anon_vma_name(vma));                            -                 NULL_VM_UFFD_CTX, anon_vma_name(vma));
-        if (prev) {                                                                -        if (prev) {
                                                                                    -            vma = prev;
                                                                                    -            goto next;
+        prev = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end,              +        vma = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end,
+                         new_flags, NULL_VM_UFFD_CTX);                             +                        new_flags, NULL_VM_UFFD_CTX);
+        if (IS_ERR(prev)) {                                                        +        if (IS_ERR(vma)) {
+            ret = PTR_ERR(prev);                                                   +            ret = PTR_ERR(prev);
+            break;                                                                 +            break;
+        }                                                                                   }
+                                                                                   
+        if (prev)                                                                  
             vma = prev;                                                            
-            goto next;                                                             
-        }                                                                          
-        if (vma->vm_start < start) {                                               -        if (vma->vm_start < start) {
-            ret = split_vma(&vmi, vma, start, 1);                                  -            ret = split_vma(&vmi, vma, start, 1);
-            if (ret)                                                               -            if (ret)

──────────────────────────────────────────────────────────────────────────────────
 fs/userfaultfd.c: static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, 
──────────────────────────────────────────────────────────────────────────────────
-                break;                                                             -                break;
-        }                                                                          -        }
-    next:                                                                          -    next:
                                                                                    +
         /*                                                                                  /*
          * In the vma_merge() successful mprotect-like case 8:                               * In the vma_merge() successful mprotect-like case 8:
          * the next vma was merged into the current one and                                  * the next vma was merged into the current one and

─────────────────────────────────────────────────────────────────────────
 mm/madvise.c: static int madvise_update_vma(struct vm_area_struct *vma, 
─────────────────────────────────────────────────────────────────────────
     struct mm_struct *mm = vma->vm_mm;                                                  struct mm_struct *mm = vma->vm_mm;
     int error;                                                                          int error;
-    pgoff_t pgoff;                                                                 -    pgoff_t pgoff;
+    struct vm_area_struct *merged;                                                 
     VMA_ITERATOR(vmi, mm, start);                                                       VMA_ITERATOR(vmi, mm, start);
                                                                                     
     if (new_flags == vma->vm_flags && anon_vma_name_eq(anon_vma_name(vma), anon_       if (new_flags == vma->vm_flags && anon_vma_name_eq(anon_vma_name(vma), anon_
                                                                         name)) {                                                                           name)) {

─────────────────────────────────────────────────────────────────────────
 mm/madvise.c: static int madvise_update_vma(struct vm_area_struct *vma, 
─────────────────────────────────────────────────────────────────────────
-        vma = *prev;                                                               -        vma = *prev;
-        goto success;                                                              -        goto success;
-    }                                                                              -    }
+    merged = vma_modify_flags_name(&vmi, *prev, vma, start, end, new_flags,        +    vma = vma_modify_flags_name(&vmi, *prev, vma, start, end, new_flags,
+                       anon_name);                                                 +                    anon_name);
+    if (IS_ERR(merged))                                                            +    if (IS_ERR(vma))
+        return PTR_ERR(merged);                                                    +        return PTR_ERR(vma);
                                                                                     
-    *prev = vma;                                                                        *prev = vma;
+    if (merged)                                                                    
+        vma = *prev = merged;                                                      
+    else                                                                           
+        *prev = vma;                                                               
                                                                                     
-    if (start != vma->vm_start) {                                                  -    if (start != vma->vm_start) {
-        error = split_vma(&vmi, vma, start, 1);                                    -        error = split_vma(&vmi, vma, start, 1);

─────────────────────────────────────────────────────────────────────────
 mm/madvise.c: static int madvise_update_vma(struct vm_area_struct *vma, 
─────────────────────────────────────────────────────────────────────────
                                                                                    
 ## mm/mempolicy.c ##                                                                ## mm/mempolicy.c ##
@@ mm/mempolicy.c: static int mbind_range(struct vma_iterator *vmi, struct vm_are  @@ mm/mempolicy.c: static int mbind_range(struct vma_iterator *vmi, struct vm_are
                                                                   a_struct *vma,                                                                     a_struct *vma,
                                                                                             struct vm_area_struct **prev, unsigned long start,
                                                                                             unsigned long end, struct mempolicy *new_pol)
 {                                                                                   {
     struct vm_area_struct *merged;                                                 -    struct vm_area_struct *merged;
     unsigned long vmstart, vmend;                                                       unsigned long vmstart, vmend;
-    pgoff_t pgoff;                                                                 -    pgoff_t pgoff;
-    int err;                                                                       -    int err;

──────────────────────────────────────────────────────────────────────────────────
 mm/mempolicy.c: static int mbind_range(struct vma_iterator *vmi, struct vm_area_ 
──────────────────────────────────────────────────────────────────────────────────
-    merged = vma_merge(vmi, vma->vm_mm, *prev, vmstart, vmend, vma->vm_flags,      -    merged = vma_merge(vmi, vma->vm_mm, *prev, vmstart, vmend, vma->vm_flags,
-             vma->anon_vma, vma->vm_file, pgoff, new_pol,                          -             vma->anon_vma, vma->vm_file, pgoff, new_pol,
-             vma->vm_userfaultfd_ctx, anon_vma_name(vma));                         -             vma->vm_userfaultfd_ctx, anon_vma_name(vma));
+    merged =  vma_modify_policy(vmi, *prev, vma, vmstart, vmend, new_pol);         
+    if (IS_ERR(merged))                                                            -    if (merged) {
                                                                                    -        *prev = merged;
+        return PTR_ERR(merged);                                                    -        return vma_replace_policy(merged, new_pol);
+                                                                                   
     if (merged) {                                                                  
         *prev = merged;                                                            
         return vma_replace_policy(merged, new_pol);                                
     }                                                                              -    }
                                                                                    
                                                                                    -
-    if (vma->vm_start != vmstart) {                                                -    if (vma->vm_start != vmstart) {
-        err = split_vma(vmi, vma, vmstart, 1);                                     -        err = split_vma(vmi, vma, vmstart, 1);
-        if (err)                                                                   -        if (err)

──────────────────────────────────────────────────────────────────────────────────
 mm/mempolicy.c: static int mbind_range(struct vma_iterator *vmi, struct vm_area_ 
──────────────────────────────────────────────────────────────────────────────────
-        if (err)                                                                   -        if (err)
-            return err;                                                            -            return err;
-    }                                                                              -    }
-                                                                                   
                                                                                    +    vma =  vma_modify_policy(vmi, *prev, vma, vmstart, vmend, new_pol);
                                                                                    +    if (IS_ERR(vma))
                                                                                    +        return PTR_ERR(vma);
                                                                                     
     *prev = vma;                                                                        *prev = vma;
     return vma_replace_policy(vma, new_pol);                                            return vma_replace_policy(vma, new_pol);
 }                                                                                  
                                                                                    
 ## mm/mlock.c ##                                                                    ## mm/mlock.c ##
@@ mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_st  @@ mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_st
                                                                       ruct *vma,                                                                         ruct *vma,

──────────────────────────────────────────────────────────────────────────────────
 mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_stru 
──────────────────────────────────────────────────────────────────────────────────
     int nr_pages;                                                                       int nr_pages;
     int ret = 0;                                                                        int ret = 0;
     vm_flags_t oldflags = vma->vm_flags;                                                vm_flags_t oldflags = vma->vm_flags;
+    struct vm_area_struct *merged;                                                 
                                                                                    
     if (newflags == oldflags || (oldflags & VM_SPECIAL) ||                         
         is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm) ||             
@@ mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_st  @@ mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_st
                                                                       ruct *vma,                                                                         ruct *vma,
         /* don't set VM_LOCKED or VM_LOCKONFAULT and don't count */                         /* don't set VM_LOCKED or VM_LOCKONFAULT and don't count */
         goto out;                                                                           goto out;

──────────────────────────────────────────────────────────────────────────────────
 mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_stru 
──────────────────────────────────────────────────────────────────────────────────
-    if (*prev) {                                                                   -    if (*prev) {
-        vma = *prev;                                                               -        vma = *prev;
-        goto success;                                                              -        goto success;
+    merged = vma_modify_flags(vmi, *prev, vma, start, end, newflags);              +    vma = vma_modify_flags(vmi, *prev, vma, start, end, newflags);
+    if (IS_ERR(merged)) {                                                          +    if (IS_ERR(vma)) {
+        ret = PTR_ERR(merged);                                                     +        ret = PTR_ERR(vma);
+        goto out;                                                                  +        goto out;
     }                                                                                   }
                                                                                     

──────────────────────────────────────────────────────────────────────────────────
 mm/mlock.c: static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_stru 
──────────────────────────────────────────────────────────────────────────────────
-        if (ret)                                                                   -        if (ret)
-            goto out;                                                              -            goto out;
-    }                                                                              -    }
+    if (merged)                                                                    
+        vma = *prev = merged;                                                      
                                                                                    
                                                                                    -
-    if (end != vma->vm_end) {                                                      -    if (end != vma->vm_end) {
-        ret = split_vma(vmi, vma, end, 0);                                         -        ret = split_vma(vmi, vma, end, 0);
-        if (ret)                                                                   -        if (ret)

────────────────────────────────────────────────────────────────────────────────
 mm/mmap.c: int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma, 
────────────────────────────────────────────────────────────────────────────────
+ *                                                                                 + *
+ * If no merge is possible and the range does not span the entirety of the VMA,    + * If no merge is possible and the range does not span the entirety of the VMA,
+ * we then need to split the VMA to accommodate the change.                        + * we then need to split the VMA to accommodate the change.
                                                                                    + *
                                                                                    + * The function returns either the merged VMA, the original VMA if a split was
                                                                                    + * required instead, or an error if the split failed.
+ */                                                                                + */
+struct vm_area_struct *vma_modify(struct vma_iterator *vmi,                        +struct vm_area_struct *vma_modify(struct vma_iterator *vmi,
+                  struct vm_area_struct *prev,                                     +                  struct vm_area_struct *prev,

────────────────────────────────────────────────────────────────────────────────
 mm/mmap.c: int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma, 
────────────────────────────────────────────────────────────────────────────────
+            return ERR_PTR(err);                                                   +            return ERR_PTR(err);
+    }                                                                              +    }
+                                                                                   +
+    return NULL;                                                                   +    return vma;
+}                                                                                  +}
+                                                                                   +
 /*                                                                                  /*

─────────────────────────────────────────────────────────────────────────────────
 mm/mprotect.c: mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb, 
─────────────────────────────────────────────────────────────────────────────────
     unsigned int mm_cp_flags = 0;                                                       unsigned int mm_cp_flags = 0;
     unsigned long charged = 0;                                                          unsigned long charged = 0;
-    pgoff_t pgoff;                                                                 -    pgoff_t pgoff;
+    struct vm_area_struct *merged;                                                 
     int error;                                                                          int error;
                                                                                     
     if (newflags == oldflags) {                                                         if (newflags == oldflags) {

─────────────────────────────────────────────────────────────────────────────────
 mm/mprotect.c: mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb, 
─────────────────────────────────────────────────────────────────────────────────
-               vma->vm_userfaultfd_ctx, anon_vma_name(vma));                       -               vma->vm_userfaultfd_ctx, anon_vma_name(vma));
-    if (*pprev) {                                                                  -    if (*pprev) {
-        vma = *pprev;                                                              -        vma = *pprev;
+    merged = vma_modify_flags(vmi, *pprev, vma, start, end, newflags);             
+    if (IS_ERR(merged)) {                                                          
+        error = PTR_ERR(merged);                                                   
+        goto fail;                                                                 
+    }                                                                              
+                                                                                   
+    if (merged) {                                                                  
+        vma = *pprev = merged;                                                     
         VM_WARN_ON((vma->vm_flags ^ newflags) & ~VM_SOFTDIRTY);                    -        VM_WARN_ON((vma->vm_flags ^ newflags) & ~VM_SOFTDIRTY);
-        goto success;                                                              -        goto success;
+    } else {                                                                       
+        *pprev = vma;                                                              
                                                                                    +    vma = vma_modify_flags(vmi, *pprev, vma, start, end, newflags);
                                                                                    +    if (IS_ERR(vma)) {
                                                                                    +        error = PTR_ERR(vma);
                                                                                    +        goto fail;
     }                                                                                   }
                                                                                     
-    *pprev = vma;                                                                       *pprev = vma;
-                                                                                   
                                                                                     
-    if (start != vma->vm_start) {                                                  -    if (start != vma->vm_start) {
-        error = split_vma(vmi, vma, start, 1);                                     -        error = split_vma(vmi, vma, start, 1);
-        if (error)                                                                 -        if (error)

Implementation

Ideally, this side-by-side view of git range-diff would just appear after I configure pager.range-diff='delta -s' in .gitconfig. Unfortunately, it’s not that easy:

Nevertheless, I hacked together a proof-of-concept preprocessor that takes the output of git range-diff and turns it into something that delta parses and presents nicely to a human:

git-range-diff-delta-preproc
#!/usr/bin/env bash

# preprocessor for "git range-diff" output to be fed into "delta" for side-by-side diff
# see ~/.config/git/config for usage (si-range-diff alias)
#
# depends on ansi2txt from https://github.com/kilobyte/colorized-logs
#
# developed against git 2.40 - git range-diff doesn't have stable output, might need adjustments
# see https://github.com/git/git/blob/ee48e70a829d1fa2da82f14787051ad8e7c45b71/range-diff.c#L376

set -eu

coproc ansi2txt {
	stdbuf -oL ansi2txt
}

while IFS= read -r l; do
	# decolor line for matching/processing
	printf "%s\n" "$l" >&"${ansi2txt[1]}"
	IFS= read -r ll <&"${ansi2txt[0]}"

	if [[ $ll == "    @@ "* ]]; then # hunk header line
		# fake hunk header for delta
		printf "@@ -0,0 +0,0 @@ %s\n" "${ll#    @@ }"
	elif [[ $ll == "    "* ]]; then # hunk diff line
		# un-indent diff
		printf "%s\n" "${ll#    }"
	else # pair header line
		# horizontal separator before the line
		tput setaf 39; tput setab 235; printf %"$(tput cols)"s | sed 's/ /─/g'; tput sgr0
		printf "%s\n" "$l"

		if [[ $ll =~ ^[[:digit:]][[:digit:]]*:"  "([[:alnum:]][[:alnum:]]*)" ! "[[:digit:]][[:digit:]]*:"  "([[:alnum:]][[:alnum:]]*)" " ]]; then
			# fake file header for delta to syntax highlight as patch
			echo "--- ${BASH_REMATCH[1]}.patch"
			echo "+++ ${BASH_REMATCH[2]}.patch"
		else
			# extra line for unmatched commits
			echo
		fi
	fi
done

To use this, override pager.range-diff when invoking git range-diff, like so:

$ git -c pager.range-diff='
        git-range-diff-delta-preproc \
        | delta \
                --side-by-side \
                --file-style=omit \
                --line-numbers-left-format="" \
                --line-numbers-right-format="│ " \
                --hunk-header-style=syntax \
        ' range-diff wx-partial...wx-partial-rebase

(Or just steal all the delta-related bits from my git config.)

Yeah, I know, it’s a massive hack. 😞

Next steps

To make this easier to use for the general public, we’d need:

Unfortunately, I can’t volunteer to do either at this point2, apologies.


  1. At the time of writing. 

  2. At present, I’m (very slowly) recovering from a mental health crisis. Just getting this blog post out took me several months.

    (This isn’t a call for help, it’s just context for why I can’t be expected to do anything besides publishing this.)