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

Exported relative paths don't respect symlinks #1589

Closed
kdex opened this Issue Jun 1, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@kdex

kdex commented Jun 1, 2017

On my machine, there is a directory named srv in my home directory which is a symlink to /srv/http.

Right now, this is part of the directory tree of ~/srv/map (realpath: /srv/http/map):

.
└── assets
|      └── tilesets
|             └── tileset.png
└── intro.tmx

The file ~/srv/map/intro.tmx uses ~/srv/map/assets/tilesets/tileset.png. Therefore, when I export ~/srv/map/intro.tmx to ~/srv/map/assets/maps/intro.json, I'd expect the relative path in the JSON file to become ../tilesets/tileset.png.

The current version, however, creates ../../../../../../srv/http/map/assets/tilesets/tileset.png, which essentially makes the file kind of unusable on my target platform without messing with the values.

Could you please respect symlinks and not force realpath?

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Jun 5, 2017

Owner

Hmm, I couldn't immediately reproduce this problem. I've re-created the situation as follows:

file ~/srv/http/map/assets/maps/intro.json
file ~/srv/http/map/assets/tilesets/sewer_tileset.png
file ~/srv/http/map/intro.tmx
symlink ~/home/bjorn/map -> ~/srv/http/map

When I load ~/home/bjorn/map/intro.tmx in Tiled and export it to ~/home/bjorn/map/assets/maps/intro.json, I get a reference to sewer_tileset.png like "image":"..\/tilesets\/sewer_tileset.png", which was what should happen.

However, when I open the file again, now using the Recent Files menu, it opened it from the canonical path, so the map directory from Tiled's perspective was now ~/srv/http/map/intro.tmx and the tileset directory was ~/srv/http/map/assets/tilesets/sewer_tileset.png. And then, when exporting I picked the location through the symbolic link, so exporting to ~/home/bjorn/map/assets/maps/intro.json, which then yields the unwanted reference "image":"..\/..\/..\/..\/..\/srv\/http\/map\/assets\/tilesets\/sewer_tileset.png".

Given that I think we want the relative link to rely on symlinks and not to always use canonical paths, for reasons I commented about at #1591, maybe the fix for this issue would be to avoid saving the recent file list as canonical paths, and just use absolute paths instead?

Owner

bjorn commented Jun 5, 2017

Hmm, I couldn't immediately reproduce this problem. I've re-created the situation as follows:

file ~/srv/http/map/assets/maps/intro.json
file ~/srv/http/map/assets/tilesets/sewer_tileset.png
file ~/srv/http/map/intro.tmx
symlink ~/home/bjorn/map -> ~/srv/http/map

When I load ~/home/bjorn/map/intro.tmx in Tiled and export it to ~/home/bjorn/map/assets/maps/intro.json, I get a reference to sewer_tileset.png like "image":"..\/tilesets\/sewer_tileset.png", which was what should happen.

However, when I open the file again, now using the Recent Files menu, it opened it from the canonical path, so the map directory from Tiled's perspective was now ~/srv/http/map/intro.tmx and the tileset directory was ~/srv/http/map/assets/tilesets/sewer_tileset.png. And then, when exporting I picked the location through the symbolic link, so exporting to ~/home/bjorn/map/assets/maps/intro.json, which then yields the unwanted reference "image":"..\/..\/..\/..\/..\/srv\/http\/map\/assets\/tilesets\/sewer_tileset.png".

Given that I think we want the relative link to rely on symlinks and not to always use canonical paths, for reasons I commented about at #1591, maybe the fix for this issue would be to avoid saving the recent file list as canonical paths, and just use absolute paths instead?

@kdex

This comment has been minimized.

Show comment
Hide comment
@kdex

kdex Jun 5, 2017

@bjorn Interesting. I'll close #1591, as it doesn't appear to fix the core problem we're experiencing. I've already been sketching out some more symlink-related cases. It would be interesting if your proposed solution handles these cases correctly:

.
└── assets
└── intro.tmx

(scenario: no symlinks involved, solution: already covered)

.
└── assets -> symlink to /usr/share/tilesets
└── intro.tmx

(scenario: references contain symlink, solution: don't expand symlinks)

. -> symlink to ~/srv
└── assets
|      └── tilesets
|             └── tileset.png
└── intro.tmx

(scenario: tmx file path contains symlink, solution: only use canonical paths)

. -> symlink to ~/srv
└── assets -> symlink to /usr/share/tilesets
└── intro.tmx

(scenario: tmx file path contains symlink and references contain symlink, solution: ?)

kdex commented Jun 5, 2017

@bjorn Interesting. I'll close #1591, as it doesn't appear to fix the core problem we're experiencing. I've already been sketching out some more symlink-related cases. It would be interesting if your proposed solution handles these cases correctly:

.
└── assets
└── intro.tmx

(scenario: no symlinks involved, solution: already covered)

.
└── assets -> symlink to /usr/share/tilesets
└── intro.tmx

(scenario: references contain symlink, solution: don't expand symlinks)

. -> symlink to ~/srv
└── assets
|      └── tilesets
|             └── tileset.png
└── intro.tmx

(scenario: tmx file path contains symlink, solution: only use canonical paths)

. -> symlink to ~/srv
└── assets -> symlink to /usr/share/tilesets
└── intro.tmx

(scenario: tmx file path contains symlink and references contain symlink, solution: ?)

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Jun 5, 2017

Owner

Since in some scenarios expanding symlinks is going to cause problems, I think this solution is simply not possible. Hence I think the problem is mostly in that Tiled uses canonical paths to store its recent files list. When it wouldn't, then the user could simply be instructed to make sure to always open his files either through the symlink or not using the symlink, and never to mix these.

Owner

bjorn commented Jun 5, 2017

Since in some scenarios expanding symlinks is going to cause problems, I think this solution is simply not possible. Hence I think the problem is mostly in that Tiled uses canonical paths to store its recent files list. When it wouldn't, then the user could simply be instructed to make sure to always open his files either through the symlink or not using the symlink, and never to mix these.

@bjorn bjorn added this to Up Next in Roadmap Jun 8, 2017

@bjorn bjorn moved this from Up Next to In Progress in Roadmap Jun 12, 2017

@bjorn bjorn closed this in 50ab693 Jun 12, 2017

@kdex

This comment has been minimized.

Show comment
Hide comment
@kdex

kdex Jun 12, 2017

Thanks a lot, @bjorn! 👍

kdex commented Jun 12, 2017

Thanks a lot, @bjorn! 👍

@bjorn bjorn removed this from In Progress in Roadmap Jun 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment