Opened 14 years ago

Closed 14 years ago

#2433 closed patch (invalid)

Fix relative paths from absolute-with-symlinks

Reported by: strk Owned by: nobody
Priority: minor: annoyance Milestone: Version 1.5.0
Component: Project Loading / Saving Version: Trunk
Keywords: Cc:
Must Fix for Release: No Platform: Debian
Platform Version: Awaiting user input: yes

Description

If your project file refers to resources by absolute path but the absolute path is formed by symlinks, the current code resolve them to a suboptimal ../../../../../looking/path.

This patch uses QT library to "canonicalize" the absolute path prior to resolve against project dir.

It is not tested under WIN. It would likely need canonicalization of the project filename too to be complete.

Attachments (2)

relative_path_from_symlink.diff (533 bytes ) - added by strk 14 years ago.
relative_path_from_symlink_2.diff (2.8 KB ) - added by strk 14 years ago.
version checking for canonicalPath to exist before using it and canonicalizing project filename too

Download all attachments as: .zip

Change History (10)

comment:1 by jef, 14 years ago

Awaiting user input: set
Component: Build/InstallProject Loading / Saving

What's the point in using two different paths to the same directory in one project?

If you use the same path for both the datasources and the project, you already get what you want - whether or not the path includes a symlink.

If different paths are used, it might be even to get exactly this behavior.

Moreover canonicalPath() only works on existing files and (some?) GRASS data sources have paths that only exist partly - and the patch would clear those. This one wouldn't:

Index: src/core/qgsproject.cpp
===================================================================
--- src/core/qgsproject.cpp	(revision 12928)
+++ src/core/qgsproject.cpp	(working copy)
@@ -1409,6 +1409,9 @@
   }
 
   QString srcPath = src;
+  QFileInfo fi( srcPath );
+  if ( fi.exists() )
+    srcPath = fi.canonicalFilePath();
   QString projPath = fileName();
 
 #if defined( Q_OS_WIN )

Still IMHO that patch is unnecessary and might even be counter productive.

in reply to:  1 ; comment:2 by strk, 14 years ago

Replying to jef:

What's the point in using two different paths to the same directory in one project?

No point. It was qgis doing that, I just used the "browse" dialog to select the files, and maybe I started it from my home directory so had to go find the files somewhere. Didn't know I would have been somewhere else when re-opening the project and "saving as relative".

If you use the same path for both the datasources and the project, you already get what you want - whether or not the path includes a symlink.

If different paths are used, it might be even to get exactly this behavior.

I'm trying hard but I can't figure a reason why a user would ever want to get that behaviour. I would really expect it to "just work".

Of course an *additional* "canonicalization" of paths could be performed at "load time" but you never know what the user want. He may have added a symlink for the precise reason to find a predictable *absolute* path in the project file for example... ... but, when you want to go "relative" you really want the relative path to be as small as possible and that's what you get if you canonicalize both paths in the project file and pathname of the project file itself. My patch only canonicalize the paths in the project file.

Moreover canonicalPath() only works on existing files and (some?) GRASS data sources have paths that only exist partly - and the patch would clear those. This one wouldn't:

Index: src/core/qgsproject.cpp
===================================================================
--- src/core/qgsproject.cpp	(revision 12928)
+++ src/core/qgsproject.cpp	(working copy)
@@ -1409,6 +1409,9 @@
   }
 
   QString srcPath = src;
+  QFileInfo fi( srcPath );
+  if ( fi.exists() )
+    srcPath = fi.canonicalFilePath();
   QString projPath = fileName();
 
 #if defined( Q_OS_WIN )

Still IMHO that patch is unnecessary and might even be counter productive.

I would rather get a warning popup if a filename in the project file can't be found. After all you can't guarantee you'll produce a project file that will work if you can't find the files.

in reply to:  2 ; comment:3 by jef, 14 years ago

Replying to strk:

Replying to jef:

What's the point in using two different paths to the same directory in one project?

No point. It was qgis doing that, I just used the "browse" dialog to select the files, and maybe I started it from my home directory so had to go find the files somewhere.

I don't understand. Didn't you use two different paths to the same directory and QGIS merily didn't notice or care, that there was a symlink involved?

I'm trying hard but I can't figure a reason why a user would ever want to get that behaviour. I would really expect it to "just work".

I does just work, doesn't it? It's just that the paths in the project file aren't the one you expected.

I would rather get a warning popup if a filename in the project file can't be found. After all you can't guarantee you'll produce a project file that will work if you can't find the files.

I was not implying that anything can't be found. For GRASS rasters the "filename" isn't actually a path to a file. The referred information is in multiple files in the directories within the GRASS mapset. The path to the mapset is still valid, existing and can be expresses relatively to the project file.

in reply to:  3 comment:4 by strk, 14 years ago

Replying to jef:

Replying to strk:

Replying to jef:

What's the point in using two different paths to the same directory in one project?

No point. It was qgis doing that, I just used the "browse" dialog to select the files, and maybe I started it from my home directory so had to go find the files somewhere.

I don't understand. Didn't you use two different paths to the same directory and QGIS merily didn't notice or care, that there was a symlink involved?

I never used paths *explicitly*. What I did was requesting the load of two shapefiles. Most likely the path to the directories containing the two shapefiles were the same as IIRC the file browser remembers last location used. In any case the paths found in the .qgs file shared the path up to (but not including) the filename, so that part was fine.

Now, the "problem" (qgis problem, IMHO), is that these paths were reached using symlinks.

Next, I wanted to convert my "project file" into something I could send to my client. A zip file containing the shapefiles AND the qgis project file. Easy thing shouldnt' it be ? At this time BOTH the shapefiles AND the project file were already in the same directory.

So I went to "project options" and specified "save paths as relative". I expected qgis being able to find them, as BOTH the shapefiles AND the project file were in the same *canonical* directory.

I does just work, doesn't it? It's just that the paths in the project file aren't the one you expected.

Yes, if you want. It still works for me, but still can't put in the zip file :)

For GRASS rasters the "filename" isn't actually a path to a file. The referred information is in multiple files in the directories within the GRASS mapset. The path to the mapset is still valid, existing and can be expresses relatively to the project file.

Uhm... ok, what about passing the dirname to the canonicalizator function then ? Is the dirname expected to exist ? Or, if other kind of "source-specification" exist, is there a way to tell what refer to a filesystem path and what not ?

Mind you: I didn't introduce this "relative paths" thing myself in qgis, so please don't take the chance to have me rewrite it all :P

comment:5 by strk, 14 years ago

How about adding this:

if ( srcPath.empty() ) srcPath = src;

Between lines 1413 and 1414 of my patch ? QDir::canonicalPath() is documented to return the empty string if the canonical path doesn't exist

comment:6 by strk, 14 years ago

I add an updated patch. Checks if the path does have a "canonical" representation (it does if it exists on the filesystem as such) and falls-back to using the original representation.

Also, canonicalizes the project filename too (this one might be unneeded if fileName() returns it already canonicalized, which I didn't check).

by strk, 14 years ago

version checking for canonicalPath to exist before using it and canonicalizing project filename too

comment:7 by strk, 14 years ago

Argh, forget the second patch. It would abort. I was assuming too much.

I think I'm off on this, isn't that much important to justify the time spent.

comment:8 by mhugent, 14 years ago

Resolution: invalid
Status: newclosed

So we can close this ticket, right?

Note: See TracTickets for help on using tickets.