Skip to content

Raise when symlinks use different drive in Windows - #6201

Merged
memsharded merged 2 commits into
conan-io:developfrom
memsharded:feature/symlinks_win_relative_error
Jan 14, 2020
Merged

memsharded merged 2 commits into
conan-io:developfrom
memsharded:feature/symlinks_win_relative_error

Conversation

@memsharded

@memsharded memsharded commented Dec 9, 2019

Copy link
Copy Markdown
Member

Changelog: Fix: Raise error for symlinks in Windows that point to a different unit.
Docs: Omit

Close #6197

Note this is almost impossible to test: it requires Administrator permissions to create a unit X: and then mklink to it to reproduce

@guban guban left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To see whether swallowing the error is appropriate, consider two scenarios:

  1. Scenario A: The user intentionally wants to export a symlink.
  2. Scenario B: The user does not export the symlink.

Also, keep in mind that the symlink is not necessarily committed to the version control: it could be made just for a local use and added to .gitignore.

In scenario A, we should fail, telling the user that exporting the symlink is a bad idea and is not supported for the user's own good. Swallowing the error in this scenario is not good.

In scenario B, we can swallow the error because the symlink is not exported, so it won't cause any problems for conan export command. In this scenario, though, we shouldn't even touch that symlink: the processing should be applied only to the files that are actually exported.

Considering that

  1. both scenarios are unusual and rare, and
  2. implementing scenario B is probably quite expensive,

a good solution is to always fail with an error (i.e., treating all scenarios as scenario A), and print the symlink path as part of error message, so that users in scenario B could easily find the culprit.

@lasote

lasote commented Jan 2, 2020

Copy link
Copy Markdown
Contributor

a good solution is to always fail with an error (i.e., treating all scenarios as scenario A), and print the symlink path as part of error message, so that users in scenario B could easily find the culprit.

+1

@memsharded

Copy link
Copy Markdown
Member Author

Understood, made it fails always with an informative message.

@memsharded memsharded changed the title skip different drive errors Raise when symlinks use different drive in Windows Jan 14, 2020
@memsharded
memsharded requested a review from jgsogo January 14, 2020 16:13

@jgsogo jgsogo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Easy solution. Totally agree this is the best we can do 👍

@memsharded
memsharded merged commit 4a69273 into conan-io:develop Jan 14, 2020
@memsharded
memsharded deleted the feature/symlinks_win_relative_error branch January 14, 2020 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"conan export" fails with short_paths and exports when recipe and conan are on different drives

4 participants