Skip to content

Conversation

RossComputerGuy
Copy link
Member

Motivation

Adds a way to call the store API's computeFSClosure function from C.

Context

I'd like to be able to call this from the nixops4 Rust bindings.

Upstreams DeterminateSystems#209


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions bot added the c api Nix as a C library with a stable interface label Sep 19, 2025
@RossComputerGuy RossComputerGuy force-pushed the upstream-RossComputerGuy/feat/expose-computefsclosure branch 2 times, most recently from 3267618 to 9237e52 Compare September 19, 2025 17:57
@Ericson2314 Ericson2314 requested a review from roberth September 19, 2025 21:11
* @param[out] context Optional, stores error information
* @param[in] store nix store reference
* @param[in] store_path The path to compute from
* @param[in] flip_direction
Copy link
Member

Choose a reason for hiding this comment

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

@roberth is the right thing to just copy the doc from the C++ function?

Copy link
Member

Choose a reason for hiding this comment

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

If the docs are good, then that's fine

* @param[in] callback The function to call for every store path
* @param[in] userdata The userdata to pass to the callback
*/
nix_err nix_store_get_fs_closure(
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to put this above nix_store_realise

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@Ericson2314
Copy link
Member

I assigned to @roberth as he is defacto code owner of this.

@RossComputerGuy RossComputerGuy force-pushed the upstream-RossComputerGuy/feat/expose-computefsclosure branch from 9237e52 to d93cd0a Compare September 19, 2025 21:30
* @param[in] flip_direction
* @param[in] include_outputs
* @param[in] include_derivers
* @param[in] callback The function to call for every store path
Copy link
Member

Choose a reason for hiding this comment

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

Let's afford the impl some more freedom

Suggested change
* @param[in] callback The function to call for every store path
* @param[in] callback The function to call for every store path, in no particular order

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@RossComputerGuy RossComputerGuy force-pushed the upstream-RossComputerGuy/feat/expose-computefsclosure branch from d93cd0a to 1f8a5cc Compare September 19, 2025 21:47
@Ericson2314
Copy link
Member

Oh and actually I would like to not expose flip direction, as many stores don't support the reverse direction.

(I had an old blocked store API PR fo this, it would be nice to not repeat the same IMO mistake in the C API.)

@RossComputerGuy
Copy link
Member Author

Oh and actually I would like to not expose flip direction, as many stores don't support the reverse direction.

Should the C++ API reflect that? Imo, I prefer having feature parity to the C++ API as close as C can get.

@roberth
Copy link
Member

roberth commented Sep 19, 2025

Is it feasible to create a small store path graph using the store operations at this point?
Ideally we could test this without getting the evaluator involved, but otherwise, we could have a test in libexpr-tests instead, where we can do instantiations and realisations, no problem.

@RossComputerGuy
Copy link
Member Author

Is it feasible to create a small store path graph using the store operations at this point?

I think we're close to making that possible, I'm not sure what else is needed but I think it may be possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c api Nix as a C library with a stable interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants