Commit Graph

8013 Commits

Author SHA1 Message Date
csoler
2f093fd7b3
Merge pull request #383 from jolavillette/fix/distant-chat-locally-closed-event
Distant chat: tell every client when a conversation is closed locally
2026-09-03 18:46:18 +02:00
jolavillette
21f01cae18 Distant chat: tell every client when a conversation is closed locally
closeDistantChatConnexion() posted no event. The client that called it knew;
the others did not: a conversation opened and then left from the web UI stayed
open -- green LED and all -- in the desktop window the core had popped for it.
Polling cannot fix that: once the tunnel is gone, getDistantChatStatus()
answers the same false for a conversation the peer closed (where the window
should stay open) and for one we closed from another client (where it should
go); only an explicit event tells the two apart.

This adds RsDistantChatEventCode::TUNNEL_STATUS_LOCALLY_CLOSED (0x05) and
posts it from closeDistantChatConnexion() whenever something was actually
closed, with the tunnel id, so every client of this core -- desktop GUI, web
UI, JSON API users -- drops the conversation at the same time. The remote
close path is untouched: markDistantChatAsClosed() stays as it was, and
REMOTELY_CLOSED keeps coming from notifyTunnelStatus() alone.

GUI counterpart: RetroShare/RetroShare#3297 (closes the popup on this event).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-03 07:34:48 +02:00
jolavillette
24b6ef5d1b Distant chat: closing a conversation left its contact behind, and always said it worked
closeDistantChatConnexion() closes the GXS tunnel and stops there, on a comment
wondering whether the contact should go too ("also remove contact. Or do we
wait for the notification?"). It should: the entry in mDistantChatContacts is
the core's record of an open conversation -- handleOutgoingItem() accepts
outgoing items for as long as it is there -- and markDistantChatAsClosed(),
the remote-close path, removes it. Nothing removed it when we closed the
conversation ourselves, and the web UI showed it: a conversation left with
"Leave Chat" kept coming back.

The return value was the constant true, so every caller -- the chat window, the
web UI, any JSON API client -- was told the conversation had been closed even
when there was nothing to close: closeExistingTunnel() answers false and logs
"Cannot close distant tunnel connection. No connection openned for tunnel id"
once p3GxsTunnelService has pruned a remotely-closed tunnel, which it does on
its own ~20s after the close arrives. And the chat-level item that would have
removed the contact on the remote-close path lost its sender in 2015
(c521e4ed1), so a still-registered contact with no tunnel behind it is the
normal state of a conversation whose peer has left. The function now answers
whether anything was actually released -- tunnel, contact, or both -- and logs
an error in the one inconsistent case, a closed tunnel with no registered
contact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-03 07:33:42 +02:00
Gioacchino Mazzurco
96d44bcbc7
Merge pull request #377 from jolavillette/fix/android-tiff-find-libm
Android: tell libtiff where libm is
2026-08-30 10:52:38 +02:00
Gioacchino Mazzurco
c4c2374d73
Merge pull request #376 from jolavillette/fix/android-rnp-probe-libdl
Android: link libdl into rnp's OpenSSL feature probe
2026-08-30 10:50:52 +02:00
jolavillette
2be2034bf6 Android: reword the libdl comment and point at the upstream rnp PR
The ordering claim only holds for linkers that resolve archives in a single
pass; the NDK's lld picks libdl.a up either way. Reference rnpgp/rnp#2473 so
the sed can be dropped once it lands.
2026-08-30 10:38:05 +02:00
Gioacchino Mazzurco
03fee47b3d
Merge pull request #338 from SchlauFuchs/fix/botan3-fallback
Auto-detect botan-3 vs botan-2 for librnp linking on Unix
2026-08-30 09:35:48 +02:00
Gioacchino Mazzurco
b5c056c160
Merge pull request #371 from defnax/remove-outdated-workflow
removed none functional workflow
2026-08-30 09:21:57 +02:00
Gioacchino Mazzurco
9a33138dff
Merge pull request #375 from jolavillette/fix/android-use-prebuilt-rnp
Android: enforce using the librnp built for the sysroot instead of rebuilding it inline
2026-08-30 09:17:25 +02:00
Gioacchino Mazzurco
77cd0e2e37
Merge pull request #373 from jolavillette/fix/android-rnp-double-emulator
Android: skip the rnp cross-compile patch when upstream already has it
2026-08-30 09:13:48 +02:00
csoler
89ed59e5bc
Merge pull request #370 from jolavillette/fix/jsonapi-stale-doxygen-xml
build: wipe the doxygen xml dir before regenerating the jsonapi bindings
2026-08-29 20:57:20 +02:00
jolavillette
29620c6a02 build(jsonapi): drop stale doxygen header XML before regenerating the bindings
A build directory that predates the removal or rename of a public header
keeps failing with:

    jsonapi-includes.inl:11:10: fatal error: retroshare/rsmsgs.h:
    No such file or directory

Doxygen never purges its OUTPUT_DIRECTORY, and both generators emit one
#include per *_8h.xml file found there, built from that XML's
<location file=...>. So the XML of a header that no longer exists is
re-read on every rebuild and turned back into a dead #include, with the
matching dead wrappers in jsonapi-wrappers.inl.

The generated clean rule lists only the two .inl files, never the xml/
directory, so a clean rebuild does not cure it: only deleting the whole
build directory does.

Delete the *_8h.xml files right before invoking doxygen, in both build
systems. Only those files drive the generators; every other XML they read
is reached through a live header's freshly regenerated XML. This costs
nothing, doxygen re-parses its whole input on every run anyway.

cmake -E rm does not expand wildcards, so the CMake side does the glob in
a small script generated next to the doxyfile.

Reported by Cyril on the dev chat.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-28 11:45:15 +02:00
csoler
e7b20d7828
Merge pull request #381 from jolavillette/fix/rs-getpass-eof-spin
Stop rs_getpass() from spinning forever on a closed stdin
2026-08-28 11:41:07 +02:00
csoler
72d9d16833
Merge pull request #366 from jolavillette/fix/jsonapi-stop-api-before-plugins
Stop the JSON API before plugins delete their resource providers
2026-08-28 11:29:15 +02:00
jolavillette
dd3cf562b3 util: stop rs_getpass() from spinning forever on a closed stdin
`getch()` forwards `getchar()`, whose EOF is -1. The POSIX rs_getpass()
stored it in an `unsigned char`, where it becomes 0xFF and never equals
RETURN, so a closed stdin made the read loop append one 0xFF byte per
iteration for good.

Measured on retroshare-service built from master, started as
`-U <sslid> < /dev/null`: 23 MB of output in 90 seconds, one core at
100%, and the process does not answer SIGTERM -- the spin never returns
to the shutdown check, so only SIGKILL ends it. That is the behaviour a
service unit or a container gets today whenever the passphrase is asked
with no terminal attached, which also covers stdin redirected from
/dev/null and an ssh session dropping while the prompt is up.

`ch` is now an int and EOF ends the read with an empty result: the same
run then exits in under a second with a 2.5 kB log. The Windows branch
had the milder form of the same bug: a closed console returns EOF from
every `_getch()`, and the loop could only leave through the PASS_MAX
branch, so it returned 512 bytes of 0xFF as if they had been typed. It
now stops on EOF as well, and accepts '\n' next to '\r' so a redirected
stdin terminates the line.

An optional `eof` out parameter tells the caller the input stream ended,
which a plain empty string cannot express: without it, a caller looping
"passphrase cannot be empty, try again" spins just as hard one level up.
It defaults to nullptr, so existing calls are unchanged.

Also check tcgetattr(): it fails whenever stdin is not a terminal, and
t_old was then applied to stdin uninitialized.
2026-08-27 03:32:33 +02:00
jolavillette
7bbeb8bd69 Android: tell libtiff where libm is
tiff cmake/FindCMath.cmake probes pow(3) twice, first with no extra
library then with the result of find_library(NAMES m). On Android pow is
in libm, and the NDK keeps libm.so under the API level directory of the
sysroot, sysroot/usr/lib/<triple>/<api>/, which andro_cmake does not add
to CMAKE_LIBRARY_PATH. find_library returns NOTFOUND, the second probe
links exactly like the first one and both fail with

  ld.lld: error: undefined symbol: pow

so configure aborts on

  Could NOT find CMath (missing: CMath_pow)

taking build_cimg and build_phash down with it.

Pass CMath_LIBRARY explicitly rather than adding the API level directory
to the search path of every dependency, which would change how the other
ten already working ones resolve their libraries.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-17 09:39:38 +02:00
jolavillette
bd4485e6b8 Android: link libdl into rnp's OpenSSL feature probe
rnp builds a findopensslfeatures helper and runs it to enumerate the
OpenSSL features. When cross-compiling it links that helper statically so
it does not need the Android dynamic linker, but OpenSSL 1.1.1
libcrypto.a pulls in dso_dlfcn.o, which references the dl* API, and
CMake's FindOpenSSL does not add ${CMAKE_DL_LIBS} to the OpenSSL::Crypto
interface here. Nothing provides those symbols and build_librnp dies at

  ld.lld: error: undefined symbol: dlopen
  >>> referenced by dso_dlfcn.c
  >>>               dso_dlfcn.o:(dlfcn_load) in archive
  >>>               .../sysroot/usr/lib/libcrypto.a

  CMake Error at cmake/Modules/FindOpenSSLFeatures.cmake:149 (message):
    Error building findopensslfeatures

The NDK ships a static libdl.a defining dlopen/dlsym/dlclose/dlerror, so
link it explicitly. It is added as a linked library, not through
CMAKE_EXE_LINKER_FLAGS, because it must appear after libcrypto.a on the
link line for the static linker to resolve from it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-17 09:33:14 +02:00
jolavillette
44d5840c45 Android: use the librnp built for the sysroot instead of rebuilding it inline
prepare-toolchain-clang.sh builds librnp and installs it in the Android
sysroot, but our own configure ignores it and builds librnp a second time,
inline, whenever ../supportlibs/librnp exists -- which is the case as soon as
libretroshare is checked out inside the RetroShare super-project instead of
standalone.

That inline build cannot work: librnp's CMake compiles findopensslfeatures
and runs it to enumerate the OpenSSL features, and that binary is an Android
executable the build host cannot execute. Where binfmt/qemu picks it up, as
on WSL, it fails on the missing Android dynamic linker:

  CMake Error at supportlibs/librnp/cmake/Modules/FindOpenSSLFeatures.cmake:151:
    Error getting supported OpenSSL hashes: 255
    qemu-aarch64: Could not open '/system/bin/linker64': No such file or directory

Prefer the pre-built library on Android, falling back to the previous
behaviour when none is installed. Desktop builds are untouched: RS_ANDROID is
OFF there, so the local librnp source is still used exactly as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-17 09:08:59 +02:00
defnax
17a4c7efc8 removed none functional workflow 2026-08-16 21:28:07 +02:00
jolavillette
40d487caba Android: skip the rnp cross-compile patch when upstream already has it
rnp merged the CMAKE_CROSSCOMPILING_EMULATOR support that this script adds
by hand (rnpgp/rnp 0a0672af, merged 2026-07-31), and LIBRNP_SOURCE_VERSION
tracks origin/main, so the sed patches now land on top of the upstream code:
the emulator is prepended to FOF twice and the OpenSSL feature probe runs
"qemu-aarch64 /usr/bin/qemu-aarch64 build/findopensslfeatures", which dies
with

  CMake Error at cmake/Modules/FindOpenSSLFeatures.cmake:184 (message):
    Error getting supported OpenSSL hashes: 255
    qemu-aarch64: /usr/bin/qemu-aarch64: Invalid ELF image for this architecture

Guard the patch on the checkout not handling the emulator itself, so both
current main and an older pinned rnp still configure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-16 21:24:45 +02:00
jolavillette
1c6fa5916d fix(shutdown): stop the JSON API before plugins delete their providers
rsGlobalShutDown() stopped the JSON API almost last, after stopPlugins().
A plugin that registered a JsonApiResourceProvider deletes it in its stop(),
but the running restbed service still holds the restbed::Resource objects
that provider returned, and their handlers capture it. Any request served
between stopPlugins() and the fullstop at the end of the function therefore
dereferences freed memory.

The window is not theoretical: everything in between -- UPnP teardown, the
auto-proxy shutdown, all registered service threads, the RsServer tick
thread and the per-peer streamers -- can take tens of seconds, and a web
interface polls throughout.

Move the fullstop to the top of the function. It also keeps an API client
from touching the configuration after ConfigFinalSave(), and it must stay
outside the wasReady branch: retroshare-service and Android start the JSON
API before login, so a shutdown from that state has to stop it too.

Without this, a plugin has to restart the whole JSON API from its stop() to
make deleting its own provider safe, which costs a burst-protection wait and
brings the server back up in the middle of teardown.
2026-08-16 18:26:17 +02:00
csoler
339c765235
Merge pull request #368 from jolavillette/ci/cmake
CMake CI: build libretroshare on Linux, Windows and macOS
2026-08-16 16:55:39 +02:00
jolavillette
8d03ad6e7b ci(windows): drop the launcher, turn broadcast discovery off instead
The compiler launcher worked -- 135 objects went through it -- and then:

    c++.exe: fatal error: no input files

only on the longest command lines. Prefixing every compile with
"bash.exe <script> " pushes those past the CreateProcess limit, and what falls
off the end is the source file. Nothing to tune: the prefix is fixed and the
command lines are what they are.

So the C++98 target is avoided rather than fought: RS_BRODCAST_DISCOVERY=OFF
drops udp-discovery-cpp from the Windows build. Linux and macOS keep it on, and
the header of the file says why the three jobs differ, so nobody reads it as an
oversight.

This should be temporary. The fix belongs in libretroshare's CMakeLists, which
fetches that project at GIT_TAG "origin/master" -- a moving third party tip --
while the super-project pins a submodule and never meets the problem.
2026-08-15 12:14:31 +02:00
jolavillette
5365ecfa3c ci(windows): use MSYS2's bash, not the WSL stub
Windows Subsystem for Linux has no installed distributions.

ninja starts the launcher through CreateProcess, which resolves a bare "bash"
against the Windows PATH and finds C:\Windows\System32\bash.exe -- the WSL
launcher, not the shell this job runs in. Every compile died there.

The MSYS2 bash is now named explicitly, and both it and the script are passed
through cygpath -m so CMake, ninja and bash all read the same path without
backslash escaping.
2026-08-15 12:07:24 +02:00
jolavillette
a57060be5b ci(windows): call the launcher through bash
ninja: fatal: CreateProcess: %1 is not a valid Win32 application

The launcher was reached, but ninja starts it with CreateProcess, which cannot
run a .sh: the shebang means nothing to Windows. CMAKE_CXX_COMPILER_LAUNCHER
takes a ;-separated list, so naming bash explicitly puts a real executable
first and the script becomes its argument.
2026-08-15 12:01:08 +02:00
jolavillette
569666321c ci(windows): rewrite -std=gnu++98 through a compiler launcher
The previous attempt passed CMAKE_CXX98_STANDARD_COMPILE_OPTION on the command
line. It had no effect: CMake sets that variable itself while detecting the
compiler, after the cache is read, so it overwrote what was given.

The flag comes from udp-discovery-cpp, which does

    set_property(TARGET udp-discovery PROPERTY CXX_STANDARD 98)

a target property, which nothing outside its own CMakeLists can override --
while its sources include headers that need C++11. The UCRT64 toolchain refuses
it; the older GCC on the Ubuntu runner does not, which is why this is a Windows
only step.

CMAKE_CXX_COMPILER_LAUNCHER prefixes every compile command, so a three line
script can rewrite that one flag and forward the rest untouched. Nothing else in
the build sees a difference, and CMakeLists.txt stays as it is. Tested locally
on the generated script.

The real fix belongs upstream: FetchContent asks for GIT_TAG "origin/master" of
that third party project, so this build follows whatever it does today.
2026-08-15 11:56:23 +02:00
jolavillette
3bbabde96b ci(windows): stop udp-discovery-cpp being compiled as C++98
c++0x_warning.h:32: error: This file requires compiler and library support
    for the ISO C++ 2011 standard

udp-discovery-cpp asks CMake for C++98, so the sub-build is compiled with
-std=gnu++98, and its own sources then include headers that need C++11. The
UCRT64 toolchain refuses; the older GCC on the Ubuntu runner lets it through,
which is why only Windows stops here.

Two reasons it does not happen in the super-project: it carries
udp-discovery-cpp as a submodule pinned at f3a3103, so the add_subdirectory
branch is taken -- and the FetchContent branch used by a standalone build asks
for GIT_TAG "origin/master", the moving tip of a third party project. That is
worth reporting on its own: this build follows whatever that repository does
today.

CMAKE_CXX98_*_COMPILE_OPTION is what CMake emits when a target asks for C++98.
Remapping it to gnu++17 leaves every other target untouched, and keeps this out
of CMakeLists.txt.
2026-08-15 11:49:17 +02:00
jolavillette
5bcde96d2a ci: install rapidjson from the system
Build stopped on every file that includes rsjson.h:

    rapidjson/document.h: No such file or directory

and the compile line carried no -I for it. CMakeLists.txt resolves rapidjson in
three steps: find_path in the system, then ../supportlibs/rapidjson, then
FetchContent. The third branch calls FetchContent_MakeAvailable(rapidjson) and
stops there -- it never adds an include directory for what it just downloaded,
so nothing reaches the compiler.

The super-project always takes the second branch, its submodule being present,
which is why this only shows up in a standalone build.

Installing the system package makes the first branch match, which is also what
the super-project's other CIs do: rapidjson-dev, mingw-w64-ucrt-x86_64-rapidjson,
rapidjson.
2026-08-15 11:42:52 +02:00
jolavillette
1b67abdf96 ci(macos): give the restbed patch step a GNU sed
The restbed FetchContent block patches the fetched CMakeLists with

    sed -i -e "s|/wd4251||g" ...

which is GNU syntax. BSD sed, the one macOS ships, reads what follows -i as a
backup suffix, so it takes "-e" as a filename and stops with

    sed: -e: No such file or directory

The super-project never runs into this: it carries restbed as a submodule, so
the add_subdirectory branch is taken and FetchContent -- with its patch step --
never runs. The standalone build has no submodule and always goes through
FetchContent, which is why this surfaces here and nowhere else.

Fixed by putting Homebrew's GNU sed first in PATH for the configure step, rather
than by touching CMakeLists.txt.
2026-08-15 11:41:41 +02:00
jolavillette
a09ea85b42 ci: install Botan, which rnp needs
First run: all three jobs stopped at configure with

    Could NOT find Botan (missing: BOTAN_LIBRARY BOTAN_INCLUDE_DIR)

Botan is not a dependency of libretroshare -- it appears in none of its
find_package calls, which is why it was missing from these lists -- but rnp
requires it, and rnp is pulled in by FetchContent as soon as RS_RNPLIB is on.
The super-project's CIs install it for the same reason.

libbotan-2-dev, mingw-w64-ucrt-x86_64-libbotan, botan@2.
2026-08-15 11:39:19 +02:00
jolavillette
d22307623b ci: same CMake build on macOS
Third and last platform, same standalone build and the same RS_* option set as
Ubuntu and Windows, so a failure on one of them points at portability rather
than at a difference in what was configured.

What differs is Homebrew: most of these formulae are keg-only, so their headers
and libraries are not on the default search paths. Same treatment as the
super-project's macOS job -- every opt/* include and lib directory harvested
into the compiler and linker flags, and the prefixes exposed to find_package()
through CMAKE_PREFIX_PATH.

None of the GUI formulae the super-project installs are here: no Qt, no ffmpeg,
no speex. This repository builds no interface, and libretroshare does not use Qt
at all -- which is also why there is no Qt5/Qt6 split in these three jobs.
2026-08-15 11:37:27 +02:00
jolavillette
c765615389 ci: same CMake build on Windows UCRT64
Linux alone would leave the platform where portability actually breaks
untested -- and it is the one the dead qmake workflow was aimed at.

Same standalone build as the Ubuntu job, same RS_* option set, so a failure on
one and not the other means a portability problem rather than a difference in
what was configured. What differs is what has to: pacman instead of apt, and the
two -include flags the super-project's Windows job also carries, since librnp
uses strlen and the fixed width integer types without including <cstring> and
<cstdint>, which this toolchain refuses.

No Qt packages here: this repository builds no GUI.

macOS is deliberately left out for now. Its counterpart in the super-project is
mostly Homebrew flag harvesting for Qt5, ffmpeg and speex -- GUI and VOIP
dependencies that libretroshare does not use -- so it would be a rewrite rather
than a transposition, with little to catch that Linux does not.
2026-08-15 11:25:43 +02:00
jolavillette
252070ed3a ci: build libretroshare with CMake, on its own
This repository has no CI that compiles anything. Its only build workflow is a
copy of the super-project's Windows qmake job, which cannot work here -- it
checks out submodule paths that exist only in the super-project -- and has
failed all of its last 40 runs, on master and on every pull request. The green
mark next to it comes from the GitLab mirror, not from a build.

CMake makes an actual CI possible for the first time: libretroshare has a top
level CMakeLists.txt of its own, project(retroshare), and pulls what it needs
through FetchContent -- rnp, OpenPGP-SDK, BitDHT, rapidjson, restbed,
udp-discovery-cpp. So it configures and builds with no super-project, no
submodules and no graft: a checkout, the system libraries, configure, build.
None of that is possible with qmake, where libretroshare/src/libretroshare.pro
is only a subproject of RetroShare.pro.

Options follow the Linux CMake job of the super-project, minus what does not
exist here (GUI, service, friendserver, plugins), and otherwise keep the values
a normal build uses so the CI compiles what people ship. RS_JSON_API is turned
on explicitly because it defaults to OFF here, and it is both the most
breakage-prone part of the tree and the reason restbed is fetched at all.

The broken qmake workflow is left untouched: replacing it is a separate
decision, and this one stands on its own.
2026-08-15 09:51:47 +02:00
csoler
ebbc30e368
Merge pull request #365 from defnax/api-for-getBoardPostSummaries
Added API for getBoardPostSummaries
2026-08-13 21:01:17 +02:00
csoler
dcd7ff4466
Merge pull request #357 from jolavillette/fix/gxs-grpmeta-batch-write
Batch group meta data updates in a single DB transaction
2026-08-11 20:18:47 +02:00
defnax
e679c45cbc Added API for get board post summaries 2026-08-11 18:04:37 +02:00
jolavillette
1a6de22c04 Batch group meta data updates in a single DB transaction
processGrpMetaChanges() wrote each queued group meta update with its own
call to updateGroupMetaData(), i.e. one implicit SQLCipher transaction and
one fsync per entry. One such write was measured at ~1 s, and services
accumulating many updates (typically identity usage stamps at startup,
~3700 identity groups) froze their tick thread for as long as 86 s while
draining the queue, blocking all other GXS activity of the service through
mGenMtx.

Mirror the existing message-side batching: add a vector variant of
updateGroupMetaData() that wraps all row updates in a single transaction
(one fsync total), and make processGrpMetaChanges() collect the entries
that pass their mask and write them in one call.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-10 09:20:54 +02:00
csoler
f49a0c44ca
Merge pull request #362 from jolavillette/fix/cmake-cc-in-implementation-headers
cmake: drop rsthreads.cc and rskbdinput.cc from RS_IMPLEMENTATION_HEADERS
2026-08-10 08:59:33 +02:00
csoler
876e276c6c
Merge pull request #363 from defnax/api-forowncustomstate
Enabled API for getOwnCustomStateString
2026-08-10 08:58:41 +02:00
csoler
6706016d80
Merge pull request #351 from jolavillette/perf/gxs-channel-loading
GXS: fix four causes of slow channel message loading
2026-08-10 08:57:23 +02:00
defnax
acffbd2ca8 Enabled API for getOwnCustomStateString 2026-08-09 14:09:46 +02:00
jolavillette
7ea31d7b12 cmake: drop rsthreads.cc and rskbdinput.cc from RS_IMPLEMENTATION_HEADERS
Both files were listed twice in src/CMakeLists.txt: once (correctly) in
RS_SOURCES, and once again in RS_IMPLEMENTATION_HEADERS right next to
their .h namesake -- a copy/paste slip dating back to the initial CMake
port (449fcbc3).

RS_IMPLEMENTATION_HEADERS does not feed add_library(), so this never
caused a duplicate compilation. It is only consumed by the install()
loop guarded by RS_LIBRETROSHARE_STANDALONE_INSTALL, which was therefore
copying the two .cc files into the installed include directory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-09 10:00:03 +02:00
jolavillette
2a2a1e241e Address review: identify posts by mThreadId alone
getChannelAllContent() split the metas into posts and comments/votes on
"mThreadId.isNull() && mParentId.isNull()". Only the first half of that test
carries information: a post never has a thread id, a comment always carries
the id of the post it belongs to, and a vote the id of the post whose comment
is being voted. mParentId proves nothing, since comments written under the old
comment paradigm have a null one.

Reported by csoler on PR #351.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-08 16:50:04 +02:00
jolavillette
494ce38f11 GXS: remove the profiling probes from the optimised code paths
Requested in review: the instrumentation added by the first commit served
to measure the four defects and verify the fixes, but it should not stay
in the optimised code. The probes are removed from rsdataservice,
rsgenexchange and p3gxschannels; convertMsgItems() loses the two timing
out-parameters that only existed to feed them.

The profiler class itself (gxs/rsgxsprofiler.h) is kept, per review, for
future measurement work. Nothing includes it anymore.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-07 01:17:16 +02:00
jolavillette
753481e6c5 GXS: stop the batched retrieval from rescanning the group once per batch
Profiling the version filtering added in the previous commit showed the
batched IN(...) retrieval costing a near constant 60 to 95ms per batch
whatever the number of messages it returned: 5 batches for 2203 messages took
454ms, 7 batches for 3127 took 451ms. That is the signature of each batch
walking the whole group, and it made a channel with almost no edited posts
slower than before (272ms in one query against 451ms in seven).

The cause is the query plan. On "grpId=... AND msgId IN (...)" sqlite has no
ANALYZE data, so it estimates an equality on the non unique group index at
about ten rows, against one row per entry of the IN list. The group therefore
looks fifty times more selective than it really is -- it matches every message
of the channel -- and gets picked, so every batch scans the group and filters.

Select on the message id alone. It is the table's primary key, hence unique
table wide, so the result is identical while sqlite can seek straight into the
implicit unique index. locked_retrieveMessages() takes an optional expected
group and drops anything else, so a caller mixing groups still cannot get
foreign messages attributed to the wrong one.

Reading most of a group by id remains slower than scanning it once, whatever
the index. So getChannelAllContent() now only filters versions out when they
are worth filtering: past MAX_READ_RATIO_FOR_VERSION_FILTERING of the group's
messages it requests the whole group and resolves versions with sortPosts() as
before. The profiling line reports which path was taken.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-07 01:17:16 +02:00
jolavillette
fa313718c0 GXS channels: read only the post versions that will be displayed
A channel keeps every version of every edited post. Only the latest of each
chain is ever shown: sortPosts() read them all, then discarded the superseded
ones keeping just their ids in mOlderVersions. Profiling a real channel shows
how much that costs: 6409 messages read for 1931 displayed posts, 195MB of
payload of which roughly two thirds belongs to versions thrown away
immediately -- thumbnails decrypted, deserialised and freed for nothing.

Resolve the version chains on the metas instead. They are small, come from
the meta cache once warm, and sortPostMetas() already works on any type
exposing a RsMsgMetaData. getChannelAllContent() now:

  - pulls the group's metas via getContentSummaries()
  - splits posts from comments and votes
  - runs sortPostMetas() to find the retained version of each chain
  - requests message data for those ids only, plus all comments and votes

Since the request now carries an explicit id set, it goes through the batched
IN(...) retrieval added earlier: a few queries instead of one, and the payload
read drops by whatever the edit history weighs.

Two behaviours of sortPosts() have to be reproduced, and applyPostVersions()
does so from the resolved chains:

  - comments hang off whichever version was current when they were written, so
    they are remapped onto the retained post before being counted, which
    replaces the old "add up the counts of all older versions" pass;
  - sortPostMetas() normalises mOrigMsgId to the top of the chain, and callers
    match edited posts on that value (GUI updateSinglePost), so the normalised
    id is carried over to the post that is returned.

The item conversion loop is factored out of getPostData() into
convertMsgItems() so both paths share it; getPostData() itself, still used by
getChannelContent() and the deprecated API, keeps calling sortPosts()
unchanged.

An empty id set means "every message of the group" to the data store, so an
empty channel returns before any request is made rather than asking for
everything.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-07 01:17:16 +02:00
jolavillette
af7ac7168d GXS: make message types movable so post arrays stop being deep copied
RsGxsImage owns a malloc'd buffer and declares a destructor, a copy
constructor and a copy assignment. RsMsgMetaData and RsGxsGenericMsgData
declare destructors too, and so does RsGxsChannelPost. Each of those user
declared destructors suppresses the implicit move operations, so every
std::move() on a post silently resolved to the copy constructor: a malloc
plus memcpy of the thumbnail.

That cost is paid far more often than it looks. A channel's post array is
copied whole at four hand-off points between the store, the service and the
model, none of which reserve, so vector growth copies on top. Worse,
std::sort falls back to copies as well, which for a few thousand posts means
tens of thousands of thumbnail duplications -- and that sort runs in the GUI
thread.

Give RsGxsImage real move operations that steal the buffer, and explicitly
default the copy and move operations of RsMsgMetaData, RsGxsGenericMsgData and
RsGxsChannelPost. The resulting RsGxsChannelPost move constructor is noexcept,
which is what std::vector requires to move rather than copy on reallocation.

Then use them where the arrays are handed over: reserve and move in
getPostData() and sortPosts() instead of copying element by element, and move
the sorted array back into the caller's vector.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-07 01:17:16 +02:00
jolavillette
4a5e8f79ac GXS: batch message retrieval into IN(...) queries instead of one per message
RsDataService::retrieveNxsMsgs() issued one prepared statement per requested
message id. Each one rebuilds the SQL text, runs a full sqlite3_prepare_v2
(SQL parse plus query planner), allocates a cursor and finalizes it -- an
overhead that dominates the actual row lookup, and is paid thousands of times
whenever a request covers a large id set.

Pack the ids into "msgId IN (...)" batches of 500 instead. The message ids are
plain hex strings so they need no escaping, and the batch size keeps both the
generated SQL and sqlite's expression tree small.

The previous commit removed this path for unfiltered whole-group requests;
this one covers everything else: a channel post with its comments, forum
threads, and any filtered request.

retrieveGxsMsgMetaData() still has the same one-query-per-id shape in its
non-empty branch, but there each id is first looked up in the meta cache, so
the remaining queries are only the cache misses. Left alone for now so it can
be measured on its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-07 01:17:16 +02:00
jolavillette
f9b90cb26b GXS: one SQL query per group when no message filter is requested
RsGxsDataAccess::getMsgData() always ran the request through getMsgIdList(),
which walks every message meta of the group and returns the explicit list of
matching message ids. That list was then handed to RsDataService, whose
retrieveNxsMsgs() has two paths: a single "WHERE grpId=..." query when the id
set is empty, and one prepared query per message otherwise.

Since a request for a whole group carries an empty id set precisely to mean
"all messages", expanding it into 6400 explicit ids meant the fast path was
never taken when opening a channel: the store issued 6400 separate
sqlite3_prepare_v2 + step + finalize cycles against an encrypted database,
plus a full preliminary pass over the metas that produced nothing the caller
did not already know.

When none of mStatusMask, mMsgFlagMask, MSG_LATEST, MSG_ORIGMSG or MSG_THREAD
is set, no filtering can occur, so pass the request through untouched. The
resulting message set is identical, the meta pass disappears, and loading a
whole group collapses to a single SQL query.

Requests that do filter are unaffected and keep the previous path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-07 01:17:16 +02:00
jolavillette
0442e59411 GXS: add opt-in profiling of the message loading path
Loading a channel with a few thousand posts takes ten seconds or more, and
the cost is spread over four layers with no way to tell which one dominates:
SQL retrieval in RsDataService, deserialisation in RsGenExchange, conversion
to service structures in p3GxsChannels, then the model update in the GUI.

Add a small header-only helper (gxs/rsgxsprofiler.h) and instrument those
layers so each reports its own breakdown on one line. Profiling stays off
unless the RS_GXS_PROFILE environment variable is set; its value is a
reporting threshold in milliseconds so only the operations worth looking at
show up (RS_GXS_PROFILE=0 reports everything).

The reported counters are the ones that matter for the known bottlenecks:
number of SQL queries issued and blob volume read in retrieveNxsMsgs, number
of metas walked in retrieveGxsMsgMetaData, mGenMtx wait and deserialisation
time in getMsgData, and the token wait in getChannelAllContent.

No behaviour change: when profiling is disabled the added work is a couple of
steady_clock reads per call and one comparison against a cached threshold.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-07 01:17:16 +02:00