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

Experimental python bindings #7735

Draft
wants to merge 42 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
2e37594
python: Initialise bindings from Pythonix
infinisil Feb 1, 2023
ed8c80b
python: Fixes for Nix changes
infinisil Feb 17, 2023
5dc8bcb
python: Integrate incremental and CI build
infinisil Feb 17, 2023
de148e4
python: Fix for IFD
infinisil Feb 17, 2023
827f634
python: Format using clang-format
infinisil Feb 17, 2023
410393f
python: Rename pythonnix namespace to nix::python
infinisil Feb 17, 2023
8837e42
python: Add exampleEnv to try out the bindings
infinisil Feb 17, 2023
4599be3
python: Install the bindings in hopefully the correct location
infinisil Feb 17, 2023
be36946
WIP: Start writing documentation and example
infinisil Feb 17, 2023
e3f56e9
Remove .cache from .gitignore
infinisil Feb 24, 2023
2c3e1c8
Always include config.h
infinisil Feb 24, 2023
9b27833
Release the GIL for Nix evaluation
infinisil Feb 24, 2023
d272171
Don't release GIL lock because it would segfault
infinisil Feb 24, 2023
b8003d6
Allow getting exact throw error messages
infinisil Feb 24, 2023
a1669ed
Readd .cache to .gitignore
infinisil Feb 24, 2023
33ca7e3
Add ThrownNixError subclass
infinisil Feb 27, 2023
7dee1c5
python: Add clang-tools to dev env
infinisil Mar 3, 2023
5bc4948
python: Properly release and reacquire GIL
infinisil Mar 3, 2023
6f8108c
python: Detect stack overflow from recursive data structure
infinisil Mar 3, 2023
446db64
Insert some TODO's
infinisil Mar 3, 2023
481f28c
python: Handle null's in expressions correctly
infinisil Mar 3, 2023
668313f
python: Fix boolean to nix conversion
infinisil Mar 3, 2023
96621a0
python: Add test for Null conversion
infinisil Mar 3, 2023
0503a73
python: Use assertRaises for tests
infinisil Mar 3, 2023
1b0960f
python: We don't need to install the bindings into a subdirectly
infinisil Mar 9, 2023
d8ce01b
Remove python from manual build again
infinisil Mar 9, 2023
9627862
Very simple Nix source filtering
infinisil Mar 9, 2023
a91f312
Fix the buildPythonApplication example
infinisil Mar 9, 2023
06d7135
Fix some unset variable problems with vars-and-functions.sh
infinisil Mar 9, 2023
34c8519
assertEquals -> assertEqual
infinisil Mar 9, 2023
91b0740
Alternate approach to calling init.sh
infinisil Mar 9, 2023
56ef3e3
Don't depend on all files for the python bindings
infinisil Mar 9, 2023
8d734cd
Make buildPythonApplication test work
infinisil Mar 9, 2023
f5fd435
Fix dev shell
infinisil Mar 10, 2023
68996d8
Add debugging capability to dev shell
infinisil Mar 10, 2023
22c9e48
Convert from eval to callExprString API
infinisil Mar 10, 2023
fb5884e
python api: write API docs for callExprString
yorickvP Apr 14, 2023
f1442f8
PyInit_Nix: elaborate on build hook workaround
yorickvP Apr 14, 2023
0d52ddc
buildPythonApplication: fix tests
yorickvP Apr 14, 2023
51440a3
python-bindings: use prev.callPackage to pass args
yorickvP Apr 14, 2023
2cb9dd0
python-to-nix: don't forward declare PyObject, doesn't work everywhere
yorickvP Apr 14, 2023
57e71a3
python: initNix -> initLibStore
yorickvP Apr 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Convert from eval to callExprString API
  • Loading branch information
