minijinja-go: MiniJinja bindings for Go

[maxbrunet]

Hello there! 👋

Another awesome library as always ❤️

I would be interested in having Go bindings for MiniJinja, the main use case would be AI Chat templates at the moment.
I started the work here: maxbrunet/minijinja-go.
And the API reference can be found here: pkg.go.dev/github.com/maxbrunet/minijinja-go/v2

There are a few options to choose from to integrate a Rust library with Go:

• C bindings with cgo (current implementation)
• WASM with wazero
• C bindings with purego

Support for multiple implementations could be considered with a common interface and "drivers" similar to the database/sql package build tags (similar to github.com/wasilibs/go-aho-corasick) for users to choose how to integrate MiniJinja with their application and the related trade-offs if there is no clearly superior implementation in term of performance/maintainability/functionality/ease-of-use...

Also not sure if you would be open to having it living in this repo (after a few initial iterations probably), it would (eventually) make it easy to release, it would only require a minijina-go/v$version Git tag (note: the leading v is required). (reference)

I am going to add questions and comments below, so they can easily be answered individually at your convenience. 🙂

[maxbrunet]

There is no way to instantiate values of kinds Bytes and Plain with minijinja-cabi, are these kinds internal only? Or is it only possible for an expression to return them? If yes, would you have examples of such expressions, please?

minijinja/minijinja-cabi/include/minijinja.h

Line 73 in d05b61b

MJ_VALUE_KIND_BYTES,

minijinja/minijinja-cabi/include/minijinja.h

Line 77 in d05b61b

MJ_VALUE_KIND_PLAIN,

[mitsuhiko]

There is probably no bytes constructor, something that definitely needs fixing. Plain objects you will come across for instance if you evaluate debug as an expression which will return the debug function, which is a plain object.

[maxbrunet]

Thank you for all the replies, that helps a ton. I have been able to test Plain with debug. And here is my humble attempt at a bytes constructor: #749

[maxbrunet]

Would it be possible to add the staticlib crate type to minijinja-cabi to allow building staticly linked binaries?

minijinja/minijinja-cabi/Cargo.toml

Line 7 in 607d2de

crate-type = ["cdylib"]

[mitsuhiko]

I don't think staticlib is portable. Is that actually the case? In general I'm not sure how static libs would work. They normally cause all kinds of linkage errors. In particular there is no way to link the stdlib against a staticlib thing, so you would also need a static version of the stdlib for this to work I believe.

[maxbrunet]

Concurency with minijinja-cabi currently seems impossible with the global error state (unless maybe ignoring errors):

minijinja/minijinja-cabi/src/error.rs

Lines 8 to 10 in 607d2de

	thread_local! {
	pub static LAST_ERROR: RefCell<Option<Error>> = const { RefCell::new(None) };
	}

That would an important limitation of any implementation based on it at the moment

[maxbrunet]

When doing any operation, I check if the error is set immediately after, if it is, return it and clear it, for example:

https://github.com/maxbrunet/minijinja-go/blob/d3d80af6d1cf411ba02998af5aa81afe593555ad/env.go#L189-L191
https://github.com/maxbrunet/minijinja-go/blob/d3d80af6d1cf411ba02998af5aa81afe593555ad/errors.go#L105-L113

If doing multiple operation concurrently (e.g. a web service handling requests), I can't think of a simple way to identify what operation is the source of the error.

In my current implementation, if using concurrent goroutines, a goroutine could trigger an error and another effectuating a perfectly sane operation could receive that error.

Note: If unfamiliar with Go, goroutines can be started, paused, and continued on a same OS thread.

[mitsuhiko]

I don't think it's possible for go to suspend between these calls. As far as I know Go will only yield on syscalls, blocking operations, runtime.Gosched() and long running loops. So this should be fine. You won't be interrupted there.

[maxbrunet]

I have been able to trigger the data race in my tests by enabling them to run in parallel with T.Parallel() (running go test -race -count=100 definitely triggers it quickly)
In general, I doubt it is reliable to rely on the internal behaviors of the Go scheduler.

[mitsuhiko]

@maxbrunet then you would need to compile in a utility function from C that captures the error before. The go scheduler would not be permitted to suspend a native C call. You can also probably use LockOSThread.

