Fix bug where imports from imports located on search paths were not c…

…orrectly resolved.

_jsonnet.c

@@ -33,7 +33,8 @@ struct ImportCtx {
     PyObject *callback;
 };
 
-static char *cpython_import_callback(void *ctx_, const char *base, const char *rel, int *success)
+static char *cpython_import_callback(void *ctx_, const char *base, const char *rel,
+                                     char **found_here, int *success)
 {
     const struct ImportCtx *ctx = ctx_;
     PyObject *arglist, *result;
@@ -57,13 +58,25 @@ static char *cpython_import_callback(void *ctx_, const char *base, const char *r
         return out;
     }
 
-    if (!PyString_Check(result)) {
-        out = jsonnet_str(ctx->vm, "import_callback did not return a string");
+    if (!PyTuple_Check(result)) {
+        out = jsonnet_str(ctx->vm, "import_callback did not return a tuple");
+        *success = 0;
+    } else if (PyTuple_Size(result) != 2) {
+        out = jsonnet_str(ctx->vm, "import_callback did not return a tuple (size 2)");
         *success = 0;
     } else {
-        const char *result_cstr = PyString_AsString(result);
-        out = jsonnet_str(ctx->vm, result_cstr);
-        *success = 1;
+        PyObject *file_name = PyTuple_GetItem(result, 0);
+        PyObject *file_content = PyTuple_GetItem(result, 1);
+        if (!PyString_Check(file_name) || !PyString_Check(file_content)) {
+            out = jsonnet_str(ctx->vm, "import_callback did not return a pair of strings");
+            *success = 0;
+        } else {
+            const char *found_here_cstr = PyString_AsString(file_name);
+            const char *content_cstr = PyString_AsString(file_content);
+            *found_here = jsonnet_str(ctx->vm, found_here_cstr);
+            out = jsonnet_str(ctx->vm, content_cstr);
+            *success = 1;
+        }
     }
 
     Py_DECREF(result);

doc/bindings.html

@@ -226,9 +226,11 @@ <h2>Python API</h2>
 
 <p>Another keyword argument <tt>import_callback</tt> can be used to pass a callable to trap the
 Jsonnet import constructs.  This allows, e.g., reading files out of archives or implementing library
-search paths.  The supplied function must take two string arguments (current working directory and
-the string given to the import construct, which can usually be concatenated to form an actual path)
-and return a string (the content of the file).</p>
+search paths.  The supplied function must take two string arguments (directory of the current file
+and the string given to the import construct, which can usually be concatenated to form an actual
+path but that is up to you).  It returns a tuple of two strings, the first being the actual path of
+the file and the second being the content.  The actual path is required so that imports can be
+resolved within the imported file.</p>
 
 <p>If an error is raised during the evaluation of the Jsonnet code, it is formed into a stack trace
 and thrown as a python RuntimeError.  Otherwise, the JSON string is returned.  To convert this

doc/bindings.html.jinja

@@ -37,9 +37,11 @@ mapping strings to strings.  The other keyword arguments should be numbers.</p>
 
 <p>Another keyword argument <tt>import_callback</tt> can be used to pass a callable to trap the
 Jsonnet import constructs.  This allows, e.g., reading files out of archives or implementing library
-search paths.  The supplied function must take two string arguments (current working directory and
-the string given to the import construct, which can usually be concatenated to form an actual path)
-and return a string (the content of the file).</p>
+search paths.  The supplied function must take two string arguments (directory of the current file
+and the string given to the import construct, which can usually be concatenated to form an actual
+path but that is up to you).  It returns a tuple of two strings, the first being the actual path of
+the file and the second being the content.  The actual path is required so that imports can be
+resolved within the imported file.</p>
 
 <p>If an error is raised during the evaluation of the Jsonnet code, it is formed into a stack trace
 and thrown as a python RuntimeError.  Otherwise, the JSON string is returned.  To convert this

jsonnet.cpp

@@ -41,7 +41,8 @@ enum ImportStatus {
 };
 
 static enum ImportStatus try_path(const std::string &dir, const std::string &rel,
-                                  std::string &content, std::string &err_msg)
+                                  std::string &content, std::string &found_here,
+                                  std::string &err_msg)
 {
     std::string abs_path;
     if (rel.length() == 0) {
@@ -74,18 +75,26 @@ static enum ImportStatus try_path(const std::string &dir, const std::string &rel
         return IMPORT_STATUS_IO_ERROR;
     }
 
+    found_here = abs_path;
+
     return IMPORT_STATUS_OK;
 }
 
-static char *import_callback (void *ctx_, const char *dir, const char *file, int *success)
+static char *from_string(JsonnetVm* vm, const std::string &v)
+{   
+    char *r = jsonnet_realloc(vm, nullptr, v.length() + 1);
+    std::strcpy(r, v.c_str());
+    return r;
+}
+
+static char *import_callback (void *ctx_, const char *dir, const char *file,
+                              char **found_here_cptr, int *success)
 {
     const auto &ctx = *static_cast<ImportCallbackContext*>(ctx_);
 
-    std::string input;
-
-    std::string err_msg;
+    std::string input, found_here, err_msg;
 
-    ImportStatus status = try_path(dir, file, input, err_msg);
+    ImportStatus status = try_path(dir, file, input, found_here, err_msg);
 
     std::vector<std::string> jpaths(*ctx.jpaths);
 
@@ -98,21 +107,18 @@ static char *import_callback (void *ctx_, const char *dir, const char *file, int
             std::strcpy(r, err);
             return r;
         }
-        status = try_path(jpaths.back(), file, input, err_msg);
+        status = try_path(jpaths.back(), file, input, found_here, err_msg);
         jpaths.pop_back();
     }
 
     if (status == IMPORT_STATUS_IO_ERROR) {
         *success = 0;
-        char *r = jsonnet_realloc(ctx.vm, nullptr, err_msg.length() + 1);
-        std::strcpy(r, err_msg.c_str());
-        return r;
+        return from_string(ctx.vm, err_msg);
     } else {
         assert(status == IMPORT_STATUS_OK);
         *success = 1;
-        char *r = jsonnet_realloc(ctx.vm, nullptr, input.length() + 1);
-        std::strcpy(r, input.c_str());
-        return r;
+        *found_here_cptr = from_string(ctx.vm, found_here);
+        return from_string(ctx.vm, input);
     }
 }
 

jsonnet_test_file.py

@@ -12,15 +12,35 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
 import sys
+
 import _jsonnet
 
 if len(sys.argv) != 2:
-    raise Exception("Usage: <filename>")
+    raise Exception('Usage: <filename>')
+
+#  Returns content if worked, None if file not found, or throws an exception
+def try_path(dir, rel):
+    if not rel:
+        raise RuntimeError('Got invalid filename (empty string).')
+    if rel[0] == '/':
+        full_path = rel
+    else:
+        full_path = dir + rel
+    if full_path[-1] == '/':
+        raise RuntimeError('Attempted to import a directory')
+
+    if not os.path.isfile(full_path):
+        return full_path, None
+    with open(full_path) as f:
+        return full_path, f.read()
+
 
-def import_callback(a, b):
-    with open(a + b) as f:
-        content = f.read()
-    return content
+def import_callback(dir, rel):
+    full_path, content = try_path(dir, rel)
+    if content:
+        return full_path, content
+    raise RuntimeError('File not found')
 
 sys.stdout.write(_jsonnet.evaluate_file(sys.argv[1], import_callback=import_callback))

jsonnet_test_snippet.py

@@ -12,15 +12,35 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
 import sys
+
 import _jsonnet
 
 if len(sys.argv) != 2:
     raise Exception("Usage: <snippet>")
 
-def import_callback(a, b):
-    with open(a + b) as f:
-        content = f.read()
-    return content
+#  Returns content if worked, None if file not found, or throws an exception
+def try_path(dir, rel):
+    if not rel:
+        raise RuntimeError('Got invalid filename (empty string).')
+    if rel[0] == '/':
+        full_path = rel
+    else:
+        full_path = dir + rel
+    if full_path[-1] == '/':
+        raise RuntimeError('Attempted to import a directory')
+
+    if not os.path.isfile(full_path):
+        return full_path, None
+    with open(full_path) as f:
+        return full_path, f.read()
+
+
+def import_callback(dir, rel):
+    full_path, content = try_path(dir, rel)
+    if content:
+        return full_path, content
+    raise RuntimeError('File not found')
 
 sys.stdout.write(_jsonnet.evaluate_snippet("snippet", sys.argv[1], import_callback=import_callback))

libjsonnet.cpp

@@ -46,7 +46,8 @@ static char *from_string(JsonnetVm* vm, const std::string &v)
 
 /** Resolve the absolute path and use C++ file io to load the file.
  */
-static char *default_import_callback(void *ctx, const char *base, const char *file, int *success)
+static char *default_import_callback(void *ctx, const char *base, const char *file,
+                                     char **found_here_cptr, int *success)
 {
     auto *vm = static_cast<JsonnetVm*>(ctx);
 
@@ -77,6 +78,7 @@ static char *default_import_callback(void *ctx, const char *base, const char *fi
         std::string input;
         input.assign(std::istreambuf_iterator<char>(f), std::istreambuf_iterator<char>());
         *success = 1;
+        *found_here_cptr = from_string(vm, abs_path);
         return from_string(vm, input);
     } catch (const std::ios_base::failure &io_err) {
         *success = 0;

libjsonnet.h

@@ -58,10 +58,13 @@ void jsonnet_string_output(struct JsonnetVm *vm, int v);
  * \param ctx User pointer, given in jsonnet_import_callback.
  * \param base The directory containing the code that did the import.
  * \param rel The path imported by the code.
+ * \param found_here Set this byref param to path to the file, absolute or relative to the
+ *     process's CWD.  This is necessary so that imports from the content of the imported file can
+ *     be resolved correctly.  Allocate memory with jsonnet_realloc.  Only use when *success = 0.
  *\ param success Set this byref param to 1 to indicate success and 0 for failure.
  * \returns The content of the imported file, or an error message.
  */
-typedef char *JsonnetImportCallback(void *ctx, const char *base, const char *rel, int *success);
+typedef char *JsonnetImportCallback(void *ctx, const char *base, const char *rel, char **found_here, int *success);
 
 /** Allocate, resize, or free a buffer.  This will abort if the memory cannot be allocated.  It will
  * only return NULL if sz was zero.

vm.cpp

@@ -411,8 +411,14 @@ namespace {
         /** Used to "name" thunks crated on the inside of an array. */
         const Identifier *idArrayElement;
 
+        struct ImportCacheValue {
+            std::string foundHere;
+            std::string content;
+        };
+
         /** Cache for imported Jsonnet files. */
-        std::map<std::pair<std::string, std::string>, const std::string *> cachedImports;
+        std::map<std::pair<std::string, std::string>,
+                 const ImportCacheValue *> cachedImports;
 
         /** External variables for std.extVar. */
         StrMap externalVars;
@@ -644,14 +650,8 @@ namespace {
          */
         AST *import(const LocationRange &loc, const std::string &file)
         {
-            std::string dir = dir_name(loc.file);
-            const std::string *input = importString(loc, file);
-
-            std::string abs_file = file;
-            if (dir.length() > 0)
-                abs_file = dir + abs_file;
-
-            auto *expr = jsonnet_parse(alloc, abs_file, input->c_str());
+            const ImportCacheValue *input = importString(loc, file);
+            AST *expr = jsonnet_parse(alloc, input->foundHere, input->content.c_str());
             jsonnet_static_analysis(expr);
             return expr;
         }
@@ -663,19 +663,23 @@ namespace {
          *
          * \param loc Location of the import statement.
          * \param file Path to the filename.
+         * \param found_here If non-null, used to store the actual path of the file
          */
-        const std::string *importString(const LocationRange &loc, const std::string &file)
+        const ImportCacheValue *importString(const LocationRange &loc, const std::string &file)
         {
             std::string dir = dir_name(loc.file);
 
             std::pair<std::string, std::string> key(dir, file);
-            const std::string *str = cachedImports[key];
-            if (str != nullptr) return str;
+            const ImportCacheValue *cached_value = cachedImports[key];
+            if (cached_value != nullptr)
+                return cached_value;
 
 
             int success = 0;
+            char *found_here_cptr;
             char *content =
-                importCallback(importCallbackContext, dir.c_str(), file.c_str(), &success);
+                importCallback(importCallbackContext, dir.c_str(), file.c_str(),
+                               &found_here_cptr, &success);
 
             std::string input(content);
 
@@ -688,7 +692,8 @@ namespace {
                     throw makeError(loc, msg);
             }
 
-            std::string *input_ptr = new std::string(input);
+            auto *input_ptr = new ImportCacheValue{found_here_cptr, input};
+            ::free(found_here_cptr);
             cachedImports[key] = input_ptr;
             return input_ptr;
         }
@@ -1012,8 +1017,8 @@ namespace {
 
                 case AST_IMPORTSTR: {
                     const auto &ast = *static_cast<const Importstr*>(ast_);
-                    const std::string *str = importString(ast.location, ast.file);
-                    scratch = makeString(*str);
+                    const ImportCacheValue *value = importString(ast.location, ast.file);
+                    scratch = makeString(value->content);
                 } break;
 
                 case AST_INDEX: {