for wheel, not follow symbolic links

[lucasiscovici2]

When a symbolic link is included into a wheel, this link is resolved an it may happen that the resource is not added as expected

• [x] Added tests for changed code.
• [x] Updated documentation for changed code.

I added the possibility to not resolve symbolic link in BuildIncludeFile.

Path.resolve uses os.path.realpath which "eliminates any symbolic links encountered in the path".

The only possibility is to use Path.absolute which is currently not documented/tested

Allow the builder to not resolve symlinks when "find_files_to_add" Only set resolve_symlinks(BuildIncludeFile) to true when resolve_symlinks(find_files_to_add) is True or file is not a symlink

Only set resolve_symlinks to false when building wheel

Add test_package_with_include_symbolic_links inside tests/masonry/builders/test_complete.py

[neersighted]

@python-poetry/triage

In the future, please refrain from pinging on PRs that are this new. This change is not obviously correct -- what issue are you trying to solve? Are you attempting to ship symlinks inside a built wheel? What is the use case/is this a common practice permitted by other tools or required for a specific use case?

[neersighted]

In fact, symlinks are currently outside the wheel spec and not supported.

See discussion at https://discuss.python.org/t/symbolic-links-in-wheels/1945 for more information.

[lucasiscovici2]

@neersighted hello, and thank you for your answer. But i don't want build wheel with symlink ! I only don't want to follow symlink when building a wheel with a include file/dir which is a symlink, and add the directory as a normal directory, what we expect when building the wheel

[lucasiscovici2]

@neersighted This Pr only five the possibility to include the directory (which is a symlink), but without following it and adding only its content, so there is no symlink in the wheel. For an example, you can look at the test I added. (https://github.com/lucasiscovici2/poetry-core/tree/d628672f28dd8902c095d82548a834bb19868cff/tests/masonry/builders/fixtures/with-include-symbolic) (https://github.com/lucasiscovici2/poetry-core/blob/d628672f28dd8902c095d82548a834bb19868cff/tests/masonry/builders/fixtures/with-include-symbolic/pyproject.toml#L35)

[lucasiscovici2]

@neersighted i think you can re-open this PR, it's usefull and it doesn't change the current behavior (when there is no symlink), but is more intuitive and expected when there is a symlink

[neersighted]

There is no need to ping three times in 10 minutes :laughing:

As a participant in this thread I will get a notification every time you comment.

There are three possible behaviors for symlinks:

1. Treat them transparently, and naively recurse into them (like a directory) and read files like they do not exist.

2. Treat them as opaque (identifying them with lstat(2)), and copy the symlink itself instead of recursing into it (incompatible with the wheel standard).

3. See through them (also identifying them with lstat(2)), resolving them to their absolute path and then recursing/manipulating that set of paths.

Poetry currently does behavior 3, and as I understand it, you want to do behavior 1. I originally thought you intended to do behavior 2.

Can you further explain what workflow this enables? sdists need to be able to produce bdists -- if this is intended to enable out-of-tree symlinks to be built into wheels, I see that as counterproductive. If you need assets that are out of your tree, they should be a dependency or incorporated in tree.

Even if we accept it, this will be a breaking change, so I'm not 100% sure about including it before the Poetry 1.3 release at the very least.

[lucasiscovici2]

Haha sorry, I like to send multiple comments.

Yes it's 1.

So the data and src directory is usually at root level. When we have to add the data to the package. There are some difficulties. If we include the directory data it will be added in site-packages (this is not what we want). I found another solution. It is to create a symbolic link of data into the package (in src). For the sdist we could keep the symbolic link and for the bdist (wheel) we could treat them transparently (as you explained (1.)

[neersighted]

Right, but the sdist will only be buildable on your system if it references paths that are not in the tree. Is the intention that sdists also should be transparent, so that the files are captured in the sdist as well as the bdist?

What use case do you have where depending on out-of-tree files is necessary and can't be solved by an in-tree method?

[lucasiscovici2]

The symlink is in the tree but not inside the package. it's as package data.

[neersighted]

I see -- so you want to include data files that are not in the package, but symlinked from elsewhere in the repo -- include = [] will not work for you as that only affects the sdist.

This makes more sense now -- I still would like to get some more eyeballs (cc @finswimmer @radoering) on it to decide if it makes sense.

Would you mind examining some other common tools (e.g. hatch, setuptools, pipenv) and summarizing their existing behavior so we can compare to the rest of the ecosystem?

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication