From d5498c8a8a4b2c332eaa7855eb88f3e3432edaed Mon Sep 17 00:00:00 2001 From: patrick96 Date: Sat, 5 Nov 2022 12:21:08 +0100 Subject: [PATCH 1/5] script: Require zero exit code to show empty module. Showing an empty module if the script failed but produced no output does not make too much sense. Ref #2857 --- CHANGELOG.md | 5 ++++- src/modules/script.cpp | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a901db9c..529b0e04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Breaking -- `custom/script`: now doesn't hide failing script if it's output is not changing ([`#2636`](https://github.com/polybar/polybar/issues/2636)). Somewhat similar behaviour can be imitated with `format-fail`, if necessary. +- `custom/script`: + - now doesn't hide failing script if it's output is not changing ([`#2636`](https://github.com/polybar/polybar/issues/2636)). Somewhat similar behaviour can be imitated with `format-fail`, if necessary. + - If the `exec` command produced no output and exited with a non-zero exit code the module is no longer completely empty, but just has an empty `%output%` token. If you relied on this behavior to hide the module under certain circumstances, make sure the script exits with an exit code of zero. ([`#2857`](https://github.com/polybar/polybar/discussions/2857), [`#2861`](https://github.com/polybar/polybar/pull/2861)) ### Build - Respect `CMAKE_INSTALL_PREFIX` when installing default config ([`#2770`](https://github.com/polybar/polybar/pull/2770)) @@ -33,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `enable-struts` option in bar section to enable/disable struts ([`#2769`](https://github.com/polybar/polybar/issues/2769), [`#2844`](https://github.com/polybar/polybar/pull/2844)) by [@VanillaViking](https://github.com/VanillaViking). ### Changed +- `custom/script`: No longer produces a completely empty module if the `exec` command failed. It only produces an empty module if the script had a zero exit code. ([`#2857`](https://github.com/polybar/polybar/discussions/2857), [`#2861`](https://github.com/polybar/polybar/pull/2861)) - `internal/fs`: Use `/` as a fallback if no mountpoints are specified ([`#2572`](https://github.com/polybar/polybar/issues/2572), [`#2705`](https://github.com/polybar/polybar/pull/2705)) - `internal/backlight`: - Detect backlight if none specified ([`#2572`](https://github.com/polybar/polybar/issues/2572), [`#2728`](https://github.com/polybar/polybar/pull/2728)) diff --git a/src/modules/script.cpp b/src/modules/script.cpp index ecdb0f0a..4ff9bebf 100644 --- a/src/modules/script.cpp +++ b/src/modules/script.cpp @@ -93,7 +93,7 @@ namespace modules { m_exit_status = data.exit_status; - if (data.output.empty()) { + if (data.output.empty() && m_exit_status == 0) { return ""; } From edf37385cb2e0e4d3c848d188473e48df618ed1a Mon Sep 17 00:00:00 2001 From: patrick96 Date: Mon, 12 Dec 2022 00:15:04 +0100 Subject: [PATCH 2/5] script: Bump poll timeout to 250ms The module has a poll timeout because it needs to periodically check if it is shutting down. Otherwise, it would be stuck polling and the bar couldn't shut down until the script produces a new line. However, this causes the bar to wake up intermittently (currently ~40/s) due to the 25ms timeout. Bumping this to 250ms still gives us timely shut downs and caps the number of wake ups to 4/s. This is only a stop-gap solution, ideally the script runner is integrated into the main event loop and uses its polling handles which don't have to wake up to check for a shutdown. Ref #1778 Ref #2337 --- CHANGELOG.md | 4 +++- src/adapters/script_runner.cpp | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 529b0e04..4d115955 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `enable-struts` option in bar section to enable/disable struts ([`#2769`](https://github.com/polybar/polybar/issues/2769), [`#2844`](https://github.com/polybar/polybar/pull/2844)) by [@VanillaViking](https://github.com/VanillaViking). ### Changed -- `custom/script`: No longer produces a completely empty module if the `exec` command failed. It only produces an empty module if the script had a zero exit code. ([`#2857`](https://github.com/polybar/polybar/discussions/2857), [`#2861`](https://github.com/polybar/polybar/pull/2861)) +- `custom/script`: + - No longer produces a completely empty module if the `exec` command failed. It only produces an empty module if the script had a zero exit code. ([`#2857`](https://github.com/polybar/polybar/discussions/2857), [`#2861`](https://github.com/polybar/polybar/pull/2861)) + - Bumped the script polling interval (not related to the `interval` setting) to decrease wakeups. Polybar may take slightly longer to shut down. [`#2879`](https://github.com/polybar/polybar/pull/2879) - `internal/fs`: Use `/` as a fallback if no mountpoints are specified ([`#2572`](https://github.com/polybar/polybar/issues/2572), [`#2705`](https://github.com/polybar/polybar/pull/2705)) - `internal/backlight`: - Detect backlight if none specified ([`#2572`](https://github.com/polybar/polybar/issues/2572), [`#2728`](https://github.com/polybar/polybar/pull/2728)) diff --git a/src/adapters/script_runner.cpp b/src/adapters/script_runner.cpp index 92dcbc11..c7ffb26a 100644 --- a/src/adapters/script_runner.cpp +++ b/src/adapters/script_runner.cpp @@ -109,7 +109,7 @@ script_runner::interval script_runner::run() { * For non-tailed scripts, we only use the first line. However, to ensure interruptability when the module shuts * down, we still need to continue polling. */ - if (io_util::poll_read(fd, 25) && !got_output) { + if (io_util::poll_read(fd, 250) && !got_output) { changed = set_output(cmd.readline()); got_output = true; } @@ -155,7 +155,7 @@ script_runner::interval script_runner::run_tail() { assert(fd != -1); while (!m_stopping && cmd.is_running() && !io_util::poll(fd, POLLHUP, 0)) { - if (io_util::poll_read(fd, 25)) { + if (io_util::poll_read(fd, 250)) { auto changed = set_output(cmd.readline()); if (changed) { From fc423c092143464062f77b83f78328e8ca96694a Mon Sep 17 00:00:00 2001 From: patrick96 Date: Thu, 5 Jan 2023 22:00:28 +0100 Subject: [PATCH 3/5] fix(i3): Deal with negative coordinates Pulls in fix in i3ipcpp repo: https://github.com/polybar/i3ipcpp/pull/12 Fixes https://github.com/polybar/polybar/issues/2888 Ref https://github.com/i3/i3/discussions/5352 --- CHANGELOG.md | 1 + lib/i3ipcpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d115955..62645e39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Error reporting for deprecated config values ([`#2724`](https://github.com/polybar/polybar/issues/2724)) - Also monitor include-files for changes when --reload is set ([`#675`](https://github.com/polybar/polybar/issues/675), [`#2759`](https://github.com/polybar/polybar/pull/2759)) - `internal/xwindow`: module does not crash when a tag is not provided in format ([`#2826`](https://github.com/polybar/polybar/issues/2826), [`#2833`](https://github.com/polybar/polybar/pull/2833)) by [@VanillaViking](https://github.com/VanillaViking) +- `internal/i3`: module errors when i3 has negative gaps ([`#2888`](https://github.com/polybar/polybar/issues/2888), [`#2889`](https://github.com/polybar/polybar/pull/2889)) ## [3.6.3] - 2022-05-04 ### Fixed diff --git a/lib/i3ipcpp b/lib/i3ipcpp index 86ddf710..85b444c0 160000 --- a/lib/i3ipcpp +++ b/lib/i3ipcpp @@ -1 +1 @@ -Subproject commit 86ddf7102c6903ae0cc543071e2d375403fc0727 +Subproject commit 85b444c06267fbae8e9e06877cae6f03b42acbfb From b3cbf0a64439442e410e7762a664c98db7c58d0f Mon Sep 17 00:00:00 2001 From: patrick96 Date: Mon, 13 Feb 2023 21:51:18 +0100 Subject: [PATCH 4/5] ci: Update codecov and checkout actions The coverage job fails because of the outdated codecov action and the checkout v2 action uses a deprecated version of nodejs --- .github/workflows/ci.yml | 14 +++++++------- .github/workflows/release.yml | 8 +++++--- src/x11/xresources.cpp | 2 ++ 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ec6ae5c..2a769b3f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: env: COLOR: "ON" steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: ref: ${{ github.event.inputs.ref }} - name: Install Dependencies @@ -48,7 +48,7 @@ jobs: python3-xcbgen \ libuv1-dev \ xcb-proto - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: submodules: true ref: ${{ github.event.inputs.ref }} @@ -115,7 +115,7 @@ jobs: if [ "$POLYBAR_BUILD_TYPE" = "tests" ]; then sudo apt-get install -y lcov fi - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: submodules: true ref: ${{ github.event.inputs.ref }} @@ -125,7 +125,7 @@ jobs: run: ./common/ci/configure.sh - name: Build run: | - cd $BUILD_DIR + cd "$BUILD_DIR" make - name: Collect initial coverage if: ${{ matrix.polybar_build_type == 'tests' }} @@ -134,17 +134,17 @@ jobs: - name: Tests if: ${{ matrix.polybar_build_type == 'tests' }} run: | - cd $BUILD_DIR + cd "$BUILD_DIR" make check - name: Collect coverage if: ${{ matrix.polybar_build_type == 'tests' }} run: | lcov --capture --no-external --directory . -o cov_tests.info lcov --add-tracefile cov_base.info --add-tracefile cov_tests.info -o cov_total.info - lcov --remove cov_total.info ${PWD}'/build/*' ${PWD}'/tests/*' ${PWD}'/lib/*' -o cov.info + lcov --remove cov_total.info "${PWD}/build/*" "${PWD}/tests/*" "${PWD}/lib/*" -o cov.info - name: Upload Coverage if: ${{ matrix.polybar_build_type == 'tests' }} - uses: codecov/codecov-action@v2 + uses: codecov/codecov-action@v3 with: flags: unittests files: ./cov.info diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3a8d34c5..4ac2f512 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -31,9 +31,11 @@ jobs: RELEASE_TAG=${GITHUB_REF#refs/tags/} fi echo "Publishing Version $RELEASE_TAG" - echo "RELEASE_TAG=$RELEASE_TAG" >> "$GITHUB_ENV" - echo "POLYBAR_DIR=polybar-$RELEASE_TAG" >> "$GITHUB_ENV" - echo "POLYBAR_ARCHIVE=polybar-$RELEASE_TAG.tar.gz" >> "$GITHUB_ENV" + { + echo "RELEASE_TAG=$RELEASE_TAG" + echo "POLYBAR_DIR=polybar-$RELEASE_TAG" + echo "POLYBAR_ARCHIVE=polybar-$RELEASE_TAG.tar.gz" + } >> "$GITHUB_ENV" # Checks out the target tag - uses: actions/checkout@v2 diff --git a/src/x11/xresources.cpp b/src/x11/xresources.cpp index 49d4aa2e..4f4e5d53 100644 --- a/src/x11/xresources.cpp +++ b/src/x11/xresources.cpp @@ -1,5 +1,7 @@ #include "x11/xresources.hpp" +#include "common.hpp" + POLYBAR_NS template <> From 8cc1b4fcfda58d64921476b9599c84064815d3a4 Mon Sep 17 00:00:00 2001 From: patrick96 Date: Mon, 13 Feb 2023 21:08:48 +0100 Subject: [PATCH 5/5] fix(build): Use CMAKE_INSTALL_FULL_ for default config Using CMAKE_INSTALL_SYSCONFDIR does respect CMAKE_INSTALL_PREFIX, but it prefixes it to CMAKE_INSTALL_SYSCONFDIR, which results in the default config being installed to /usr/etc/polybar/config.ini or /usr/local/etc/polybar/config.ini CMAKE_INSTALL_FULL_SYSCONFDIR gives an absolute path that respects the prefix but does the right thing (uses /etc) if it is /usr Ref: #2770 --- CHANGELOG.md | 3 ++- CMakeLists.txt | 11 +++++++++-- cmake/05-summary.cmake | 11 ++++++----- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62645e39..5703aaac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - If the `exec` command produced no output and exited with a non-zero exit code the module is no longer completely empty, but just has an empty `%output%` token. If you relied on this behavior to hide the module under certain circumstances, make sure the script exits with an exit code of zero. ([`#2857`](https://github.com/polybar/polybar/discussions/2857), [`#2861`](https://github.com/polybar/polybar/pull/2861)) ### Build -- Respect `CMAKE_INSTALL_PREFIX` when installing default config ([`#2770`](https://github.com/polybar/polybar/pull/2770)) +- Respect `CMAKE_INSTALL_PREFIX` when installing default config ([`#2770`](https://github.com/polybar/polybar/pull/2770), [`#2917`](https://github.com/polybar/polybar/pull/2917)) +- Change default `CMAKE_INSTALL_PREFIX` to `/usr`. Installations with default flags will now go into `/usr` instead of `/usr/local` ([`#2917`](https://github.com/polybar/polybar/pull/2917)) - Bump C++ version to C++17 ([`#2847`](https://github.com/polybar/polybar/pull/2847)) ### Deprecated diff --git a/CMakeLists.txt b/CMakeLists.txt index 0979f8af..acb58b6b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,6 +22,14 @@ else() set(APP_VERSION "${version_txt}") endif() +# Set the default installation prefix to /usr +# Otherwise the default value is /usr/local which causes the default config +# file to be installed to /usr/local/etc, with /usr, cmake has special handling +# for this. +if(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT) + set(CMAKE_INSTALL_PREFIX "/usr" CACHE PATH "Project-default installation prefix" FORCE) +endif() + list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake ${PROJECT_SOURCE_DIR}/cmake/common @@ -60,10 +68,9 @@ if(BUILD_TESTS) add_subdirectory(tests) endif() - if(BUILD_CONFIG) install(FILES ${CMAKE_SOURCE_DIR}/doc/config.ini - DESTINATION ${CMAKE_INSTALL_SYSCONFDIR}/${PROJECT_NAME} + DESTINATION ${CMAKE_INSTALL_FULL_SYSCONFDIR}/${PROJECT_NAME} COMPONENT config) endif() diff --git a/cmake/05-summary.cmake b/cmake/05-summary.cmake index 5f71936f..c649647d 100644 --- a/cmake/05-summary.cmake +++ b/cmake/05-summary.cmake @@ -13,11 +13,12 @@ if (BUILD_DOC) endif() message(STATUS " Install Paths:") -message_colored(STATUS " PREFIX: ${CMAKE_INSTALL_PREFIX}" "32") -message_colored(STATUS " BINDIR: ${CMAKE_INSTALL_FULL_BINDIR}" "32") -message_colored(STATUS " DATADIR: ${CMAKE_INSTALL_FULL_DATADIR}" "32") -message_colored(STATUS " DOCDIR: ${CMAKE_INSTALL_FULL_DOCDIR}" "32") -message_colored(STATUS " MANDIR: ${CMAKE_INSTALL_FULL_MANDIR}" "32") +message_colored(STATUS " PREFIX: ${CMAKE_INSTALL_PREFIX}" "32") +message_colored(STATUS " BINDIR: ${CMAKE_INSTALL_FULL_BINDIR}" "32") +message_colored(STATUS " DATADIR: ${CMAKE_INSTALL_FULL_DATADIR}" "32") +message_colored(STATUS " DOCDIR: ${CMAKE_INSTALL_FULL_DOCDIR}" "32") +message_colored(STATUS " MANDIR: ${CMAKE_INSTALL_FULL_MANDIR}" "32") +message_colored(STATUS " SYSCONFDIR: ${CMAKE_INSTALL_FULL_SYSCONFDIR}" "32") message(STATUS " Targets:") colored_option(" polybar" BUILD_POLYBAR)