util+doc: Clearer errors when attempting to run examples + polished docs

hodlinator commented at 12:13 pm on September 18, 2025: contributor

Found while reviewing #201.

DrahtBot commented at 12:13 pm on September 18, 2025: none

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky, Eunovo, theuni

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

hodlinator force-pushed on Sep 18, 2025

hodlinator commented at 12:55 pm on September 18, 2025: contributor

Let me know if you see any merit to the Drahtbot recommendation (https://github.com/bitcoin-core/libmultiprocess/pull/213#issuecomment-3307125359):

Its return values -> Return values [“Its” incorrectly refers to the preceding expression; use “Return values” to make the sentence grammatical and clear]

in example/example.cpp:34 in 10d31a6711 outdated

29@@ -30,6 +30,8 @@ static auto Spawn(mp::EventLoop& loop, const std::string& process_argv0, const s
30         fs::path path = process_argv0;
31         path.remove_filename();
32         path.append(new_exe_name);
33+        if (!fs::exists(path))
34+            throw std::runtime_error{std::string{path} + " is missing, probably needs to be built?"};

ryanofsky commented at 1:54 pm on September 18, 2025:

In commit “example: Hint at possible problem when binaries are missing” (10d31a6711821cf7d1a68326b113ca20b557610c)

This is an improvement, but if this could be updated to just handle errors from the Spawn() calls below and suggest binaries may be missing then, I think that would be better. Better generally to handle errors than try to predict them IMO. And in theory this change could also trigger errors unnecessarily if if binaries were not in the PWD but were in the PATH.

Also would be good to follow bitcoin style and either use a single-line if or add braces around multiline if.

hodlinator commented at 9:35 pm on September 18, 2025:

Oof, I misremembered your suggestion and ended up moving the error to inside ExecProcess() in util.cpp instead. Let me know if you prefer something closer to your suggestion, no problem.

ryanofsky commented at 4:34 pm on September 22, 2025:

re: #213 (review)

Oof, I misremembered your suggestion and ended up moving the error to inside ExecProcess() in util.cpp instead. Let me know if you prefer something closer to your suggestion, no problem.

This is pretty good. I think your approach makes sense and is a clear improvement. I made my suggestion forgetting that the exec error is only available in the child process, and it would take extra effort to send the errno back to the parent process to use in an exception. Apparently there is a pattern for doing this using a temporary pipe and cloexec but it’s more complicated and probably would be better to do in a followup.

in doc/usage.md:9 in 02d2702a69 outdated

 3@@ -4,9 +4,9 @@
 4 
 5 _libmultiprocess_ is a library and code generator that allows calling C++ class interfaces across different processes. For an interface to be available from other processes, it needs two definitions:
 6 
 7-- An **API definition** declaring how the interface is called. Included examples: [calculator.h](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/calculator.h), [printer.h](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/printer.h), [init.h](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/init.h). Bitcoin examples: [node.h](https://github.com/ryanofsky/bitcoin/blob/ipc-export/src/interfaces/node.h), [wallet.h](https://github.com/ryanofsky/bitcoin/blob/ipc-export/src/interfaces/wallet.h), [echo.h](https://github.com/ryanofsky/bitcoin/blob/ipc-export/src/interfaces/echo.h), [init.h](https://github.com/ryanofsky/bitcoin/blob/ipc-export/src/interfaces/init.h).
 8+- An **API definition** declaring how the interface is called. Included examples: [calculator.h](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/calculator.h), [printer.h](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/printer.h), [init.h](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/init.h). Bitcoin examples: [node.h](https://github.com/bitcoin/bitcoin/blob/master/src/interfaces/node.h), [wallet.h](https://github.com/bitcoin/bitcoin/blob/master/src/interfaces/wallet.h), [echo.h](https://github.com/bitcoin/bitcoin/blob/master/src/interfaces/echo.h), [init.h](https://github.com/bitcoin/bitcoin/blob/master/src/interfaces/init.h).
 9 
10-- A **data definition** declaring how interface calls get sent across the wire. Included examples: [calculator.capnp](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/calculator.capnp), [printer.capnp](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/printer.capnp), [init.capnp](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/init.capnp). Bitcoin examples: [node.capnp](https://github.com/ryanofsky/bitcoin/blob/ipc-export/src/ipc/capnp/node.capnp), [wallet.capnp](https://github.com/ryanofsky/bitcoin/blob/ipc-export/src/ipc/capnp/wallet.capnp), [echo.capnp](https://github.com/ryanofsky/bitcoin/blob/ipc-export/src/ipc/capnp/echo.capnp), [init.capnp](https://github.com/ryanofsky/bitcoin/blob/ipc-export/src/ipc/capnp/init.capnp).
11+- A **data definition** declaring how interface calls get sent across the wire. Included examples: [calculator.capnp](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/calculator.capnp), [printer.capnp](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/printer.capnp), [init.capnp](https://github.com/bitcoin-core/libmultiprocess/blob/master/example/init.capnp). Bitcoin examples: [node.capnp](https://github.com/bitcoin/bitcoin/blob/master/src/ipc/capnp/node.capnp), [wallet.capnp](https://github.com/bitcoin/bitcoin/blob/master/src/ipc/capnp/wallet.capnp), [echo.capnp](https://github.com/bitcoin/bitcoin/blob/master/src/ipc/capnp/echo.capnp), [init.capnp](https://github.com/bitcoin/bitcoin/blob/master/src/ipc/capnp/init.capnp).

ryanofsky commented at 1:58 pm on September 18, 2025:

In commit “doc: Replace references to ryanofsky/bitcoin/ with bitcoin/bitcoin/” (02d2702a696735d8dfd2890f69ac5d4759ef42c6)

Some of these new links are broken because node and wallet capnp files are only in the IPC branch and https://github.com/bitcoin/bitcoin/pull/10102, not in master. Could keep the existing links which are not broken, or link to an interface like mining.capnp which is in master.

hodlinator commented at 9:36 pm on September 18, 2025:

Fixed broken links, also used relative paths for the libmultiprocess repo as was already done further down in the file.

ryanofsky approved

ryanofsky commented at 2:02 pm on September 18, 2025: collaborator

Code review ACK 91606b232b4f62efcad4795f7b48cbfea60e2276. Thanks for the PR! Left some suggestions below, but overall I think this is an improvement.

Let me know if you see any merit to the Drahtbot recommendation (#213 (comment))

Not too important, I think it’d make sense to replace “Its return values” with “The return values.” Currently it seems like “its” refers to capnp::RpcSystem which doesn’t really make sense.

hodlinator force-pushed on Sep 18, 2025

hodlinator renamed this:
~~example+doc: Clearer errors when attempting to run examples + polished docs~~
util+doc: Clearer errors when attempting to run examples + polished docs
on Sep 18, 2025

hodlinator force-pushed on Sep 18, 2025

hodlinator marked this as a draft on Sep 19, 2025

hodlinator force-pushed on Sep 20, 2025

hodlinator marked this as ready for review on Sep 20, 2025

hodlinator force-pushed on Sep 20, 2025

util: Add helpful error message when failing to execute file

Useful when forgetting to build necessary targets.

286fe469c9

doc: Where possible, remove links to ryanofsky/bitcoin/ 2b43481935

doc: Precision and typos

* Spell out "class template specialization"
* Uppercase c++
* Typo: require -writing- adding maps

ec03a9639a

hodlinator force-pushed on Sep 20, 2025

ryanofsky approved

ryanofsky commented at 4:37 pm on September 22, 2025: collaborator

Code review ACK ec03a9639ab55de85df4b688c08b1d7faec01c67 just addressing previous comments since last review: changing the missing executable check and updating markdown docs. All changes look very good now