From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Bisecting display bugs Date: Mon, 11 Jul 2016 17:22:36 +0300 Message-ID: <837fcsdzkj.fsf@gnu.org> References: <5782E073.6000505@gmail.com> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: ger.gmane.org 1468246997 13757 80.91.229.3 (11 Jul 2016 14:23:17 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 11 Jul 2016 14:23:17 +0000 (UTC) Cc: emacs-devel@gnu.org To: =?utf-8?Q?Cl=C3=A9ment?= Pit--Claudel Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Jul 11 16:23:11 2016 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1bMc75-0007Kw-8f for ged-emacs-devel@m.gmane.org; Mon, 11 Jul 2016 16:23:11 +0200 Original-Received: from localhost ([::1]:33999 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMc74-0000JM-43 for ged-emacs-devel@m.gmane.org; Mon, 11 Jul 2016 10:23:10 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:57684) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMc6u-0000FY-S7 for emacs-devel@gnu.org; Mon, 11 Jul 2016 10:23:02 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bMc6r-0003NQ-DF for emacs-devel@gnu.org; Mon, 11 Jul 2016 10:23:00 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:48039) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMc6r-0003N6-9W; Mon, 11 Jul 2016 10:22:57 -0400 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:3802 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1bMc6o-0007rN-Ve; Mon, 11 Jul 2016 10:22:55 -0400 In-reply-to: <5782E073.6000505@gmail.com> (message from =?utf-8?Q?Cl=C3=A9?= =?utf-8?Q?ment?= Pit--Claudel on Mon, 11 Jul 2016 01:55:31 +0200) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:205545 Archived-At: > From: Clément Pit--Claudel > 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 ’ to automate > +the process. ‘’ 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, ‘’ 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.