Fix symlink collision bug #3
Conversation
| # 2) if that fails don't create the symlink at all | ||
| # we can sometimes skip symlink creation because of ocassional | ||
| # duplicates in pkg_config output | ||
| if dest.exists: |
There was a problem hiding this comment.
Instead of just looking up 1 level in the directory tree, what do you think about generating unique identifiers from the complete srcpath? (For example: s.replace("_", "__").replace("/", "_slash_")?) This will be more robust to collisions in file paths.
| if dest.exists: | ||
| dest = base.get_child(src.basename + src.dirname.basename) | ||
| if _symlink_no_duplicate(ctx, src, dest): | ||
| result += [str(dest)[rootlen:]] |
There was a problem hiding this comment.
Modifying the name of the symlink that is created like this may lead to nondeterministic include path naming (or spooky action at a distance, where include path names depend on the order in which symlinks are created). Moreover, if you look at how the return value of _symlinks is used, you should see that this changes how the strip_include option to the pkg_config rule works.
For example, consider a package with include paths /usr/include/x86_64-linux-gnu/MyPackage and /usr/include/MyPackage. It provides the include files /usr/include/x86_64-linux-gnu/MyPackage/my.h and /usr/include/MyPackage/package.h.
A user of the pkg_config Bazel rule might specify a strip_prefix value of MyPackage, so that they can just #include "my.h" and #include "package.h" directly.
Under this PR, Bazel will create symlinks named includes/MyPackage and includes/include/MyPackage or includes/x86_64-linux-gnu/MyPackage, depending on which gets created first. There is no single strip_prefix value that can correctly strip out both of these. Furthermore, the names of the symlinks that get created depends on the order in which they are created, which is undesirable for multiple reasons.
There was a problem hiding this comment.
yes, that makes a lot of sense; a strip_prefix or prefix (for adding a path prefix)
Resolves #1
Tested with gtkmm