Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python bindings: add Value type #17

Open
wants to merge 39 commits into
base: python-bindings
Choose a base branch
from

Conversation

roberth
Copy link

@roberth roberth commented Mar 11, 2023

@infinisil I hope this helps. A custom type doesn't seem so bad!

infinisil and others added 30 commits February 24, 2023 13:24
And fix a test failure in the sandbox due to /home
existing on Darwin but not being accessible in the sandbox since it's a
symlink to /System/Volumes/Data/home, see
https://github.com/NixOS/nix/actions/runs/4205378453/jobs/7297384658#step:6:2127:

    C++ exception with description "error: getting status of /home/schnitzel/darmstadt/pommes: Operation not permitted" thrown in the test body.

On Linux this wasn't a problem because there /home doesn't exist in the sandbox
Copies part of the tree from Pythonix at
Mic92/pythonix@fbc8490
into the ./python subdirectory.

These Python bindings don't build yet with the current Nix version,
which is why they're not built yet in this commit.

Mic92 confirmed that he's okay with the license being changed to the Nix
one: NixOS#7735 (comment)

Co-Authored-By: Jörg Thalheim <joerg@thalheim.io>
The python bindings initialised from Pythonix haven't been updated in
some Nix versions and would not compile anymore. This commit still
doesn't include a working python bindings build, but these are the
Nix-update-caused changes that will be necessary for that
Makes the python bindings build, both incrementally and for CI.
Documentation is not yet included
Using the configuration file from NixOS#6721 for less conflicts
Will be useful for documentation
And don't use the deprecated python3 meson module
Not sure why I added it originally
Mirroring what the main Nix build does
And test that null bytes currently end strings
It didn't cause a problem because it converted True to 1, which still
allowed 1 == True to succeed in Python (using assertEqual)
Using a symlink hacky and breaks in some ways
So that we don't have to rebuild Nix every time the python bindings
change
- BASH_SOURCE[0] doesn't exist for evaluations under `eval`
- NIX_STORE may not be set
infinisil and others added 9 commits March 10, 2023 00:09
The former is deprecated
Previously the python bindings derivation would use Nix as its source,
call configure on that, just to extract the two files tests/init.sh and
(the generated one) tests/common/vars-and-functions.sh

The new approach is to instead have a separate derivation extracting
these two files and having a small wrapper around them
Tests that the Nix bindings can be used as a Python dependency in Nix
builds
Creating a new one for each call is wasteful and doesn't scale
to a composable interface, as the same EvalState must be used for
operations on the same Values.

Ideally we'd store EvalState in the python module, or make it a
first class object.
@yorickvP
Copy link

Did you see https://gist.github.com/yorickvP/08afdaae98cad3516a9629e7e8df343d ? I think we can almost implement it with this and a python wrapper. Just need a way to

  • get type of a value (including unevaluated types?)
  • toPythonLazy
  • get string context

@roberth
Copy link
Author

roberth commented Mar 14, 2023

I did and I was inspired by it! Would you like to resume this?
I felt that I had to make time to make sure this direction of development isn't avoided for the wrong reasons, but I can't spend more time on this for the foreseeable future.

  • get type of a value

Would be nice but not essential.

  • get type of a value including unevaluated types?

We need to perform an undecidable amount of evaluation to return that information, and that'd be a feature to implement in the evaluator itself. I don't think it's worth the effort. Instead, you could formalize the interface of the expression with a schema expressed in python. It makes the runtime type checking at the boundary easy and consistent. Haskell hercules-ci-cnix-expr already implements this idea.

  • toPythonLazy

Also nice to have. A combination of v.${k} and toPythonStrict goes a long way!

  • get string context

Can be achieved by wrapping builtins.getContext. Can be re-done later if the error messages aren't great.

I think this PR is already a valid MVP.


## Build integration

In the future these Python bindings will be available from Nixpkgs as `python3Packages.nix`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In the future these Python bindings will be available from Nixpkgs as `python3Packages.nix`.

Let's not talk about the future here.


See [index.md](./doc/index.md), which is also rendered in the HTML manual.

To hack on these bindings, see [hacking.md](./doc/hacking.md), also rendered in the HTML manual.
Copy link
Member

Choose a reason for hiding this comment

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

This seems empty though. Let's not reference things that don't exist, or create those things first.


## Documentation

See [index.md](./doc/index.md), which is also rendered in the HTML manual.
Copy link
Member

Choose a reason for hiding this comment

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

I may be missing something but I don't see where it's rendered in the manual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants