Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use xcb-errors library if it is available #2665

Merged
merged 2 commits into from
Mar 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,15 @@ jobs:
- *BASE_PACKAGES
- liblua5.3-dev
- lua5.3
- env: LUA=5.1 LUANAME=lua5.1 BUILD_IN_DIR=/tmp/awesome-build
- env: LUA=5.1 LUANAME=lua5.1 BUILD_IN_DIR=/tmp/awesome-build WITH_XCB_ERRORS=yes
Copy link
Member

@blueyed blueyed Feb 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for not noticing earlier, but it would be better to add this to the DO_COVERAGE=codecov job.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went ahead and amended your last commit.

addons:
apt:
packages:
- *BASE_PACKAGES
- liblua5.1-dev
- lua5.1
# For xcb-errors
- xcb-proto
- env: LUA=5.1 LUANAME=luajit-2.0 LUALIBRARY=/usr/lib/x86_64-linux-gnu/libluajit-5.1.so LUAROCKS_ARGS=--lua-suffix=jit-2.0.4
addons:
apt:
Expand Down Expand Up @@ -110,6 +112,16 @@ install:
./autogen.sh --prefix=/usr
make && sudo make install)

# Install xcb-errors if needed
- |
set -e
if [[ "$WITH_XCB_ERRORS" == "yes" ]]; then
git clone --depth 1 --recursive https://gitlab.freedesktop.org/xorg/lib/libxcb-errors.git /tmp/xcb-errors
(cd /tmp/xcb-errors
./autogen.sh --prefix=/usr
make && sudo make install)
psychon marked this conversation as resolved.
Show resolved Hide resolved
fi

- |
# Install Lua (per env).
if [[ "$LUANAME" == "luajit-2.0" ]]; then
Expand Down
8 changes: 8 additions & 0 deletions awesome.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ awesome_atexit(bool restart)

/* Disconnect *after* closing lua */
xcb_cursor_context_free(globalconf.cursor_ctx);
#ifdef WITH_XCB_ERRORS
xcb_errors_context_free(globalconf.errors_ctx);
#endif
xcb_disconnect(globalconf.connection);

close(sigchld_pipe[0]);
Expand Down Expand Up @@ -751,6 +754,11 @@ main(int argc, char **argv)
globalconf.visual->visual_id);
}

#ifdef WITH_XCB_ERRORS
if (xcb_errors_context_new(globalconf.connection, &globalconf.errors_ctx) < 0)
fatal("Failed to initialize xcb-errors");
#endif

/* Get a recent timestamp */
acquire_timestamp();

Expand Down
11 changes: 11 additions & 0 deletions awesomeConfig.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ autoOption(GENERATE_MANPAGES "generate manpages")
option(COMPRESS_MANPAGES "compress manpages" ON)
option(GENERATE_DOC "generate API documentation" ON)
option(DO_COVERAGE "build with coverage" OFF)
autoOption(WITH_XCB_ERRORS "build with xcb-errors")
if (GENERATE_DOC AND DO_COVERAGE)
message(STATUS "Not generating API documentation with DO_COVERAGE")
set(GENERATE_DOC OFF)
Expand Down Expand Up @@ -213,6 +214,16 @@ if(WITH_DBUS)
autoDisable(WITH_DBUS "DBus not found.")
endif()
endif()

if(WITH_XCB_ERRORS)
pkg_check_modules(XCB_ERRORS xcb-errors)
if(XCB_ERRORS_FOUND)
set(AWESOME_OPTIONAL_LDFLAGS ${AWESOME_OPTIONAL_LDFLAGS} ${XCB_ERRORS_LDFLAGS})
set(AWESOME_OPTIONAL_INCLUDE_DIRS ${AWESOME_OPTIONAL_INCLUDE_DIRS} ${XCB_ERRORS_INCLUDE_DIRS})
else()
autoDisable(WITH_XCB_ERRORS "xcb-errors not found.")
endif()
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this default to AUTO, and cause an error if ON was used, but it is not available?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no AUTO for option(). It is a TRUE / FALSE thing. I briefly looked into it (it is possible, just not with option()) and then decided to just copy what we do for DBus, since apparently that was good enough so far.

If you disagree, I can look into implementing AUTO for WITH_DBUS, GENERATE_MANPAGES, GENERATE_DOC, and anything else that needs this. But I'd do that in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, just a quick idea.
I think it would be nice in general, but not really necessary.

