C API: Add copy_path to Store API#14768
Conversation
|
Sounds good, but needs release notes. |
pkpbynum
force-pushed
the
capi/copy-path
branch
from
540637e to
418f31f
Compare
pkpbynum
force-pushed
the
capi/copy-path
branch
from
418f31f to
50ba340
Compare
done |
pkpbynum
force-pushed
the
capi/copy-path
branch
from
50ba340 to
72ab64b
Compare
| context->last_err_code = NIX_OK; | ||
| try { | ||
| if (srcStore == nullptr) | ||
| return nix_set_err_msg(context, NIX_ERR_UNKNOWN, "Source store is null"); |
There was a problem hiding this comment.
I wonder if we should add a new error type like NIX_ERR_INVALID_ARGUMENT for such cases.
There was a problem hiding this comment.
I'm just following the existing usage here: https://github.com/NixOS/nix/blob/master/src/libmain-c/nix_api_main.cc#L26
Could we save that sort of change for follow-up? It seems like it would/should involve a wider cleanup of error handling in C API:
- Improve error types/messages
- check inputs for nullptr
There was a problem hiding this comment.
Frankly I wouldn't mind if this was just an assert, or not here at all, since both arguments being non-null are morally part of the signature.
There was a problem hiding this comment.
We can address this in a follow-up I suppose. The current state of things where stuff just segfaults isn't ideal and certainly not the long-term goal. Adding an error code does pose a backwards-compatibility issue and the current set of error codes isn't particularly extensible and the error codes aren't yet documented.
There was a problem hiding this comment.
Are there any additional changes I should make, or is this good to merge as-is?
There was a problem hiding this comment.
I think @roberth has made a comment about having some tests for this, but maybe this can be done in a follow-up.
4 participants
Motivation
This provides more granular control over paths as they are copied in a few ways:
My primary motivation is to be able to monitor copy progress on a per-path basis. AFAIK the alternative way to do this is to log & parse the JSON format--not possible from FFI (?). Secondarily, I'd like to repair individual paths without copying the entire closure (otherwise I'd just add the
repairparam to the existingcopyClosure).Context
Tested on my fork calling from Rust.
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.