Ingo Bürk [Fri, 2 Dec 2016 18:05:43 +0000 (19:05 +0100)]
Do not set input focus in i3-input. (#2598)
This commit removes all traces of setting and reverting the input focus
in i3-input. We don't need to do this because grabbing the keyboard is
sufficient to have the attention we need.
Changing the input focus and reverting it can cause situations where i3
executes the IPC command before processing the FocusIn events. This leads
to i3's input focus change to be rejected due to the timing, leading to
an inconsistent focus state.
Ingo Bürk [Mon, 28 Nov 2016 21:09:39 +0000 (22:09 +0100)]
Respect minimum size hints for floating windows. (#2508)
This commit introduces proper support for the minimum size on floating
windows by ensuring that it is respected during mapping, later changes as
well as resizes.
Furthermore, this commit fixes minor issues with how the hints are handled
during calculations.
This comes with the intentionally undocumented --disable-randr15 command
line flag and disable-randr15 configuration directive. We will add
documentation before the release if and only if it turns out that users
actually need to use this flag in their setups. Ideally, nobody would
need to use the flag and everything would just keep working, but it’s
better to be safe than sorry.
This tool is similar to xtrace in usage in that it intercepts traffic to
the X server. The motivating feature for writing the tool is its ability
to inject prepared reply messages instead of the server’s reply. In
this particular case, we’ll inject a RRGetMonitors reply to test i3’s
RandR 1.5 code paths.
The added testcase is a noop for now, but with the code that’s lingering
in the randr15 branch, i3 does actually detect monitors as per the
injected reply:
2016-11-20 21:10:05 - randr.c:__randr_query_outputs:618 -
RandR 1.5 available, querying monitors
2016-11-20 21:10:05 - randr.c:__randr_query_outputs:628 -
1 RandR monitors found (timestamp 0)
2016-11-20 21:10:05 - randr.c:__randr_query_outputs:646 -
name DP3, x 0, y 0, width 3840 px, height 2160 px, width 520 mm,
height 290 mm, primary 1, automatic 1
configure.ac: verify macros in m4/ are being replaced (Thanks sur5r) (#2571)
See the comment for more details, and see the motivating blog post:
https://blogs.noname-ev.de/sur5r/index.php?/archives/7-Another-instance-of-AC_DEFINE-being-undefined.html
Direct leak of 159 byte(s) in 1 object(s) allocated from:
#0 0x7fea40855602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
#1 0x4c4c4a in smalloc ../../i3/libi3/safewrappers.c:24
#2 0x4c3aee in ipc_recv_message ../../i3/libi3/ipc_recv_message.c:61
#3 0x44dc2e in display_running_version ../../i3/src/display_version.c:94
#4 0x472947 in main ../../i3/src/main.c:269
#5 0x7fea3d0c982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
Direct leak of 39 byte(s) in 2 object(s) allocated from:
#0 0x7fea40855602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
#1 0x7fea3d11f7d7 in vasprintf (/lib/x86_64-linux-gnu/libc.so.6+0x767d7)
SUMMARY: AddressSanitizer: 198 byte(s) leaked in 3 allocation(s).
https://llvm.org/bugs/show_bug.cgi?id=30353 was filed for the unintended
line break between in e.g. “TAILQ_ENTRY(foo)\nbar;”.
Until that’s fixed or a workaround is known, we’ll live with line
breaks. To make it a bit easier for readers to see what’s going on, I
added extra line breaks around each such struct member/variable
definition, so that they at least visually are a single unit.
debian: prefix paths debian/tmp/ to avoid confusion
Without the prefix, the etc/ directory will be copied from the source
directory, as opposed to the debian/tmp directory into which files were
installed.
Fix the issue #2421 (https://github.com/i3/i3/issues/2421).
floating_enable() invokes tree_close_internal() to free con->parent.
After con->parent is freed in tree_close_internal() but before con->parent is reassigned by the caller, con->parent may be dereferenced and causes i3 crash.
The backtrace below is an example.
The already-freed pointer is dereferenced again through the pointer "focused" in x_push_changes().
Reassign con->parent before calling tree_close_internal() to fix this use-after-free bug.
0x0000000000416372 in con_get_workspace (con=0x7ab9c0) at ../i3/src/con.c:375
0x0000000000416103 in con_has_managed_window (con=0x7ab9c0) at ../i3/src/con.c:266
0x000000000042b413 in x_push_changes (con=0x78d190) at ../i3/src/x.c:1132
0x0000000l0004533e8 in tree_render () at ../i3/src/tree.c:504
0x0000000000452b4f in tree_close_internal (con=0x7b67c0, kill_window=DONT_KILL_WINDOW, dont_kill_parent=false, force_set_focus=false)
../i3/src/tree.c:314
0x00000000004196f0 in con_on_remove_child (con=0x7b67c0) at ../i3/src/con.c:1801
0x0000000000452eb7 in tree_close_internal (con=0x783840, kill_window=DONT_KILL_WINDOW, dont_kill_parent=false, force_set_focus=false)
../i3/src/tree.c:364
0x0000000000431516 in floating_enable (con=0x7ab9c0, automatic=false) at ../i3/src/floating.c:183
0x0000000000431eed in toggle_floating_mode (con=0x7ab9c0, automatic=false) at ../i3/src/floating.c:379
0x0000000000420d92 in cmd_floating (current_match=0x679a20 , cmd_output=0x679aa0 , floating_mode=0x7ab8c0 "toggle")
../i3/src/commands.c:1088
0x000000000043e5ae in GENERATED_call (call_identifier=60, result=0x679aa0 ) at include/GENERATED_command_call.h:486
0x000000000043ee19 in next_state (token=0x675d70 ) at ../i3/src/commands_parser.c:187
0x000000000043f2fb in parse_command (input=0x7b4fe0 "floating toggle", gen=0x0) at ../i3/src/commands_parser.c:308
0x00000000004125f8 in run_binding (bind=0x784260, con=0x0) at ../i3/src/bindings.c:792
0x000000000042bace in handle_key_press (event=0x7a01a0) at ../i3/src/key_press.c:33
0x000000000044e6aa in handle_event (type=2, event=0x7a01a0) at ../i3/src/handlers.c:1420
0x0000000000439533 in xcb_check_cb (loop=0x7ffff532f8e0, w=0x68c140, revents=32768) at ../i3/src/main.c:133
0x00007ffff5125d73 in ev_invoke_pending () from /usr/lib/x86_64-linux-gnu/libev.so.4
0x00007ffff51293de in ev_run () from /usr/lib/x86_64-linux-gnu/libev.so.4
0x0000000000439418 in ev_loop (loop=0x7ffff532f8e0, flags=0) at /usr/include/ev.h:835
0x000000000043d51d in main (argc=3, argv=0x7fffffffe0a8) at ../i3/src/main.c:913
Including config.h is necessary to get e.g. the _GNU_SOURCE define and
any other definitions that autoconf declares. Hence, config.h needs to
be included as the first header in each file.
This is done either via:
1. Including "common.h" (i3bar)
2. Including "libi3.h"
3. Including "all.h" (i3)
4. Including <config.h> directly
Also remove now-unused I3__FILE__, add copyright/license statement
where missing and switch include/all.h to #pragma once.
This commit probably comes as a surprise to some, given that one of i3’s
explicitly stated goals used to be “Do not use programs such as
autoconf/automake for configuration and creating unreadable/broken makefiles”.
I phrased this goal over 7 years ago, based largely on a grudge that I
inherited, which — as I’ve realized in the meantime — was largely held against
FOSS in general, and not actually nuanced criticism of autotools.
In the meantime, I have come to realize that the knee-jerk reaction of “I could
do this better!” (i.e. writing our own build system in this particular case) is
usually misguided, and nowadays I strongly suggest trying hard to fix the
existing system for the benefit of all existing and future users.
Further, I recently got to experience the other side of the coin, as I packaged
a new version of FreeRADIUS for Debian, which at the time of writing used
autoconf in combination with boilermake, a custom make-based build system that
only FreeRADIUS uses. Understanding the build system enough to fix issues and
enable parallel compilation took me an entire day. That time is time which
potentially every downstream maintainer needs to invest, and the resulting
knowledge cannot be applied to any other project.
Hence, I believe it’s a good idea switch i3 to autotools. Yes, it might be that
particular features were easier to implement/understand in our custom
Makefiles, and there might be individuals who have an easier time reading
through our custom Makefiles than learning autotools. All of these
considerations are outweighed by the benefits we get from using the same build
system as literally thousands of other FOSS software packages.
Aside from these somewhat philosophical considerations, there’s also practical
improvements which this change brings us. See the “changes” section below.
┌──────────────────────────────────────────────────────────────────────────────┐
│ new workflow │
└──────────────────────────────────────────────────────────────────────────────┘
You can now build i3 like you build any other software package which uses
autotools. Here’s a memory refresher:
autoreconf -fi
mkdir -p build && cd build
../configure
make -j8
(The autoreconf -fi step is unnecessary if you are building from a release
tarball, but shouldn’t hurt either.)
I very much recommend reading “A Practitioner's Guide to GNU Autoconf,
Automake, and Libtool” by John Calcote (https://www.nostarch.com/autotools.htm).
That book is from 2010 and, AFAICT, is the most up to date comprehensive
description of autotools. Do not read older documentation. In particular, if a
document you’re reading mentions configure.in (deprecated filename) or
recursive make (now considered harmful), it’s likely outdated.
This commit implements the following new functionality/changes in behavior:
• We use the AX_ENABLE_BUILDDIR macro to enforce builds happening in a separate
directory. This is a prerequisite for the AX_EXTEND_SRCDIR macro and building
in a separate directory is common practice anyway. In case this causes any
trouble when packaging i3 for your distribution, please let me know.
• “make check” runs the i3 testsuite.
You can still use ./testcases/complete-run.pl to get the interactive progress
output.
• “make distcheck” (runs testsuite on “make dist” result, tiny bit quicker
feedback cycle than waiting for the travis build to catch the issue).
• “make uninstall” (occasionally requested by users who compile from source)
• “make” will build manpages/docs by default if the tools are installed.
Conversely, manpages/docs are not tried to be built for users who don’t want
to install all these dependencies to get started hacking on i3.
• non-release builds will enable address sanitizer by default. Use the
--disable-sanitizers configure option to turn off all sanitizers, and see
--help for available sanitizers.
• Support for pre-compiled headers (PCH) has been dropped for now in the
interest of simplicitly. Maybe we can re-add it later.
• coverage reports are now generated using “make check-code-coverage”, which
requires specifying --enable-code-coverage when calling configure.
┌──────────────────────────────────────────────────────────────────────────────┐
│ build system feature parity/testing │
└──────────────────────────────────────────────────────────────────────────────┘
In addition to what’s described above, I tested the following features:
• “make install” installs the same files (plus documentation and manpages)
cd i3-old && make install PREFIX=/tmp/inst/old
cd i3-new && ./configure --prefix=/tmp/inst/new
cd /tmp/inst
(cd old && for f in $(find); do [ -e "../new/$f" ] || echo "$f missing"; done)
• make dist generates a tarball which includes the same files
cd i3-old && make dist
cd i3-new/x86_64-pc-linux-gnu && make dist
colordiff -u <(tar tf i3-old/i3-4.12.tar.bz2 | sort) \
<(tar tf i3-new/x86_64-pc-linux-gnu/i3-4.12.tar.gz | sort)
There are some expected differences:
• Some files have been renamed (e.g. the new etc/ and share/ subdirectories)
• Some files will now be generated at build-time, so only their corresponding
.in file is shipped (e.g. testcases/complete-run.pl)
• The generated parser files are shipped in the dist tarball (they only
depend on the parser-specs/* files, not on the target system)
• autotools infrastructure is shipped (e.g. “configure”, “missing”, etc.)
• DLOG and ELOG statements still produce the same file name in logfiles
• Listing source code in gdb still works.
• gdb backtraces contain the i3-<version> path component
• release.sh still works
• version embedding
1. git checkout shows “4.12-136-gf720023 (2016-10-10, branch "autotools")”
2. tarball of a git version shows “4.12-non-git”
3. release tarball shows 4.13
• debug mode is enabled by default for non-release builds
There is no noticeable difference in compilation speed itself (of binaries,
documentation and manpages):
i3-old $ time make all docs mans -j8
make all docs mans -j8 28.92s user 2.15s system 640% cpu 4.852 total
i3-new $ time make -j8
make -j8 27.08s user 1.92s system 620% cpu 4.669 total
In terms of one-time costs:
configuring the build system (../configure) takes about 2.7s on my machine,
generating the build system (autoreconf -fi) takes about 3.1s on my machine.
All files in m4/ have been copied from the autoconf-archive package in version b6aeb1988f4b6c78bf39d97b6c4f6e1d594d59b9 and should be updated whenever they
change.
This commit has been tested with autoconf 2.69 and automake 1.15.
Ingo Bürk [Tue, 11 Oct 2016 18:46:25 +0000 (20:46 +0200)]
Handle ResizeRequests for tray clients. (#2495)
Some tray clients such as VLC use override_redirect on their tray window. As per
specification this means i3bar won't receive a ConfigureRequest, but instead a
ResizeRequest will be triggered. If not selected, the X server will simply confirm
the request which leads to a broken tray window size.
This commit selects and handles the event just like a configure request is handled.
The idea was to ensure the symbol would always be present. For that, we need
__attribute__((used)), not __attribute__((unused)). Further, ensure the
variable has static storage, as the used attribute only applies to variables
with static storage. See also http://stackoverflow.com/a/29545417/712014
Remove conditional compilation for cairo/pangocairo (#2480)
We strive to avoid conditional compilation in i3 as much as possible.
cairo and pangocairo have been around long enough in the versions that
we need that it’s time to unconditionally depend on them.
Also update DEPENDS with the last-known-good-versions while at it.
Handle _NET_ACTIVE_WINDOW for scratchpad windows. (#2458)
We call scratchpad_show() on _NET_ACTIVE_WINDOW requests if the request
came from a pager. This is consistent with the i3 »focus« command because
we assume the user requested to see the window, so we do the only
sensible thing.
Check output crossing on ENTER_NOTIFY to dockarea. (#2477)
When receiving an ENTER_NOTIFY event on a dock client we returned as to not
focus the dock client (cf. #321 and #323). However, we still need to check
for crossing output boundaries and if that happened focus the new output.
Otherwise it can happen that the cursor is on a different output than the
focused output. When opening a window, this would open it on the old output
and then warp the mouse there. This effect will be even worse if the window
is immediately moved with 'move position mouse' as the window will end up
in its correct position on the new output and the cursor warped to the old
output.
We add $HOME to the environment variables we define for a test case
in order to redirect it from the user's actual home directory. This
is necessary because xcb-util-xrm will fall back to $HOME/.Xresources
when determining the DPI. If a user has this set to, e.g., 192 on their
machine, this would break tests.
Since tests shouldn't rely on the system they run in, we redirect the
home directory altogether to simulate a clean slate.
When I3SOCK is present, socket_path might be a pointer to an
environment variable, which cannot be free'd in line 157. This
commit duplicates the string if I3SOCK is present, thus making
socket_path a free-able pointer again.
* Using the static default colormap we determine on startup if the
con has the corresponding depth. This avoids creating pointless
colormaps.
* Not freeing the default colormap to not have stray colormaps on
containers. This fixes an issue with certain programs such as xwd.
* Creating a custom colormap when necessary and freeing it when the
container is killed.
projects / i3 / i3 / log