Skip to content
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

Fix issue 21234: Import expression can read files outside of -J path in case of symlink/hardlink #11836

Merged
merged 2 commits into from
Oct 14, 2020
Merged

Conversation

andrey-zherikov
Copy link
Contributor

This change make import expression (import("...")) to work on Posix the same way as it works on Windows.

The original code tries to resolve symlinks on Posix and rejects a file if its real path is outside of -J options (although hardlinks pointing outside of -J are allowed).

This approach has the following issues:

  • It doesn't allow the use case where -J directory has symlinks to cherry-picked files from other places (hardlinks and/or copying of files are not always possible).
  • Different behavior on Posix vs. Windows.

From the other side, providing -J option to compiler means explicit consent to use the content of -J directory regardless of the type of this content (regular file/symlink/pipe etc).

See https://issues.dlang.org/show_bug.cgi?id=21234 for more details.

Copy link
Contributor

@MoonlightSentinel MoonlightSentinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test case

@dlang-bot
Copy link
Contributor

dlang-bot commented Oct 9, 2020

Thanks for your pull request and interest in making D better, @andrey-zherikov! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
21234 normal Import expression can read files outside of -J path in case of symlink/hardlink

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11836"

@andrey-zherikov
Copy link
Contributor Author

Please add a test case

Done.

@andrey-zherikov
Copy link
Contributor Author

Sorry, rebase didn't work the way I expected so I re-applied my changes on top of master and force-pushed them.

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

Successfully merging this pull request may close these issues.

4 participants