Skip to content
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
9 changes: 6 additions & 3 deletions src/lib/fcitx-utils/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,19 @@
*/

#include "event.h"
#include <sys/types.h>
#include <cstdint>
#include <cstring>
#include <ctime>
#include <memory>
#include <stdexcept>
#include <utility>
#include "event_p.h"
#include "eventloopinterface.h"
#include "macros.h"

#if defined(_WIN32)
#include <pthread.h>
#endif

namespace fcitx {

class EventLoopPrivate {
Expand Down Expand Up @@ -69,7 +72,7 @@ bool EventLoop::exec() {

void EventLoop::exit() {
FCITX_D();
return d->impl_->exit();
d->impl_->exit();
}

std::unique_ptr<EventSourceIO> EventLoop::addIOEvent(int fd, IOEventFlags flags,
Expand Down
5 changes: 5 additions & 0 deletions src/lib/fcitx-utils/event.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#ifndef _FCITX_UTILS_EVENT_H_
#define _FCITX_UTILS_EVENT_H_

#include <sys/types.h>
#include <cstdint>
#include <functional>
#include <memory>
Expand All @@ -15,6 +16,10 @@
#include <fcitx-utils/flags.h>
#include <fcitx-utils/macros.h>

#if defined(_WIN32)
#include <pthread.h>
#endif

Comment on lines +19 to +22

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate relevant files =="
git ls-files | rg '(^|/)src/lib/fcitx-utils/(event\.h|event_libuv\.h|eventloopinterface\.h)$|CMakeLists\.txt|meson\.build|cmake|\.cmake$|config'

echo
echo "== headers snippets =="
for f in src/lib/fcitx-utils/event.h src/lib/fcitx-utils/event_libuv.h src/lib/fcetx-utils/eventloopinterface.h; do
  truecase=${f/src/lib/fcetx-utils/src/lib/fcitx-utils}
  if [ -f "$truecase" ]; then
    echo "--- $truecase ---"
    sed -n '1,80p' "$truecase" | cat -n
  fi
done

echo
echo "== include pthread occurrences =="
rg -n '`#include` <pthread\.h>|pthread_t|pthread_' src include CMakeLists.txt meson.build .github 2>/dev/null || true

echo
echo "== cmake/meson target definitions mentioning fcitx-utils/libuv/event =="
rg -n 'fcitx-utils|fcitx-utils.*(event|interface)|Pthread|thread|LIBUV|libuv|pthread|h' -g 'CMakeLists.txt' -g '*.cmake' -g 'meson.build' -g 'CMakeLists.txt' .

Repository: fcitx/fcitx5

Length of output: 43046


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== src/lib/fcitx-utils/CMakeLists.txt relevant sections =="
sed -n '1,220p' src/lib/fcetx-utils/CMakeLists.txt | cat -n || sed -n '1,220p' src/lib/fcitx-utils/CMakeLists.txt | cat -n

echo
echo "== FindPthread.cmake full =="
sed -n '1,90p' cmake/FindPthread.cmake | cat -n

echo
echo "== Fcitx5Macros export helpers =="
sed -n '1,120p' src/lib/fcetx-utils/Fcitx5Macros.cmake | cat -n || sed -n '1,120p' src/lib/fcitx-utils/Fcitx5Macros.cmake | cat -n

echo
echo "== consumer link to Fcitx5Utils and pthread =="
rg -n 'Fcitx5Utils|Fcitx5::Utils|Fcitx5::Utils|Fcitx5::Utils|Fcitx5::Utils|Pthread::Pthread|pthread\.h' src test modules config -g '*CMakeLists.txt' -g '*.h' -g '*.cpp' 2>/dev/null || true

Repository: fcitx/fcitx5

Length of output: 21724


🌐 Web query:

CMake target_link_libraries PRIVATE INTERFACE PUBLIC INTERFACE_INCLUDE_DIRECTORIES exported targets include directories

💡 Result:

In CMake, the keywords PRIVATE, PUBLIC, and INTERFACE define how dependencies and their usage requirements (such as include directories) are propagated to other targets [1][2]. When using target_link_libraries, these keywords control the visibility of the link dependency and the propagation of its usage requirements [1][3]. PRIVATE The dependency is used only by the target itself for its own implementation [2][3]. It is not part of the target's public link interface, so consumers of the target do not need to link against this dependency, nor do they inherit its usage requirements (such as INTERFACE_INCLUDE_DIRECTORIES) [1][2]. INTERFACE The dependency is not linked into the target itself, but it is part of the target's link interface [1][3]. This means consumers of the target will inherit the dependency's usage requirements (including its INTERFACE_INCLUDE_DIRECTORIES) and will link against it, but the target itself does not use it for its own implementation [2][3]. PUBLIC This is a combination of PRIVATE and INTERFACE [2]. The dependency is linked into the target, and it is also part of the target's public link interface [1][3]. Consequently, both the target and any consumers of the target will link against it and inherit its usage requirements [1][2]. INTERFACE_INCLUDE_DIRECTORIES and Exported Targets The INTERFACE_INCLUDE_DIRECTORIES property is key to how CMake handles transitive usage requirements [4][5]. When a target is exported via install(EXPORT) or export, CMake automatically generates IMPORTED targets [5]. These generated targets carry the appropriate usage requirements—such as INTERFACE_INCLUDE_DIRECTORIES—so that when a consumer links to these imported targets using target_link_libraries, CMake automatically provides the correct include paths required to compile against the dependency [4][5]. Best Practices It is highly recommended to always specify one of these keywords when using target_link_libraries, as relying on the legacy keywordless signature (which defaults to PUBLIC) can lead to unintended propagation of dependencies and headers [6][3]. When setting include directories, use target_include_directories with the same scoping keywords (PRIVATE, PUBLIC, INTERFACE) to ensure the target's own headers are properly exposed to its consumers [7][8]. Avoid hard-coding absolute paths to dependencies in your export configurations; instead, rely on the INTERFACE_INCLUDE_DIRECTORIES properties of the imported dependency targets themselves [7][5][8].

Citations:


Make Pthread::Pthread part of Fcitx5Utils’ public usage requirements.

src/lib/fcitx-utils/CMakeLists.txt:153 links Pthread::Pthread with PRIVATE, so downstream targets that only consume Fcitx5::Utils do not inherit ${PTHREAD_INCLUDE_DIR}. The public headers include <pthread.h> under _WIN32 at src/lib/fcitx-utils/event.h, src/lib/fcitx-utils/event_libuv.h, and src/lib/fcitx-utils/eventloopinterface.h, so use PUBLIC here or link consumers directly.

📍 Affects 3 files
  • src/lib/fcitx-utils/event.h#L19-L22 (this comment)
  • src/lib/fcitx-utils/event_libuv.h#L22-L25
  • src/lib/fcitx-utils/eventloopinterface.h#L19-L22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/fcitx-utils/event.h` around lines 19 - 22, Update the Fcitx5Utils
CMake target’s Pthread::Pthread linkage from PRIVATE to PUBLIC so consumers of
Fcitx5::Utils inherit the required pthread include configuration. This applies
to the public-header usage in src/lib/fcitx-utils/event.h lines 19-22,
src/lib/fcitx-utils/event_libuv.h lines 22-25, and
src/lib/fcitx-utils/eventloopinterface.h lines 19-22; no direct changes are
required in those headers.

namespace fcitx {

using EventLoopFactory = std::function<std::unique_ptr<EventLoopInterface>()>;
Expand Down
13 changes: 11 additions & 2 deletions src/lib/fcitx-utils/event_libuv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

#include "event_libuv.h"
#include <sys/types.h>
#include <cstdint>
#include <cstdlib>
#include <ctime>
Expand All @@ -21,18 +22,26 @@
#include "log.h"
#include "trackableobject.h"

#if defined(_WIN32)
#include <pthread.h>
#endif

#define FCITX_LIBUV_DEBUG() FCITX_LOGC(::fcitx::libuv_logcategory, Debug)

namespace fcitx {

namespace {

FCITX_DEFINE_LOG_CATEGORY(libuv_logcategory, "libuv");

}

std::unique_ptr<EventLoopInterface> createDefaultEventLoop() {
return std::make_unique<EventLoopLibUV>();
}

const char *defaultEventLoopImplementation() { return "libuv"; }

FCITX_DEFINE_LOG_CATEGORY(libuv_logcategory, "libuv");

static int IOEventFlagsToLibUVFlags(IOEventFlags flags) {
int result = 0;
if (flags & IOEventFlag::In) {
Expand Down
9 changes: 6 additions & 3 deletions src/lib/fcitx-utils/event_libuv.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#ifndef _FCITX_UTILS_EVENT_LIBUV_H_
#define _FCITX_UTILS_EVENT_LIBUV_H_

#include <sys/types.h>
#include <cstdint>
#include <cstdlib>
#include <memory>
Expand All @@ -18,6 +19,10 @@
#include <fcitx-utils/trackableobject.h>
#include <uv.h>

#if defined(_WIN32)
#include <pthread.h>
#endif

namespace fcitx {

struct UVLoop {
Expand Down Expand Up @@ -95,9 +100,7 @@ struct LibUVSource : public Interface, public LibUVSourceBase {
return state_ == LibUVSourceEnableState::Oneshot;
}

inline HandleType *handle() {
return reinterpret_cast<HandleType *>(handle_);
}
HandleType *handle() { return reinterpret_cast<HandleType *>(handle_); }

void init(uv_loop_t *loop) override {
handle_ = static_cast<uv_handle_t *>(calloc(1, sizeof(HandleType)));
Expand Down
2 changes: 2 additions & 0 deletions src/lib/fcitx-utils/event_sdevent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*
*/

#include <sys/types.h>
#include <cstdint>
#include <cstdlib>
#include <exception>
Expand All @@ -15,6 +16,7 @@
#include <utility>
#include <sys/epoll.h>
#include "eventloopinterface.h"
#include "fs.h"
#include "log.h"
#include "macros.h"
#include "misc_p.h"
Expand Down
5 changes: 3 additions & 2 deletions src/lib/fcitx-utils/eventloopinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <cstring>
#include <ctime>
#include <stdexcept>
#include <string>
#include <format>

namespace fcitx {
Expand Down Expand Up @@ -39,8 +40,8 @@ uint64_t timespec_load(const struct timespec *ts) {
return USEC_INFINITY;
}

return (uint64_t)ts->tv_sec * USEC_PER_SEC +
(uint64_t)ts->tv_nsec / NSEC_PER_USEC;
return ((uint64_t)ts->tv_sec * USEC_PER_SEC) +
((uint64_t)ts->tv_nsec / NSEC_PER_USEC);
}

uint64_t now(clockid_t clock_id) {
Expand Down
5 changes: 5 additions & 0 deletions src/lib/fcitx-utils/eventloopinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#ifndef _FCITX_UTILS_EVENTLOOPINTERFACE_H_
#define _FCITX_UTILS_EVENTLOOPINTERFACE_H_

#include <sys/types.h>
#include <cstdint>
#include <functional>
#include <memory>
Expand All @@ -15,6 +16,10 @@
#include <fcitx-utils/flags.h>
#include <fcitx-utils/macros.h>

#if defined(_WIN32)
#include <pthread.h>
#endif

namespace fcitx {

enum class IOEventFlag {
Expand Down
6 changes: 6 additions & 0 deletions test/testcustomeventloop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

#include <poll.h>
#include <sys/types.h>
#include <unistd.h>
#include <algorithm>
#include <cassert>
Expand All @@ -18,13 +19,18 @@
#include "fcitx-utils/event.h"
#include "fcitx-utils/eventdispatcher.h"
#include "fcitx-utils/eventloopinterface.h"
#include "fcitx-utils/fs.h"
#include "fcitx-utils/intrusivelist.h"
#include "fcitx-utils/macros.h"
#include "fcitx-utils/misc_p.h"
#include "fcitx-utils/trackableobject.h"
#include "fcitx-utils/unixfd.h"
#include "eventlooptests.h"

#if defined(_WIN32)
#include <pthread.h>
#endif

using namespace fcitx;

// Following is also an example of poll() based event loop
Expand Down
Loading