"normalize" path instead of realpath(3) #112

New Issue

ariadne · 2017-02-04T06:05:44Z

jhgit commented

2017-02-04 06:05:44 +00:00

(Migrated from github.com)

Here is a patch to "normalize" path to remove extra / path separators rather than using realpath(3).

4cb900d08d

===========
Normalize the path to remove duplicate / separators rather than
possibly altering the path with realpath(3). Leave sym links
as is in path components. This is also cheaper than realpath(3),
and works on platforms that don't have realpath(3).

Note: if this is accepted, the check for realpath in configure.ac
can be removed, and some docs that mention realpath will be adjusted.

==============

If that seems good, I'll tidy up the dangling parts that reference realpath.

Let me know what you think.

See also discussion at https://github.com/pkgconf/pkgconf/issues/110. Note a couple extra comments after issue 110 was closed (more comments about the merits of not using realpath to normalize the path).

Here is a patch to "normalize" path to remove extra / path separators rather than using realpath(3). https://github.com/jhgit/pkgconf/commit/4cb900d08dbb98fafd7b839b6859e1bfc4b29b2e =========== Normalize the path to remove duplicate / separators rather than possibly altering the path with realpath(3). Leave sym links as is in path components. This is also cheaper than realpath(3), and works on platforms that don't have realpath(3). Note: if this is accepted, the check for realpath in configure.ac can be removed, and some docs that mention realpath will be adjusted. ============== If that seems good, I'll tidy up the dangling parts that reference realpath. Let me know what you think. See also discussion at https://github.com/pkgconf/pkgconf/issues/110. Note a couple extra comments after issue 110 was closed (more comments about the merits of not using realpath to normalize the path).

ariadne commented

2017-02-04 06:08:02 +00:00

In some cases (path list deduplication such as PKG_CONFIG_PATH handling), realpath(3) may be better. What do you think about optionally using realpath() in those cases?

jhgit commented

2017-02-04 06:28:16 +00:00

(Migrated from github.com)

Can you give me an example of what you mean (by the path list dedup)?

I'm okay with options, but I also appreciate the value of avoiding extra complexity.

Can you give me an example of what you mean (by the path list dedup)? I'm okay with options, but I also appreciate the value of avoiding extra complexity.

ariadne commented

2017-02-04 08:06:55 +00:00

realpath(3) is desirable for the issue described at https://bugs.freedesktop.org/show_bug.cgi?id=79777

jhgit commented

2017-02-04 16:42:30 +00:00

(Migrated from github.com)

Ah. I understand now what you mean. I thought that problem (PKG_CONFIG_PATH dup) was solved with the PKGCONF_CACHE_INODES code introduced in 1.1.0 (which doesn't need realpath). Hmm... not if the sym link is the leaf I guess, since then the inodes for two otherwise identical pc path dirs would be different. I guess I would have used readlink(2) for each entry in PKG_CONFIG_PATH [1]. That's more efficient than using realpath on all file lookups, and also readlink(2) is available on more platforms than realpath(3).

[1] or just add "/." to the path when you do the inode lookup.

Ah. I understand now what you mean. I thought that problem (PKG_CONFIG_PATH dup) was solved with the PKGCONF_CACHE_INODES code introduced in 1.1.0 (which doesn't need realpath). Hmm... not if the sym link is the leaf I guess, since then the inodes for two otherwise identical pc path dirs would be different. I guess I would have used readlink(2) for each entry in PKG_CONFIG_PATH [1]. That's more efficient than using realpath on _all_ file lookups, and also readlink(2) is available on more platforms than realpath(3). [1] or just add "/." to the path when you do the inode lookup.

ariadne commented

2017-02-05 00:33:48 +00:00

I'm open to adjusting the behaviour for 1.3 series. I don't necessarily like the inode/dnode stuff, but I originally went that way to avoid using realpath(3). We wound up adding realpath(3) anyway though...

jhgit commented

2017-02-05 21:57:18 +00:00

(Migrated from github.com)

Fix inode caching to check link target rather than link (if PKG_CONFIG_PATH is a sym link):

5177d97477

Fix inode caching to check link target rather than link (if PKG_CONFIG_PATH is a sym link): https://github.com/jhgit/pkgconf/commit/5177d97477ffd745fb7362c981b9078a09292637

jhgit commented

2017-02-05 22:15:22 +00:00

(Migrated from github.com)

I think I prefer the inode checks for PKG_CONFIG_PATH dedup over realpath(3) for efficiency reasons.

And I like path normalizing over realpath(3) for efficiency and fewer undesired side effects (undesired sym link expansion; realpath(3) only works for existing matching filesystem layout - may not be desired for chroot and cross build usage).

My main sticking point with realpath right now is that the paths in -I arguments from 'pkgconf --cflags --libs' have changed since realpath was added for path lookups - and they get expanded to host-specific paths on the build machine. Especially since some packages store those paths in distributed files.

I think I prefer the inode checks for PKG_CONFIG_PATH dedup over realpath(3) for efficiency reasons. And I like path normalizing over realpath(3) for efficiency and fewer undesired side effects (undesired sym link expansion; realpath(3) only works for existing matching filesystem layout - may not be desired for chroot and cross build usage). My main sticking point with realpath right now is that the paths in -I arguments from 'pkgconf --cflags --libs' have changed since realpath was added for path lookups - and they get expanded to host-specific paths on the build machine. Especially since some packages store those paths in distributed files.

ariadne commented

2017-02-09 21:55:12 +00:00

Lets go with this for 1.3 series. Can you prepare the pull request? We are likely to do 1.3 soonish.