Import path should be opaque

[petr-k]

When importing jsonnet documents using the import statement, the current implementation assumes that the resource returned from importer's FoundHere is a filesystem path. On subsequent imports, the base dir of that path is derived via a call to path.Dir() (see e.g. https://github.com/google/go-jsonnet/blob/master/interpreter.go#L415). This function always performs a path cleanup.

This is a problem when implementing custom importers that do not use filesystem paths, but for example URLs. E.g. when an importer returns https://github.com/raw/ksonnet/ksonnet-lib/master/ksonnet.beta.2/k.libsonnet in FoundHere, upon resolving relative imports based on that URL, the importDir comes back to the importer cleaned up as https:/raw.githubusercontent.com/ksonnet/ksonnet-lib/master/ksonnet.beta.2 (notice the single forward slash in https:/).

go-jsonnet should not assume filesystem paths, in fact the resource "path" should be opaque.

[sbarzowski]

You're right. Thanks for pointing this out.

This was done for compatibility with C++ jsonnet.

IMO the right way to do it would be to have canonicalizePath(codePath, importedPath string) function in Importer interface. This way interpreter would assume nothing about the structure of the path. That would also help with caching (caching once per code directory is not ideal). Having a way to canonicalize paths would let us cache every file exactly once.

@sparkprime What do you think?

[sparkprime]

Yes I was thinking the same. I also think that was proposed in google/jsonnet#239

[sparkprime]

In other words, if the importer is responsible for deriving the new path from the current file and the possibly relative import string, then there is no need for it to expose a splitting function.

[petr-k]

Just to provide some context, I stumbled upon this when implementing an importer that would support URLs in import statements and search paths for the [kubecfg] tool: (https://github.com/ksonnet/kubecfg) vmware-archive/kubecfg#182

[sparkprime]

C++ implementation did not canonicalize paths: https://github.com/google/jsonnet/blob/master/core/vm.cpp#L37

[sbarzowski]

This has been solved a while back.