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

Resolve relative paths to source projects #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eschwartz
Copy link

eg. tmsource://../../project.tm2source

eg. `tmsource://../../project.tm2source`
@eschwartz
Copy link
Author

I have two main use cases for this:

  • I am editing style projects on my local machine, which point to source projects (also on my local machine). Once I'm done editing, I deploy the project files to a remote server for generating tiles. Because the file systems differ, I cannot have project files which work on both my local machine and the remote server.
  • I have a system which generates project files ("Project Manager"), and saves them to cloud storage. Another system ("Tile Server") downloads the project files, and serves tiles. The Project Manager has no way to know absolute paths on the Tile Server.

I am aware that this format will not work in Mapbox Studio Classic. However, resolving relative paths in tilelive would still be a big help.

I have tested this locally, but I'm a little nervous about breaking functionality that I don't even know about. Do you have any way to run it through some existing projects, and make sure I'm not breaking anything here?

toUri = url.parse(toUri);

// Url.parse reads the first ".." as the hostname
const toUriPathName = path.join(toUri.hostname, toUri.path);
Copy link
Owner

Choose a reason for hiding this comment

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

fromUri should get the same treatment.

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@mojodna
Copy link
Owner

mojodna commented May 6, 2016

This is definitely a useful thing, though it has me thinking that having to whitelist is a little bit gnarly (and wouldn't catch nested sources, such as those used by tilelive-merge).

In theory, other modules should be able to resolve relative paths based on process.cwd(), but that assumes that references are relative to your working directory not the referencing config and dynamically setting process.cwd() would have unforeseen consequences.

I'm wondering if using a special token (__dirname, which already has meaning within a source file) as the host portion of a URI possibly makes more sense. That would allow tilelive-tmstyle (for example) to replace the token with the project's path before loading it. I.e. tmsource://__dirname/../../project.tm2source referenced from tmstyle:///path/to/whatever.tm2 would be converted to tmsource:///path/to/whatever.tm2/../../project.tm2source before being loaded.

Thoughts?

@eschwartz
Copy link
Author

Thanks for the notes.

I agree that the whitelist is a little bit gnarly. There's no way for tilelive-tmstyle to have a definitive list of all available tilelive implementations. It would be a bummer for new implementations not to work, because tilelive-tmstyle didn't hard-code them into its whitelist.

I see what you're saying about confusion with tilelive implementation which don't use the file system. But it seems to me that if you configure your project.yml with a relative source path, you are clearly intending to reference the file system. ie. if I look at my project.yml and see source: tilelive-http://../../proj.tmsource, I'm going to know that something is wrong, and I would expect it to behave strangely.

I would say -- let's always resolve relative paths, and leave it up to the user to configure their project properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants