Bisecting display bugs

Bisecting display bugs

[Clément Pit--Claudel]

[-- Attachment #1.1.1: Type: text/plain, Size: 539 bytes --]

Hey emacs-devel,

I've written up notes while bisecting a display bug (#23938) on master.  Since it didn't seem easy to identify the problem automatically from Elisp, I used a small `git bisect run' script that automatically captured, cropped, and compared screenshots of Emacs against a known-good reference.  All in all, things worked nicely: git bisect took 4 hours to complete, but it didn't require any interaction after starting.

I attached my notes.  Is there interest in including them in admin/notes/?

Cheers,
Clément.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Add-notes-about-using-git-bisect-to-pinpoint-Emacs-b.patch --]
[-- Type: text/x-diff; name="0001-Add-notes-about-using-git-bisect-to-pinpoint-Emacs-b.patch", Size: 5681 bytes --]

From 15196c30e37aa937b121abcd7d95d35e0b3350d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit--Claudel?= <clement.pitclaudel@live.com>
Date: Mon, 11 Jul 2016 01:47:01 +0200
Subject: [PATCH] Add notes about using `git bisect' to pinpoint Emacs bugs

* admin/notes/bisecting: New file.
* admin/notes/repo: Add pointer to admin/notes/bisecting.
---
 admin/notes/bisecting | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
 admin/notes/repo      |   3 +-
 2 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 admin/notes/bisecting

diff --git a/admin/notes/bisecting b/admin/notes/bisecting
new file mode 100644
index 0000000..4efb5cc
--- /dev/null
+++ b/admin/notes/bisecting
@@ -0,0 +1,105 @@
+HOW TO USE GIT BISECT TO PINPOINT EMACS BUGS  -*- outline -*-
+
+This documents explains how to use git bisect to find the commit that
+introduced an Emacs bug.  Bisecting works best if the bug is easy and
+relatively quick to reproduce in a clean ‘emacs -Q’ instance.
+
+For general information about bisecting, use ‘M-x man git-bisect’.
+This document contains quick-start instructions and Emacs-specific
+tips.
+
+* Bisecting Emacs bugs
+
+First, find a recipe that reproduces the bug in emacs -Q.  Then use
+this recipe to locate a revision in which the bug doesn't happen.  If
+it's a new bug, a not-too-recent revision could do; otherwise,
+checking for the bug previous releases of Emacs works well.
+
+Then, run the following command to start bisecting
+
+    git bisect start <buggy-revision> <good-revision>
+
+<buggy-revision> is usually HEAD in practice.
+
+Git will then check out revisions between these two bounds,; at each
+step, you need to run ‘git bisect good’ or ‘git bisect bad’ to inform
+git of the status of the current revision: ‘bad’ if it has the bug;
+good if it doesn't.
+
+If Emacs' build is broken in the current revision, use ‘git bisect
+skip’ to move to a neighboring revision.
+
+** Automating ‘git bisect’
+
+You can also write ‘git bisect run <some-shell-script>’ to automate
+the process.  ‘<some-shell-script>’ should build Emacs, run your bug
+reproduction test, and return 0 if the current revision is good, and 1
+otherwise.  Returning 125 is equivalent to doing ‘git bisect skip’.
+
+Concretely, ‘<some-shell-script>’ usually looks like this:
+
+    #!/usr/bin/env bash
+
+    # Remove leftovers from previous runs
+    git clean -xfd > /dev/null
+    # Build Emacs and skip commit if build fails
+    (./autogen.sh && ./configure --cache-file=/tmp/emacs.config.cache && make -j4) || exit 125
+
+    # Reproduce the bug, writing output somewhere
+    src/emacs -Q -l "../reproduce-the-bug.el" || exit 125
+    # Remove leftovers again
+    git clean -xfd > /dev/null
+    # Analyze output and produce exit code
+    cmp ../reference-output current-output
+
+Some caveats:
+
+- This script cleans Emacs' source directory with ‘git clean -xfd’, so
+  make sure your uncommitted changes are saved somewhere else.
+
+- You should produce the ‘../reproduce-your-bug.el’ script on your own
+  (it should check if the bug exists, and save a different output to a
+  file named ‘current-output’ based on the result of that check), as
+  well as a file ‘../reference-output’ containing the expected output
+  in the good case.
+
+** Using ‘git bisect’ to find display-related bugs
+
+Most bugs that manifest graphically can be checked for by
+programmatically inspecting text properties, but some are only
+apparent through visual inspection.  Since building Emacs takes a long
+time, it can be a pain to debug these manually.
+
+Fortunately, it's relatively easy to bisect such bugs automatically.
+Use the following template for ‘../reproduce-the-bug.el’:
+
+    (defun take-screenshot-and-exit (fname x y w h)
+      "Save a screenshot of Emacs as FNAME, then exit.
+    X and Y are the coordinates of the top-left point of the area of interest.
+    W, and H are its dimensions."
+      (let ((window-id (frame-parameter nil 'outer-window-id)))
+        (call-process "xwd" nil nil nil "-silent" "-id" window-id "-out" fname))
+      (call-process "mogrify" nil nil nil fname "-crop" (format "%dx%d+%d+%d" w h x y))
+      (kill-emacs))
+
+    (defun main ()
+      ;; Reproduce your bug here
+      …
+      ;; Force a redisplay
+      (redisplay t)
+      ;; Insert rough X, Y, W, H values below
+      (run-with-timer 0 nil #'take-screenshot-and-exit "screenshot.xwd" … … … …))
+
+    (main)
+
+This script makes a screenshot of Emacs after reproducing your bug (if
+‘xwd’ isn't available, you can use ImageMagick's ‘import’ tool,
+passing it a ‘-window’ argument where ‘xwd’ wants ‘id’).  Cropping the
+image is useful to weed out unrelated display changes; try to include
+only a small portion of the screen containing your bug.
+
+Then to produce the right exit code use ImageMagick's ‘identify’ tool:
+
+    cmp <(identify -quiet -format "%#" "../screenshot.xwd") <(identify -quiet -format "%#" "../good.xwd")
+
+‘good.xwd’ should be the image produced by your script in the good state.
diff --git a/admin/notes/repo b/admin/notes/repo
index 3ab3da7..60373bd 100644
--- a/admin/notes/repo
+++ b/admin/notes/repo
@@ -114,8 +114,7 @@ again.
 
 * Bisecting
 
-This is a semi-automated way to find the revision that introduced a bug.
-Browse 'git help bisect' for technical instructions.
+See admin/notes/bisecting.
 
 * Maintaining ChangeLog history
 
-- 
2.9.0


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Bisecting display bugs

[Óscar Fuentes]

Clément Pit--Claudel <clement.pit@gmail.com> writes:

> I've written up notes while bisecting a display bug (#23938) on
> master. Since it didn't seem easy to identify the problem
> automatically from Elisp, I used a small `git bisect run' script that
> automatically captured, cropped, and compared screenshots of Emacs
> against a known-good reference. All in all, things worked nicely: git
> bisect took 4 hours to complete, but it didn't require any interaction
> after starting.
>
> I attached my notes.  Is there interest in including them in admin/notes/?

Speaking as someone who painfully bisected Emacs many times (read:
having to visually check good/bad result on each iteration), the part
about comparing the display's image against a good sample is highly
interesting, and not only for hunting display bugs.

Re: Bisecting display bugs

[Eli Zaretskii]

> From: Clément Pit--Claudel <clement.pit@gmail.com>
> Date: Mon, 11 Jul 2016 01:55:31 +0200
> 
> I've written up notes while bisecting a display bug (#23938) on master.  Since 
> it didn't seem easy to identify the problem automatically from Elisp, I used a 
> small `git bisect run' script that automatically captured, cropped, and 
> compared screenshots of Emacs against a known-good reference.  All in all, 
> things worked nicely: git bisect took 4 hours to complete, but it didn't 
> require any interaction after starting.
> 
> I attached my notes.  Is there interest in including them in admin/notes/?

Thank you for doing this.  I'm okay with adding this kind of notes,
but I have some comments about the contents.

> +For general information about bisecting, use ‘M-x man git-bisect’.
                                                 ^^^^^^^^^^^^^^^^^^
This doesn't work on Windows (the Windows port of Git doesn't install
man pages), suggest using "git help bisect" instead.

> +* Bisecting Emacs bugs

This section doesn't say anything "git help bisect" doesn't.  It is
not Emacs-specific.  I'm not sure we should repeat the text that is
quite clear in the Git documentation, and also includes useful
examples.

> +it's a new bug, a not-too-recent revision could do; otherwise,
> +checking for the bug previous releases of Emacs works well.
                      ^^^
"in" missing here.

> +** Automating ‘git bisect’
> +
> +You can also write ‘git bisect run <some-shell-script>’ to automate
> +the process.  ‘<some-shell-script>’ should build Emacs, run your bug
> +reproduction test, and return 0 if the current revision is good, and 1
> +otherwise.  Returning 125 is equivalent to doing ‘git bisect skip’.

This part is also described on the Git bisect man page.

> +Concretely, ‘<some-shell-script>’ usually looks like this:
> +
> +    #!/usr/bin/env bash
> +
> +    # Remove leftovers from previous runs
> +    git clean -xfd > /dev/null
> +    # Build Emacs and skip commit if build fails
> +    (./autogen.sh && ./configure --cache-file=/tmp/emacs.config.cache && make -j4) || exit 125
> +
> +    # Reproduce the bug, writing output somewhere
> +    src/emacs -Q -l "../reproduce-the-bug.el" || exit 125
> +    # Remove leftovers again
> +    git clean -xfd > /dev/null
> +    # Analyze output and produce exit code
> +    cmp ../reference-output current-output

IMO, this template could benefit from more generality.  The result
doesn't have to come from 'cmp', it can come from 'diff' or some other
program, like 'cat' or 'ls'.  It can also come from the exit code
produced by Emacs itself (which is probably a whole lot more
convenient when possible).  Finally, what about bugs that cause Emacs
to crash?  I think the template or the notes that come with it should
include some advice on that.

> +Some caveats:
> +
> +- This script cleans Emacs' source directory with ‘git clean -xfd’, so
> +  make sure your uncommitted changes are saved somewhere else.

While bootstrapping after each step of bisect is safe, it makes the
run much longer, so I'd try bisecting without a bootstrap first,
especially if the range of commits to bisect is relatively small.

> +** Using ‘git bisect’ to find display-related bugs
> +
> +Most bugs that manifest graphically can be checked for by
> +programmatically inspecting text properties

Not just text properties: also the position of point, the text itself,
the window start position, and much more.  I find the lack of details
in this part to be quite a gap, since familiarity with the respective
facilities is an important part of being efficient in debugging
display problems.

Specifically, I'd like to see at least the following facilities
mentioned here:

 . window-start and window-end
 . posn-at-point
 . pos-visible-in-window-p
 . frame-geometry
 . window--dump-frame
 . frame--size-history
 . display-monitor-attributes-list
 . C-u C-x =
 . trace-redisplay and trace-to-stderr
 . dump-glyph-matrix and dump-frame-glyph-matrix

Each one of these should be accompanied with a short (1-2 lines)
description of what it does and when is it useful.

>                                              but some are only
> +apparent through visual inspection.  Since building Emacs takes a long
> +time, it can be a pain to debug these manually.

I don't follow this logic: building is an automated process, while
visual inspection is usually very fast; and the automated comparison
you are about to suggest doesn't decrease the build time.  So how does
the conclusion follow?

> +Fortunately, it's relatively easy to bisect such bugs automatically.

This is too optimistic, IMO.  I would reword this in a more cautious
way, something like:

  If the display bug has a clear manifestation in a screenshot of a
  particular portion of Emacs display, and you have a program, like
  'xwd', that can capture the content of the Emacs frame, and also
  have ImageMagick installed, you can automate the comparison of the
  redisplay results to make the bisection process fully automatic.

This technique, while valuable in certain situations, has quite a few
caveats that make it inappropriate in other situations, so we cannot
tell users "here's your magic wand".  One important caveat is that the
size of an Emacs frame or window might not be constant across
revisions, so cropping the images correctly is not that trivial.
Another example of a caveat is when we deliberately change the visual
appearance of a part of the Emacs display (such as the mode line or
the tool bar) -- in these cases the baseline screenshot produced from
a "good" build will no longer be useful.  And there are more caveats,
so being excessively optimistic when recommending this technique might
be a disservice to the reader.

> +      (let ((window-id (frame-parameter nil 'outer-window-id)))
> +        (call-process "xwd" nil nil nil "-silent" "-id" window-id "-out" fname))
> +      (call-process "mogrify" nil nil nil fname "-crop" (format "%dx%d+%d+%d" w h x y))
> +      (kill-emacs))
> +
> +    (defun main ()
> +      ;; Reproduce your bug here
> +      …
> +      ;; Force a redisplay
> +      (redisplay t)
> +      ;; Insert rough X, Y, W, H values below
> +      (run-with-timer 0 nil #'take-screenshot-and-exit "screenshot.xwd" … … … …))
> +
> +    (main)
> +
> +This script makes a screenshot of Emacs after reproducing your bug (if
> +‘xwd’ isn't available, you can use ImageMagick's ‘import’ tool,
> +passing it a ‘-window’ argument where ‘xwd’ wants ‘id’).  Cropping the
> +image is useful to weed out unrelated display changes; try to include
> +only a small portion of the screen containing your bug.
> +
> +Then to produce the right exit code use ImageMagick's ‘identify’ tool:
> +
> +    cmp <(identify -quiet -format "%#" "../screenshot.xwd") <(identify -quiet -format "%#" "../good.xwd")

If you're OK with invoking external programs from within Emacs, why
not compare the results using Emacs capabilities, and produce the
output via the exit code (kill-emacs can instruct Emacs to return an
exit code to the parent shell)?  This has 2 advantages: (1) it doesn't
require the reader to be too proficient in Posix shell scripting, and
(2) the resulting script will be much more portable.

> --- a/admin/notes/repo
> +++ b/admin/notes/repo
> @@ -114,8 +114,7 @@ again.
 
>  * Bisecting
>  
> -This is a semi-automated way to find the revision that introduced a bug.
> -Browse 'git help bisect' for technical instructions.
> +See admin/notes/bisecting.

I wouldn't delete those 2 lines, they are useful for the reader to
decide whether she wants to read the details about bisecting.

Thanks again for working on this.

Re: Bisecting display bugs

[Óscar Fuentes]

Eli Zaretskii <eliz@gnu.org> writes:

[snip]

>> +Some caveats:
>> +
>> +- This script cleans Emacs' source directory with ‘git clean -xfd’, so
>> +  make sure your uncommitted changes are saved somewhere else.
>
> While bootstrapping after each step of bisect is safe, it makes the
> run much longer, so I'd try bisecting without a bootstrap first,
> especially if the range of commits to bisect is relatively small.

Although that sounds good in principle, my experience tells me that
taking the short path does waste more time than it saves on this case.
YMMV, but too many times the bisection stopped with a build failure that
wouldn't happen with bootstrap, or simply gave wrong results because the
build didn't work correctly.

Speaking of this, the sample script could offer bug failure detection
and `git bisect skip'.

[snip]

>>                                              but some are only
>> +apparent through visual inspection.  Since building Emacs takes a long
>> +time, it can be a pain to debug these manually.
>
> I don't follow this logic: building is an automated process, while
> visual inspection is usually very fast; the automated comparison
> you are about to suggest doesn't decrease the build time.  So how does
> the conclusion follow?

Because I have to be there in front of the terminal. And I can make
mistakes. Anything that makes the bisection automatic is a big
advantage.

[snip]

Re: Bisecting display bugs

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Mon, 11 Jul 2016 19:46:42 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> [snip]
> 
> >> +Some caveats:
> >> +
> >> +- This script cleans Emacs' source directory with ‘git clean -xfd’, so
> >> +  make sure your uncommitted changes are saved somewhere else.
> >
> > While bootstrapping after each step of bisect is safe, it makes the
> > run much longer, so I'd try bisecting without a bootstrap first,
> > especially if the range of commits to bisect is relatively small.
> 
> Although that sounds good in principle, my experience tells me that
> taking the short path does waste more time than it saves on this case.

My experience is different.  I almost never bootstrap.  (I also keep
past binaries, which allows a very fast pseudo-bisection.)

> >>                                              but some are only
> >> +apparent through visual inspection.  Since building Emacs takes a long
> >> +time, it can be a pain to debug these manually.
> >
> > I don't follow this logic: building is an automated process, while
> > visual inspection is usually very fast; the automated comparison
> > you are about to suggest doesn't decrease the build time.  So how does
> > the conclusion follow?
> 
> Because I have to be there in front of the terminal.

No, you don't.  While Emacs builds, you can do whatever else you need
to do.

Re: Bisecting display bugs

[Óscar Fuentes]

Eli Zaretskii <eliz@gnu.org> writes:

>> Because I have to be there in front of the terminal.
>
> No, you don't.  While Emacs builds, you can do whatever else you need
> to do.

Bisecting usually requires building several times. Also, I like to focus
on a single task for a long time. Anything that requires occasional
attention is a no-no unless I'm too tired to do something else. Hence
the high risk of making mistakes when I have to decide good/bad/skip.

Re: Bisecting display bugs

[Clément Pit--Claudel]

[-- Attachment #1.1.1: Type: text/plain, Size: 98 bytes --]

> Thanks again for working on this.

Thanks for your comments! I've attached an updated patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Add-notes-about-using-git-bisect-to-pinpoint-Emacs-b.patch --]
[-- Type: text/x-diff; name="0001-Add-notes-about-using-git-bisect-to-pinpoint-Emacs-b.patch", Size: 5629 bytes --]

From bb565a03c35103c5ec60dd42582eafdbdc21b725 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit--Claudel?= <clement.pitclaudel@live.com>
Date: Mon, 11 Jul 2016 01:47:01 +0200
Subject: [PATCH] Add notes about using `git bisect' to pinpoint Emacs bugs

* admin/notes/bisecting: New file.
* admin/notes/repo: Add pointer to admin/notes/bisecting.
---
 admin/notes/bisecting | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
 admin/notes/repo      |   3 +-
 2 files changed, 107 insertions(+), 1 deletion(-)
 create mode 100644 admin/notes/bisecting

diff --git a/admin/notes/bisecting b/admin/notes/bisecting
new file mode 100644
index 0000000..3a335a4
--- /dev/null
+++ b/admin/notes/bisecting
@@ -0,0 +1,105 @@
+HOW TO USE GIT BISECT TO PINPOINT EMACS BUGS  -*- outline -*-
+
+This documents explains how to automate git bisect to find the commit
+that introduced an Emacs bug.  Bisecting automatically works best if
+the bug is easy and quick to reproduce in a clean ‘emacs -Q’ instance.
+
+For an Emacs-independent tutorial on bisecting, run ‘git help bisect’.
+
+* Automating ‘git bisect’
+
+Use ‘git bisect run <some-shell-script>’ to automate the bisecting
+process.  ‘<some-shell-script>’ should build Emacs, run your bug
+reproduction test, and return 0 if the current revision is good, 1 if
+it's bad, and 125 to skip the current revision.
+
+Concretely, ‘<some-shell-script>’ usually looks like this:
+
+    #!/usr/bin/env bash
+
+    # Remove leftovers from previous runs
+    git clean -xfd > /dev/null
+    # Build Emacs and skip commit if build fails
+    (./autogen.sh && ./configure --cache-file=/tmp/emacs.config.cache && make -j4) || exit 125
+
+    # Reproduce the bug, writing output somewhere
+    src/emacs -Q -l "../reproduce-the-bug.el" || exit 125
+
+Some caveats:
+
+- This script cleans Emacs' source directory with ‘git clean -xfd’, so
+  make sure your uncommitted changes are saved somewhere else.
+
+- You should produce the ‘../reproduce-your-bug.el’ script on your own
+  (it should check if the bug exists, and return the right error code
+  using ‘(kill-emacs EXIT-CODE)’).
+
+* Using ‘git bisect’ to find display-related bugs
+
+** Ways to programmatically detect display bugs
+
+Most bugs that manifest graphically can be checked for by
+programmatically inspecting the following elements:
+
+- text properties
+- window-start and window-end
+- posn-at-point
+- pos-visible-in-window-p
+- frame-geometry
+- window--dump-frame
+- frame--size-history
+- display-monitor-attributes-list
+- C-u C-x =
+- trace-redisplay and trace-to-stderr
+- dump-glyph-matrix and dump-frame-glyph-matrix
+
+** When the above fails
+
+Some bugs are only apparent through visual inspection.  Since building
+Emacs takes a long time, it can be a pain to debug these manually.
+
+If your display bug has a clear manifestation in a screenshot of a
+particular portion of Emacs display, and you have a program, like
+'xwd', that can capture the content of the Emacs frame, and also have
+ImageMagick installed, you can automate the comparison of the
+redisplay results to make the bisection process fully automatic.
+
+Use the following template for ‘../reproduce-the-bug.el’: it requires
+ImageMagick (mogrify and identify) and xwd (if ‘xwd’ isn't available,
+you can use ImageMagick's ‘import’ tool, passing it a ‘-window’
+argument where ‘xwd’ wants ‘id’).
+
+    (defun image-checksum (img-fname)
+      "Compute a checksum of IMG-FNAME's image data."
+      (car (process-lines "identify" "-quiet" "-format" "%#" img-fname)))
+
+    (defun take-screenshot-and-exit (fname x y w h reference-fname)
+      "Save a screenshot of Emacs as FNAME, then exit.
+    X and Y are the coordinates of the top-left point of the area of
+    interest.  W, and H are its dimensions.  This function sets
+    Emacs' return code to 0 if the resulting screenshot matches
+    REFERENCE-FNAME, and 1 otherwise."
+      (let ((wid (frame-parameter nil 'outer-window-id))
+            (crop-spec (format "%dx%d+%d+%d" w h x y)))
+        (call-process "xwd" nil nil nil "-silent" "-id" wid "-out" fname)
+        (call-process "mogrify" nil nil nil fname "-crop" crop-spec))
+      (let ((same-picture (equal (image-checksum fname)
+                                 (image-checksum reference-fname))))
+        (kill-emacs (if same-picture 0 1))))
+
+    (defun main ()
+      ;; Reproduce your bug here
+      …
+      ;; Force a redisplay
+      (redisplay t)
+      ;; Insert rough X, Y, W, H values below
+      (run-with-timer 0 nil #'take-screenshot-and-exit
+                      "screenshot.xwd" … … … … "reference-screenshot.xwd"))
+
+    (main)
+
+This script takes and crops a screenshot of Emacs after reproducing
+your bug, then compares the result to a reference (cropped)
+screenshot, and returns 0 if they match, and 1 otherwise.  Cropping is
+useful to weed out unrelated display changes; try to include only a
+small portion of the screen containing your bug.
diff --git a/admin/notes/repo b/admin/notes/repo
index 3ab3da7..f169468 100644
--- a/admin/notes/repo
+++ b/admin/notes/repo
@@ -115,7 +115,8 @@ again.
 * Bisecting
 
 This is a semi-automated way to find the revision that introduced a bug.
-Browse 'git help bisect' for technical instructions.
+Browse 'git help bisect' and admin/notes/bisecting for technical
+instructions.
 
 * Maintaining ChangeLog history
 
-- 
2.7.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Bisecting display bugs

[Eli Zaretskii]

> Cc: Emacs developers <emacs-devel@gnu.org>
> From: Clément Pit--Claudel <clement.pit@gmail.com>
> Date: Fri, 22 Jul 2016 14:55:36 -0400
> 
> Thanks for your comments! I've attached an updated patch.

LGTM, thanks.  One comment:

> +- You should produce the ‘../reproduce-your-bug.el’ script on your own
> +  (it should check if the bug exists, and return the right error code
> +  using ‘(kill-emacs EXIT-CODE)’).

Suggest to say explicitly what is "the right error code" (yes, that
means repeating what you said just 20 lines above, but the relation is
not 100% clear).

Re: Bisecting display bugs

[Clément Pit--Claudel]

[-- Attachment #1.1.1: Type: text/plain, Size: 1198 bytes --]

On 2016-07-23 04:51, Eli Zaretskii wrote:
>> Cc: Emacs developers <emacs-devel@gnu.org>
>> From: Clément Pit--Claudel <clement.pit@gmail.com>
>> Date: Fri, 22 Jul 2016 14:55:36 -0400
>>
>> Thanks for your comments! I've attached an updated patch.
> 
> LGTM, thanks.  One comment:
> 
>> +- You should produce the ‘../reproduce-your-bug.el’ script on your own
>> +  (it should check if the bug exists, and return the right error code
>> +  using ‘(kill-emacs EXIT-CODE)’).
> 
> Suggest to say explicitly what is "the right error code" (yes, that
> means repeating what you said just 20 lines above, but the relation is
> not 100% clear).

Thanks; here's a diff against the previous patch (I've attached the updated patch). I also added a note about full rebuilds.

---
 
Some caveats:

-  make sure your uncommitted changes are saved somewhere else.
+  make sure your uncommitted changes are saved somewhere else.  Also,
+  for bisecting small ranges, a full clean and rebuild might not be
+  needed.
 
-  using ‘(kill-emacs EXIT-CODE)’).
+  using ‘(kill-emacs EXIT-CODE)’: 1 if the bug is present, 0 if it is
+  not, and 125 if it can't tell).

---

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Add-notes-about-using-git-bisect-to-pinpoint-Emacs-b.patch --]
[-- Type: text/x-diff; name="0001-Add-notes-about-using-git-bisect-to-pinpoint-Emacs-b.patch", Size: 5789 bytes --]

From 0facdcbf9d3722e96c5810ee837fea0fa496cb79 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit--Claudel?= <clement.pitclaudel@live.com>
Date: Mon, 11 Jul 2016 01:47:01 +0200
Subject: [PATCH] Add notes about using `git bisect' to pinpoint Emacs bugs

* admin/notes/bisecting: New file.
* admin/notes/repo: Add pointer to admin/notes/bisecting.
---
 admin/notes/bisecting | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++
 admin/notes/repo      |   3 +-
 2 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 admin/notes/bisecting

diff --git a/admin/notes/bisecting b/admin/notes/bisecting
new file mode 100644
index 0000000..dadd46a
--- /dev/null
+++ b/admin/notes/bisecting
@@ -0,0 +1,108 @@
+HOW TO USE GIT BISECT TO PINPOINT EMACS BUGS  -*- outline -*-
+
+This documents explains how to automate git bisect to find the commit
+that introduced an Emacs bug.  Bisecting automatically works best if
+the bug is easy and quick to reproduce in a clean ‘emacs -Q’ instance.
+
+For an Emacs-independent tutorial on bisecting, run ‘git help bisect’.
+
+* Automating ‘git bisect’
+
+Use ‘git bisect run <some-shell-script>’ to automate the bisecting
+process.  ‘<some-shell-script>’ should build Emacs, run your bug
+reproduction test, and return 0 if the current revision is good, 1 if
+it's bad, and 125 to skip the current revision.
+
+Concretely, ‘<some-shell-script>’ usually looks like this:
+
+    #!/usr/bin/env bash
+
+    # Remove leftovers from previous runs
+    git clean -xfd > /dev/null
+    # Build Emacs and skip commit if build fails
+    (./autogen.sh && ./configure --cache-file=/tmp/emacs.config.cache && make -j4) || exit 125
+
+    # Reproduce the bug, writing output somewhere
+    src/emacs -Q -l "../reproduce-the-bug.el" || exit 125
+
+Some caveats:
+
+- This script cleans Emacs' source directory with ‘git clean -xfd’, so
+  make sure your uncommitted changes are saved somewhere else.  Also,
+  for bisecting small ranges, a full clean and rebuild might not be
+  needed.
+
+- You should produce the ‘../reproduce-your-bug.el’ script on your own
+  (it should check if the bug exists, and return the right error code
+  using ‘(kill-emacs EXIT-CODE)’: 1 if the bug is present, 0 if it is
+  not, and 125 if it can't tell).
+
+* Using ‘git bisect’ to find display-related bugs
+
+** Ways to programmatically detect display bugs
+
+Most bugs that manifest graphically can be checked for by
+programmatically inspecting the following elements:
+
+- text properties
+- window-start and window-end
+- posn-at-point
+- pos-visible-in-window-p
+- frame-geometry
+- window--dump-frame
+- frame--size-history
+- display-monitor-attributes-list
+- C-u C-x =
+- trace-redisplay and trace-to-stderr
+- dump-glyph-matrix and dump-frame-glyph-matrix
+
+** When the above fails
+
+Some bugs are only apparent through visual inspection.  Since building
+Emacs takes a long time, it can be a pain to debug these manually.
+
+If your display bug has a clear manifestation in a screenshot of a
+particular portion of Emacs display, and you have a program, like
+'xwd', that can capture the content of the Emacs frame, and also have
+ImageMagick installed, you can automate the comparison of the
+redisplay results to make the bisection process fully automatic.
+
+Use the following template for ‘../reproduce-the-bug.el’: it requires
+ImageMagick (mogrify and identify) and xwd (if ‘xwd’ isn't available,
+you can use ImageMagick's ‘import’ tool, passing it a ‘-window’
+argument where ‘xwd’ wants ‘id’).
+
+    (defun image-checksum (img-fname)
+      "Compute a checksum of IMG-FNAME's image data."
+      (car (process-lines "identify" "-quiet" "-format" "%#" img-fname)))
+
+    (defun take-screenshot-and-exit (fname x y w h reference-fname)
+      "Save a screenshot of Emacs as FNAME, then exit.
+    X and Y are the coordinates of the top-left point of the area of
+    interest.  W, and H are its dimensions.  This function sets
+    Emacs' return code to 0 if the resulting screenshot matches
+    REFERENCE-FNAME, and 1 otherwise."
+      (let ((wid (frame-parameter nil 'outer-window-id))
+            (crop-spec (format "%dx%d+%d+%d" w h x y)))
+        (call-process "xwd" nil nil nil "-silent" "-id" wid "-out" fname)
+        (call-process "mogrify" nil nil nil fname "-crop" crop-spec))
+      (let ((same-picture (equal (image-checksum fname)
+                                 (image-checksum reference-fname))))
+        (kill-emacs (if same-picture 0 1))))
+
+    (defun main ()
+      ;; Reproduce your bug here
+      …
+      ;; Force a redisplay
+      (redisplay t)
+      ;; Insert rough X, Y, W, H values below
+      (run-with-timer 0 nil #'take-screenshot-and-exit
+                      "screenshot.xwd" … … … … "reference-screenshot.xwd"))
+
+    (main)
+
+This script takes and crops a screenshot of Emacs after reproducing
+your bug, then compares the result to a reference (cropped)
+screenshot, and returns 0 if they match, and 1 otherwise.  Cropping is
+useful to weed out unrelated display changes; try to include only a
+small portion of the screen containing your bug.
diff --git a/admin/notes/repo b/admin/notes/repo
index 3ab3da7..f169468 100644
--- a/admin/notes/repo
+++ b/admin/notes/repo
@@ -115,7 +115,8 @@ again.
 * Bisecting
 
 This is a semi-automated way to find the revision that introduced a bug.
-Browse 'git help bisect' for technical instructions.
+Browse 'git help bisect' and admin/notes/bisecting for technical
+instructions.
 
 * Maintaining ChangeLog history
 
-- 
2.7.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Bisecting display bugs

[Clément Pit--Claudel]

If there are no further comments, I'll push this soon :)

On 2016-07-23 09:22, Clément Pit--Claudel wrote:
> On 2016-07-23 04:51, Eli Zaretskii wrote:
>>> Cc: Emacs developers <emacs-devel@gnu.org>
>>> From: Clément Pit--Claudel <clement.pit@gmail.com>
>>> Date: Fri, 22 Jul 2016 14:55:36 -0400
>>>
>>> Thanks for your comments! I've attached an updated patch.
>>
>> LGTM, thanks.  One comment:
>>
>>> +- You should produce the ‘../reproduce-your-bug.el’ script on your own
>>> +  (it should check if the bug exists, and return the right error code
>>> +  using ‘(kill-emacs EXIT-CODE)’).
>>
>> Suggest to say explicitly what is "the right error code" (yes, that
>> means repeating what you said just 20 lines above, but the relation is
>> not 100% clear).
> 
> Thanks; here's a diff against the previous patch (I've attached the updated patch). I also added a note about full rebuilds.
> 
> ---
>  
> Some caveats:
> 
> -  make sure your uncommitted changes are saved somewhere else.
> +  make sure your uncommitted changes are saved somewhere else.  Also,
> +  for bisecting small ranges, a full clean and rebuild might not be
> +  needed.
>  
> -  using ‘(kill-emacs EXIT-CODE)’).
> +  using ‘(kill-emacs EXIT-CODE)’: 1 if the bug is present, 0 if it is
> +  not, and 125 if it can't tell).
> 
> ---
> 

Re: Bisecting display bugs

[Noam Postavsky]

+    src/emacs -Q -l "../reproduce-the-bug.el" || exit 125

Wouldn't this exit with status 125 if "../reproduce-the-bug.el" exits
with status 1?

Re: Bisecting display bugs

[Clément Pit-Claudel]

On 2017-04-16 11:32, Noam Postavsky wrote:
> +    src/emacs -Q -l "../reproduce-the-bug.el" || exit 125
> 
> Wouldn't this exit with status 125 if "../reproduce-the-bug.el" exits
> with status 1?

You're right, thanks; it seems this || exit 125 is superfluous.

Re: Bisecting display bugs

[Herring, Davis]

>> +    src/emacs -Q -l "../reproduce-the-bug.el" || exit 125
>>
>> Wouldn't this exit with status 125 if "../reproduce-the-bug.el" exits
>> with status 1?
> 
> You're right, thanks; it seems this || exit 125 is superfluous.

The status 125 means "cannot be tested" (due to unrelated build failure or similar).  If you have a script that produces a meaningful exit code, then of course you don't want it.  It might be good to have the reproducer exit with some value other than EXIT_FAILURE==1 and have the script do something like

src/emacs -Q -l ../reproduce.el
if [ $? -eq 1 ]; then exit 125; else exit $?; done

so that other Emacs failures do register as "skip".

Davis

Re: Bisecting display bugs

[Teemu Likonen]

[-- Attachment #1: Type: text/plain, Size: 723 bytes --]

(Not knowing anything about the discussion itself but adding some shell
info...)


Davis Herring [2017-04-17 13:13:39Z] wrote:

> src/emacs -Q -l ../reproduce.el
> if [ $? -eq 1 ]; then exit 125; else exit $?; done

Note that the [ command has its own exit code and $? is reset. If [ ... ]
is true (=0) then "exit 125". If [ ... ] is false (=1) then "exit $?"
which is always the same as "exit 1".

You need to save the exit code before running the [ command:

    src/emacs -Q -l ../reproduce.el
    code=$?
    if [ "$code" -eq 1 ]; then exit 125; else exit "$code"; fi

-- 
/// Teemu Likonen   - .-..   <https://keybase.io/tlikonen> //
// PGP: 4E10 55DC 84E9 DFF6 13D7 8557 719D 69D3 2453 9450 ///

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 454 bytes --]