[maxbrunet]

TIL ❤️ I did not know LockOSThread was a thing, it seems to be its purpose:

A goroutine should call LockOSThread before calling OS services or non-Go library functions that depend on per-thread state.

I have surround all calls to MiniJinja with lock/unlock OSThread and now the tests can run in parallel without any issue (maxbrunet/minijinja-go@8c5e76b)

[maxbrunet]

Does enabling debugging with minijinja-cabi as any effect at the moment? I could not notice any difference in the error details

minijinja/minijinja-cabi/include/minijinja.h

Line 169 in fd06e65

MINIJINJA_API void mj_env_set_debug(struct mj_env *env, bool val);

Would it possible to return the error details as a string, mostly the source part could be useful, so it can be accessible through the regular error handling flow?

minijinja/minijinja-cabi/include/minijinja.h

Line 238 in fd06e65

MINIJINJA_API bool mj_err_print(void);

[mitsuhiko]

Happy to accept PRs to expose more information from the errors. The debug flag should work. It will give you the source code output in an error message, but it's possible that this printing does not work well enough through the C binding (yet).

[maxbrunet]

Currently it seems to me like the source code is returned even if debug is enable, am looking at the wrong thing?

With debug

mj_env *env = mj_env_new();
mj_env_set_debug(env, true);

mj_env_add_template(env, "hello", "{% if");
mj_err_print();

// Output:
// error: syntax error: unexpected end of input, expected expression (in hello:1)
//
// ------------------------------------ hello ------------------------------------
//    1 > {% if
//      i    ^^ syntax error
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// No referenced variables
// -------------------------------------------------------------------------------

Without debug

mj_env *env = mj_env_new();

mj_env_add_template(env, "hello", "{% if");
mj_err_print();

// Output:
// error: syntax error: unexpected end of input, expected expression (in hello:1)
//
// ------------------------------------ hello ------------------------------------
//    1 > {% if
//      i    ^^ syntax error
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// No referenced variables
// -------------------------------------------------------------------------------

Edit: Seems to be only the case for mj_env_add_template, it works for mj_env_render_template for example

[maxbrunet]

Is parsing the string representation of an MJ value the expected way to convert it to a native type with minijinja-cabi? Or would it possible to return it directly (e.g. bool, int...)?

[mitsuhiko]

I hope I exposed enough API that you never need to go via strings. For which value types do you think you need to inspect the string representation?

[maxbrunet]

I am going through strings for all of them, except interables where there is enough to inspect the elements, am I missing something? 😅

E.g. maxbrunet/minijinja-go@d3d80af/value_decode.go#L53

[mitsuhiko]

Why don't you do this?

if (mj_value_get_kind(v) == MJ_VALUE_KIND_BOOL) {
     return mj_value_is_true(v);
}

Maybe I'm missing something here and we are discussing something else. But all the _as and _is methods are there to allow you to inspect values.

[maxbrunet]

how did I miss those 🤦, thanks!

[maxbrunet]

Would you have any recommendations how to get started with WASM and Rust :: wazero? Target wasm32-unknown-unknown vs wasm32-wasi? What Rust code would need to be written if any?

cargo build --package=minijinja --release --target=wasm32-unknown-unknown does not produce any output, it seems everything needs to be exported with no_mangle, does it mean a wrapper library would need to be written for that? Or is there a way to generate it?

I had a look at the new minijinja-js, but unless I am mistaken, it does not appear useful for wazero.

[mitsuhiko]

I am not sure what problems you are running into. MiniJinja in general compiles nicely to WASM as demonstrated both by minijinja-js and the playground which is also using WASM. That directly compiling minijinja for wasm doesn't have much useful output is likely because you will need to generate bindings. Otherwise the linker will see that nothing is used and throw everything away.

[maxbrunet]

That directly compiling minijinja for wasm doesn't have much useful output is likely because you will need to generate bindings. Otherwise the linker will see that nothing is used and throw everything away.

Yes, this sounds like what I observed.

I was able to build minijinja-js and minijinja-cabi to WASM, but both WASM modules do not seem to expose the necessary for Wazero to function. wasm-pack probably has its own magic specifically for Nodejs.