# }}}

# {{{ Install path and configuration variables
Expand Down
8 changes: 7 additions & 1 deletion common/version.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ eprint_version(void)
#else
const char *has_dbus = "✘";
#endif
#ifdef WITH_XCB_ERRORS
const char *has_xcb_errors = "✔";
#else
const char *has_xcb_errors = "✘";
blueyed marked this conversation as resolved.
Show resolved Hide resolved
#endif
#ifdef HAS_EXECINFO
const char *has_execinfo = "✔";
#else
Expand All @@ -63,12 +68,13 @@ eprint_version(void)
printf("awesome %s (%s)\n"
" • Compiled against %s (running with %s)\n"
" • D-Bus support: %s\n"
" • xcb-errors support: %s\n"
" • execinfo support: %s\n"
" • xcb-randr version: %d.%d\n"
" • LGI version: %s\n",
AWESOME_VERSION, AWESOME_RELEASE,
LUA_RELEASE, lua_tostring(L, -2),
has_dbus, has_execinfo,
has_dbus, has_xcb_errors, has_execinfo,
XCB_RANDR_MAJOR_VERSION, XCB_RANDR_MINOR_VERSION,
lua_tostring(L, -1));
lua_close(L);
Expand Down
1 change: 1 addition & 0 deletions config.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#define AWESOME_DEFAULT_CONF "@AWESOME_SYSCONFDIR@/rc.lua"

#cmakedefine WITH_DBUS
#cmakedefine WITH_XCB_ERRORS
#cmakedefine HAS_EXECINFO

#endif //_CONFIG_H_
Expand Down
2 changes: 2 additions & 0 deletions docs/01-readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ Additionally, the following optional dependencies exist:
generate slightly better backtraces on crashes
- `Xephyr` or `Xvfb` for running integration tests
- [GTK+ >= 3.10](https://www.gtk.org/) for `./themes/gtk/`
- [xcb-errors](https://gitlab.freedesktop.org/xorg/lib/libxcb-errors) for
pretty-printing of X11 errors

## Running Awesome

Expand Down
20 changes: 17 additions & 3 deletions event.c
Original file line number Diff line number Diff line change
Expand Up @@ -1036,10 +1036,24 @@ xerror(xcb_generic_error_t *e)
&& e->major_code == XCB_CONFIGURE_WINDOW))
return;

warn("X error: request=%s (major %d, minor %d), error=%s (%d)",
xcb_event_get_request_label(e->major_code),
#ifdef WITH_XCB_ERRORS
const char *major = xcb_errors_get_name_for_major_code(
globalconf.errors_ctx, e->major_code);
const char *minor = xcb_errors_get_name_for_minor_code(
globalconf.errors_ctx, e->major_code, e->minor_code);
const char *extension = NULL;
const char *error = xcb_errors_get_name_for_error(
globalconf.errors_ctx, e->error_code, &extension);
#else
const char *major = xcb_event_get_request_label(e->major_code);
const char *minor = NULL;
const char *extension = NULL;
const char *error = xcb_event_get_error_label(e->error_code);
#endif
warn("X error: request=%s%s%s (major %d, minor %d), error=%s%s%s (%d)",
major, minor == NULL ? "" : "-", NONULL(minor),
e->major_code, e->minor_code,
xcb_event_get_error_label(e->error_code),
NONULL(extension), extension == NULL ? "" : "-", error,
e->error_code);

return;
Expand Down
9 changes: 9 additions & 0 deletions globalconf.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@
#include <xcb/xcb_xrm.h>
#include <X11/Xresource.h>

#include "config.h"
#ifdef WITH_XCB_ERRORS
#include <xcb/xcb_errors.h>
#endif

#include "objects/key.h"
#include "common/xembed.h"
#include "common/buffer.h"
Expand Down Expand Up @@ -82,6 +87,10 @@ typedef struct
int default_screen;
/** xcb-cursor context */
xcb_cursor_context_t *cursor_ctx;
#ifdef WITH_XCB_ERRORS
/** xcb-errors context */
xcb_errors_context_t *errors_ctx;
#endif
/** Keys symbol table */
xcb_key_symbols_t *keysyms;
/** Logical screens */
Expand Down