infinisil authored and yorickvP committed Apr 19, 2023
commit 22c9e48724f4be2d9b473984e191834aa9fc8cf1
2 changes: 1 addition & 1 deletion python/doc/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ $ nix run github:NixOS/nix#nix.python-bindings.exampleEnv
Python 3.10.8 (main, Oct 11 2022, 11:35:05) [GCC 11.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import nix
>>> nix.eval('"Hello ${name}!"', vars=dict(name="Python"))
>>> nix.callExprString('"Hello ${name}!"', arg={"name": "Python"}))
'Hello Python!'
```

Expand Down
2 changes: 1 addition & 1 deletion python/examples/buildPythonApplication/hello/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import nix

def greet():
print("Evaluating 1 + 1 in Nix gives: " + str(nix.eval("1 + 1")))
print("Evaluating 1 + 1 in Nix gives: " + str(nix.callExprString("_: 1 + 1", None)))
24 changes: 13 additions & 11 deletions python/src/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,16 @@ const char * currentExceptionTypeName()
return res ? res : "(null)";
}

static PyObject * _eval(const std::string & expression, PyObject * vars)
static PyObject * _eval(const std::string & expression, PyObject * argument)
{
nix::Strings storePath;
nix::EvalState state(storePath, nix::openStore());
Copy link
Member

Choose a reason for hiding this comment

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

Creating an EvalState for each call is wasteful and somewhat risky, because that's a less frequently used code path. While I would like to push the envelope on that, it's perhaps not the right to do that.
See tweag#17 for a global EvalState solution.


nix::Env * env = nullptr;
auto staticEnv = pythonToNixEnv(state, vars, &env);
if (!staticEnv) {
auto nixArgument = pythonToNixValue(state, argument);
if (!nixArgument) {
return nullptr;
}
auto staticEnvPointer = std::make_shared<nix::StaticEnv>(*staticEnv);
nix::Value fun;
nix::Value v;


Expand All @@ -44,25 +43,28 @@ static PyObject * _eval(const std::string & expression, PyObject * vars)
});

// TODO: Should the "." be something else here?
auto e = state.parseExprFromString(expression, ".", staticEnvPointer);
e->eval(state, *env, v);
auto e = state.parseExprFromString(expression, ".");
state.eval(e, fun);
// TODO: Add position
state.callFunction(fun, *nixArgument, v, noPos);
state.forceValueDeep(v);
}

nix::PathSet context;
return nixToPythonObject(state, v, context);
}

// TODO: Rename this function to callExprString, matching the Python name
PyObject * eval(PyObject * self, PyObject * args, PyObject * keywds)
{
PyObject * expressionObject;
PyObject * vars = nullptr;
PyObject * argument = nullptr;

const char * kwlist[] = {"expression", "vars", nullptr};
const char * kwlist[] = {"expression", "arg", nullptr};

// See https://docs.python.org/3/c-api/arg.html for the magic string
if (!PyArg_ParseTupleAndKeywords(
args, keywds, "U|O!", const_cast<char **>(kwlist), &expressionObject, &PyDict_Type, &vars)) {
args, keywds, "UO", const_cast<char **>(kwlist), &expressionObject, &argument)) {
return nullptr;
}

Expand All @@ -75,7 +77,7 @@ PyObject * eval(PyObject * self, PyObject * args, PyObject * keywds)
std::string expression(expressionBase, expressionSize);

try {
return _eval(expression, vars);
return _eval(expression, argument);
} catch (nix::ThrownError & e) {
return PyErr_Format(ThrownNixError, "%s", e.message().c_str());
} catch (nix::Error & e) {
Expand Down
1 change: 0 additions & 1 deletion python/src/internal/python-to-nix.hh
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,4 @@ namespace nix::python {

nix::Value * pythonToNixValue(nix::EvalState & state, PyObject * obj);

std::optional<nix::StaticEnv> pythonToNixEnv(nix::EvalState & state, PyObject * vars, nix::Env ** env);
} // namespace nix::python
2 changes: 1 addition & 1 deletion python/src/python-module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ PyObject * ThrownNixError = nullptr;
PyObject * NixError = nullptr;

static PyMethodDef NixMethods[] = {
{"eval", (PyCFunction) eval, METH_VARARGS | METH_KEYWORDS, "Eval nix expression"}, {NULL, NULL, 0, NULL}};
{"callExprString", (PyCFunction) eval, METH_VARARGS | METH_KEYWORDS, "Eval nix expression"}, {NULL, NULL, 0, NULL}};

static struct PyModuleDef nixmodule = {
PyModuleDef_HEAD_INIT, "nix", "Nix expression bindings",
Expand Down
32 changes: 0 additions & 32 deletions python/src/python-to-nix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,36 +130,4 @@ nix::Value * pythonToNixValue(nix::EvalState & state, PyObject * obj)
}
return v;
}

std::optional<nix::StaticEnv> pythonToNixEnv(nix::EvalState & state, PyObject * vars, nix::Env ** env)
{
Py_ssize_t pos = 0;
PyObject *key = nullptr, *val = nullptr;

*env = &state.allocEnv(vars ? PyDict_Size(vars) : 0);
(*env)->up = &state.baseEnv;

nix::StaticEnv staticEnv(false, state.staticBaseEnv.get());

if (!vars) {
return staticEnv;
}

auto displ = 0;
while (PyDict_Next(vars, &pos, &key, &val)) {
auto name = checkAttrKey(key);
if (!name) {
return {};
}

auto attrVal = pythonToNixValue(state, val);
if (!attrVal) {
return {};
}
staticEnv.vars.emplace_back(state.symbols.create(name), displ);
(*env)->values[displ++] = attrVal;
}

return staticEnv;
}
} // namespace nix::python
29 changes: 15 additions & 14 deletions python/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,64 +5,65 @@
class TestPythonNix(unittest.TestCase):
def test_dict(self):
val = dict(a=1)
self.assertEqual(nix.eval("a", vars=dict(a=val)), val)
self.assertEqual(nix.callExprString("{ a }: a", arg=dict(a=val)), val)
Copy link
Member Author

Choose a reason for hiding this comment

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

As previously hinted, in the latest commit I now changed the API to a more explicit function call like

nix.eval("{ a }: a", arg=dict(a=1))

from the original

nix.eval("a", vars=dict(a=1))

because that worked the same as builtins.scopedImport, which is known as a wart in the Nix language because it doesn't do file caching as import does.

However, after noticing that it's now very odd when you don't need to take any arguments, like this:

nix.callExprString("{}: null", arg={})

I decided to reinvestigate the original idea, and whether builtins.scopedImport is really as bad as we make it out to be, I wrote this down here: #8024

Looking into this I'm concluding that the three issues listed there shouldn't be a problem for the Python bindings, see the above issue for more context:

  • Confusing free variables: Compared to scopedImport, the original API allows having the expression string right next to the variables it uses, making it very similar to a function declaration.
  • Doesn't allow file evaluation to be cached: The original API can be seen as implicitly prepending the expression with function arguments, and anything under a function argument in a file can't be cached anyways.
  • Combined with the ability to shadow builtins: Indeed a problem, but it's neither up to scopedImport nor the Python bindings to fix.

Because of that I'm considering going back to the original API

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 not overthink this one. We can replace this function later and focus on other aspects such as a stable intermediate C/C++ interface, validating the packaging (including in Nixpkgs).

Doesn't allow file evaluation to be cached

You could return a Value to cache it from Python. I'll shut up now.

Copy link
Member

Choose a reason for hiding this comment

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

I've decided to spend a small amount of time to explore how a custom type can be added, and it doesn't seem so bad.
This sets up a nix.Value type

It's a bit MVP, but it establishes the pattern.
It has also given me a better insight into how the bindings work.


def test_string(self):
self.assertEqual(nix.eval("a", vars=dict(a="foo")), "foo")
self.assertEqual(nix.callExprString("{ a }: a", arg=dict(a="foo")), "foo")

def test_bool(self):
self.assertEqual(nix.eval("a", vars=dict(a=True)), True)
self.assertEqual(nix.callExprString("{ a }: a", arg=dict(a=True)), True)

def test_none(self):
self.assertEqual(nix.eval("a", vars=dict(a=None)), None)
self.assertEqual(nix.callExprString("{ a }: a", arg=dict(a=None)), None)

def test_ifd(self):
expression = """
{}:
builtins.readFile (derivation {
name = "test";
args = [ "-c" "printf \\"%s\\" test > $out" ];
builder = "/bin/sh";
system = builtins.currentSystem;
})
"""
self.assertEqual(nix.eval(expression, vars=dict()), "test")
self.assertEqual(nix.callExprString(expression, arg={}), "test")

def test_throw(self):
errorString = "hello hi there\ntest"
with self.assertRaises(nix.ThrownNixError) as cm:
nix.eval("throw str", vars=dict(str=errorString))
nix.callExprString("{ str }: throw str", arg=dict(str=errorString))
self.assertEqual(cm.exception.args[0], errorString)

def test_syntax_error(self):
with self.assertRaises(nix.NixError) as cm:
nix.eval("{")
nix.callExprString("{", arg={})

def test_GIL_case(self):
with self.assertRaises(nix.ThrownNixError) as cm:
nix.eval("{ a = throw \"nope\"; }")
nix.callExprString("{}: { a = throw \"nope\"; }", arg={})
self.assertEqual(cm.exception.args[0], "nope")

def test_infinity(self):
with self.assertRaises(nix.NixError):
nix.eval("let x = { inherit x; }; in x")
nix.callExprString("{}: let x = { inherit x; }; in x", arg={})

def test_null_expression(self):
# Null characters should be allowed in expressions, even if they aren't
# very useful really, though at least null's should be supported in
# strings in the future https://github.com/NixOS/nix/issues/1307)
self.assertEqual(nix.eval("\"ab\x00cd\""), "ab")
self.assertEqual(nix.callExprString("{}: \"ab\x00cd\"", arg={}), "ab")

def test_throw_null(self):
with self.assertRaises(nix.ThrownNixError) as cm:
nix.eval("throw \"hello\x00there\"")
nix.callExprString("{}: throw \"hello\x00there\"", arg={})
self.assertEqual(cm.exception.args[0], "hello")

def test_booleans(self):
self.assertIs(nix.eval("assert a == true; a", vars=dict(a=True)), True)
self.assertIs(nix.eval("assert a == false; a", vars=dict(a=False)), False)
self.assertIs(nix.callExprString("{ a }: assert a == true; a", arg=dict(a=True)), True)
self.assertIs(nix.callExprString("{ a }: assert a == false; a", arg=dict(a=False)), False)

def test_null(self):
self.assertIs(nix.eval("assert a == null; a", vars=dict(a=None)), None)
self.assertIs(nix.callExprString("{ a }: assert a == null; a", arg=dict(a=None)), None)

if __name__ == '__main__':
unittest.main()