Revert to requiring Python import callbacks to return bytes rather than string, and add version string to python module

Author: sparkprime

python/_jsonnet.c

@@ -247,33 +247,21 @@ static int cpython_import_callback(void *ctx_, const char *base, const char *rel
         PyObject *file_name = PyTuple_GetItem(result, 0);
         PyObject *file_content = PyTuple_GetItem(result, 1);
 #if PY_MAJOR_VERSION >= 3
-        if (!PyUnicode_Check(file_name) || !(PyUnicode_Check(file_content) || PyBytes_Check(file_content))) {
+        if (!PyUnicode_Check(file_name) || !PyBytes_Check(file_content)) {
 #else
-        if (!PyString_Check(file_name) || !(PyString_Check(file_content) || PyBytes_Check(file_content))) {
+        if (!PyString_Check(file_name) || !PyBytes_Check(file_content)) {
 #endif
-            *buf = jsonnet_str_nonull(ctx->vm, "import_callback did not return (string, bytes) or (string, string)", buflen);
+            *buf = jsonnet_str_nonull(ctx->vm, "import_callback did not return (string, bytes)  Since 0.19.0 imports should be returned as bytes instead of as a string.  You may want to call .encode() on your string.", buflen);
             success = 0;
         } else {
-            const char *content_buf;
+            char *content_buf;
             ssize_t content_len;
 #if PY_MAJOR_VERSION >= 3
             const char *found_here_cstr = PyUnicode_AsUTF8(file_name);
 #else
             const char *found_here_cstr = PyString_AsString(file_name);
 #endif
-            if (PyBytes_Check(file_content)) {
-                char *content_buf2;
-                PyBytes_AsStringAndSize(file_content, &content_buf2, &content_len);
-                content_buf = content_buf2;
-            } else {
-#if PY_MAJOR_VERSION >= 3
-                content_buf = PyUnicode_AsUTF8(file_content);
-                content_len = strlen(content_buf);
-#else
-                content_buf = PyString_AsString(file_content);
-                content_len = strlen(content_buf);
-#endif
-            }
+            PyBytes_AsStringAndSize(file_content, &content_buf, &content_len);
             *found_here = jsonnet_str(ctx->vm, found_here_cstr);
             *buflen = content_len;
             *buf = jsonnet_realloc(ctx->vm, NULL, *buflen);
@@ -677,11 +665,18 @@ static struct PyModuleDef _jsonnet =
 
 PyMODINIT_FUNC PyInit__jsonnet(void)
 {
-    return PyModule_Create(&_jsonnet);
+    PyObject *module = PyModule_Create(&_jsonnet);
+    PyObject *version_str = PyUnicode_FromString(LIB_JSONNET_VERSION);
+    PyModule_AddObjectRef(module, "version", version_str);
+    return module;
 }
 #else
 PyMODINIT_FUNC init_jsonnet(void)
 {
-    Py_InitModule3("_jsonnet", module_methods, "A Python interface to Jsonnet.");
+    PyObject *module = Py_InitModule3("_jsonnet", module_methods, "A Python interface to Jsonnet.");
+    PyObject *version_str = PyUnicode_FromString(LIB_JSONNET_VERSION);
+    if (PyModule_AddObject(module, "version", PyString_FromString(LIB_JSONNET_VERSION)) < 0) {
+      Py_XDECREF(version_str);
+    }
 }
 #endif

python/_jsonnet_test.py

@@ -24,9 +24,7 @@
 # is invalid or an IO error occured.
 # It caches both hits and misses in the `cache` dict. Exceptions
 # do not need to be cached, because they abort the computation anyway.
-# if do_encode is True, then the file is returned as bytes
-# instead of as a string (this does not make a difference in Python2).
-def try_path_cached(cache, dir, rel, do_encode):
+def try_path_cached(cache, dir, rel):
     if not rel:
         raise RuntimeError('Got invalid filename (empty string).')
     if rel[0] == '/':
@@ -40,22 +38,12 @@ def try_path_cached(cache, dir, rel, do_encode):
             cache[full_path] = None
         else:
             with open(full_path) as f:
-                if do_encode:
-                    cache[full_path] = f.read().encode()
-                else:
-                    cache[full_path] = f.read()
+                cache[full_path] = f.read().encode()
     return full_path, cache[full_path]
 
 def import_callback_encode(dir, rel):
     cache = {}
-    full_path, content = try_path_cached(cache, dir, rel, do_encode=True)
-    if content:
-        return full_path, content
-    raise RuntimeError('File not found')
-
-def import_callback_no_encode(dir, rel):
-    cache = {}
-    full_path, content = try_path_cached(cache, dir, rel, do_encode=False)
+    full_path, content = try_path_cached(cache, dir, rel)
     if content:
         return full_path, content
     raise RuntimeError('File not found')
@@ -64,10 +52,6 @@ def import_callback_empty_file_encode(dir, rel):
     return dir, b''
 
 
-def import_callback_empty_file_no_encode(dir, rel):
-    return dir, ''
-
-
 # Test native extensions
 def concat(a, b):
     return a + b
@@ -101,6 +85,9 @@ def setUp(self):
         with open(self.input_filename, "r") as infile:
             self.input_snippet = infile.read()
 
+    def test_version(self):
+        self.assertEqual(type(_jsonnet.version), str)
+
     def test_evaluate_file_encode(self):
         json_str = _jsonnet.evaluate_file(
             self.input_filename,
@@ -109,14 +96,6 @@ def test_evaluate_file_encode(self):
         )
         self.assertEqual(json_str, "true\n")
 
-    def test_evaluate_file_no_encode(self):
-        json_str = _jsonnet.evaluate_file(
-            self.input_filename,
-            import_callback=import_callback_no_encode,
-            native_callbacks=native_callbacks,
-        )
-        self.assertEqual(json_str, "true\n")
-
     def test_evaluate_snippet_encode(self):
         json_str = _jsonnet.evaluate_snippet(
             self.test_filename,
@@ -126,15 +105,6 @@ def test_evaluate_snippet_encode(self):
         )
         self.assertEqual(json_str, "true\n")
 
-    def test_evaluate_snippet_no_encode(self):
-        json_str = _jsonnet.evaluate_snippet(
-            self.test_filename,
-            self.input_snippet,
-            import_callback=import_callback_no_encode,
-            native_callbacks=native_callbacks,
-        )
-        self.assertEqual(json_str, "true\n")
-
     def test_evaluate_snippet_encode(self):
         json_str = _jsonnet.evaluate_snippet(
             self.test_filename,
@@ -144,15 +114,6 @@ def test_evaluate_snippet_encode(self):
         )
         self.assertEqual(json_str, "true\n")
 
-    def test_evaluate_snippet_no_encode(self):
-        json_str = _jsonnet.evaluate_snippet(
-            self.test_filename,
-            self.input_snippet,
-            import_callback=import_callback_no_encode,
-            native_callbacks=native_callbacks,
-        )
-        self.assertEqual(json_str, "true\n")
-
     def test_import_encode(self):
         json_str = _jsonnet.evaluate_snippet(
             self.test_filename,
@@ -162,15 +123,6 @@ def test_import_encode(self):
         )
         self.assertEqual(json_str, "42\n")
 
-    def test_import_no_encode(self):
-        json_str = _jsonnet.evaluate_snippet(
-            self.test_filename,
-            "import 'trivial.jsonnet'",
-            import_callback=import_callback_no_encode,
-            native_callbacks=native_callbacks,
-        )
-        self.assertEqual(json_str, "42\n")
-
     def test_import_no_eol_encode(self):
         json_str = _jsonnet.evaluate_snippet(
             self.test_filename,
@@ -180,15 +132,6 @@ def test_import_no_eol_encode(self):
         )
         self.assertEqual(json_str, "42\n")
 
-    def test_import_no_eol_no_encode(self):
-        json_str = _jsonnet.evaluate_snippet(
-            self.test_filename,
-            "import 'trivial_no_eol.jsonnet'",
-            import_callback=import_callback_no_encode,
-            native_callbacks=native_callbacks,
-        )
-        self.assertEqual(json_str, "42\n")
-
     def test_import_binary_encode(self):
         json_str = _jsonnet.evaluate_snippet(
             self.test_filename,
@@ -198,15 +141,6 @@ def test_import_binary_encode(self):
         )
         self.assertEqual(json_str, "[\n   1,\n   2,\n   3\n]\n")
 
-    def test_import_binary_no_encode(self):
-        json_str = _jsonnet.evaluate_snippet(
-            self.test_filename,
-            "importbin 'binary123.bin'",
-            import_callback=import_callback_no_encode,
-            native_callbacks=native_callbacks,
-        )
-        self.assertEqual(json_str, "[\n   1,\n   2,\n   3\n]\n")
-
     def test_import_binary_sentinel_encode(self):
         json_str = _jsonnet.evaluate_snippet(
             self.test_filename,
@@ -216,20 +150,6 @@ def test_import_binary_sentinel_encode(self):
         )
         self.assertEqual(json_str, "[\n   1,\n   2,\n   3,\n   0,\n   1,\n   2,\n   3\n]\n")
 
-    def test_import_binary_sentinel_no_encode(self):
-        json_str = _jsonnet.evaluate_snippet(
-            self.test_filename,
-            "importbin 'binary1230123.bin'",
-            import_callback=import_callback_no_encode,
-            native_callbacks=native_callbacks,
-        )
-        if sys.version_info.major < 3:
-            # In Python2, the string can actually have a 0 in the middle
-            self.assertEqual(json_str, "[\n   1,\n   2,\n   3,\n   0,\n   1,\n   2,\n   3\n]\n")
-        else:
-            # In Pyton3, the string is truncated
-            self.assertEqual(json_str, "[\n   1,\n   2,\n   3\n]\n")
-
     def test_import_str_empty_file_encode(self):
         json_str = _jsonnet.evaluate_snippet(
             self.test_filename,
@@ -239,15 +159,6 @@ def test_import_str_empty_file_encode(self):
         )
         self.assertEqual(json_str, "\"\"\n")
 
-    def test_import_str_empty_file_no_encode(self):
-        json_str = _jsonnet.evaluate_snippet(
-            self.test_filename,
-            "importstr 'binary123.bin'",
-            import_callback=import_callback_empty_file_no_encode,
-            native_callbacks=native_callbacks,
-        )
-        self.assertEqual(json_str, "\"\"\n")
-
     def test_import_binary_empty_file_encode(self):
         json_str = _jsonnet.evaluate_snippet(
             self.test_filename,
@@ -257,15 +168,6 @@ def test_import_binary_empty_file_encode(self):
         )
         self.assertEqual(json_str, "[ ]\n")
 
-    def test_import_binary_empty_file_no_encode(self):
-        json_str = _jsonnet.evaluate_snippet(
-            self.test_filename,
-            "importbin 'binary123.bin'",
-            import_callback=import_callback_empty_file_no_encode,
-            native_callbacks=native_callbacks,
-        )
-        self.assertEqual(json_str, "[ ]\n")
-
     def test_double_import(self):
         json_str = _jsonnet.evaluate_snippet(
             self.test_filename,