The Wazero doc shows an examples like:

#[no_mangle]
pub extern "C" fn add(x: i32, y: i32) -> i32 {
    x + y
}

And explains:

The rust source includes #[no_mangle] and extern "C". Add these to functions you want to export to the WebAssembly host.

So I guess I have to write a wrapper by hand to add #[no_mangle] and extern "C" everywhere, in addition to Go bindings

[mitsuhiko]

The general issue with WASM is that you need to target something. The js bindings are targeting WASM running in JavaScript, there can probably also be a WASM with WASI binding. But then what is on the other side? A template engine wants to convert objects somehow at the boundary and WASM does not define foreign objects. In fact, without platform support you can't even pass strings across the boundary (strings typically are pointers to somewhere in memory in WASM).

I think you need to better define what exactly you want to accomplish for this to make sense. If the goal is to make this work for Go which is also compiled to WASM, then I would expect this to work out of the box.

[maxbrunet]

A template engine wants to convert objects somehow at the boundary and WASM does not define foreign objects. In fact, without platform support you can't even pass strings across the boundary (strings typically are pointers to somewhere in memory in WASM).

Yes, I am not really familiar with WASM, that's the kind of limitations I need to discover and experiment with

If the goal is to make this work for Go which is also compiled to WASM, then I would expect this to work out of the box.

The goal would be running MiniJinja as a WASM program inside a Go amd64/arm64 binary

[maxbrunet]

I am seeing a weird issue with mj_err_get_template_name, sometimes the string is returned with an extra 0x7f byte/character (on amd64. I found it to be 0xff on arm64). It only occurs if the environment is also instantiated in the loop (if the env is instantiated before the loop and re-used inside, it does not happen).

The same occurs with mj_err_get_detail. I have not been able to reproduce from C.

Here is a minimal Go reproduction, anything that stands out in the handling of strings?

main.go

package main

// #cgo CFLAGS: -I${SRCDIR}/include
// #cgo LDFLAGS: -L${SRCDIR}/lib -lminijinja_cabi
// #include <stdlib.h>
// #include <minijinja.h>
import "C"

import (
	"fmt"
	"os"
	"unsafe"
)

func main() {
	name := "hello"
	cName := C.CString(name)
	defer C.free(unsafe.Pointer(cName))
	source := C.CString("{% if")
	defer C.free(unsafe.Pointer(source))

	fmt.Printf("%#x\n", name)

	for i := 0; ; i++ {
		env := C.mj_env_new()

		if ok := C.mj_env_add_template(env, cName, source); ok {
			panic("add template should fail")
		}
		errName := C.GoString(C.mj_err_get_template_name())

		if len(errName) > len(name) {
			fmt.Printf("%#x %d\n", errName, i)
			os.Exit(1)
		}

		C.mj_err_clear()
		C.mj_env_free(env)
	}
	// Output:
	// 0x68656c6c6f
	// 0x68656c6c6f7f 164549
}

attempted C reproduction

#include <minijinja.h>
#include <stdio.h>
#include <assert.h>
#include <string.h>

int main()
{
    bool ok;
    int i;
    const char *name = "hello";
    const char *source = "{% if";

    for (i = 0;;i++) {
        mj_env *env = mj_env_new();

        ok = mj_env_add_template(env, name, source);
        assert(!ok);

        const char *err_name = mj_err_get_template_name();

        printf("0x");
        for (int i = 0; name[i] != '\0'; i++) {
            printf("%x", (unsigned char)err_name[i]);
        }
        printf(" %d\n", i);

        assert(strlen(err_name) == strlen(name));

        mj_err_clear();
        mj_env_free(env);
    }

    return 0;
    // Output:
    // [...]
    // 0x68656c6c6f 1210956
    // 0x68656c6c6f 1210957
    // 0x68656c6c6f 1210958
    // 0x68656c6c6f 1210959
    // [...]
}

[maxbrunet]

Proposing a solution: #754. I tested it and it solves the problem. I think the alternative would be having CStrings in the Error struct and the caller would not have the responsibility to free them, but I suppose that is not an option since this struct is used in non-C code. The functions could also probably return the length of the strings and then they could be read without relying on the NUL-termination.