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

Add a caveat to std::os::windows::fs::symlink_file #91049

Merged
merged 1 commit into from
Nov 29, 2021
Merged

Add a caveat to std::os::windows::fs::symlink_file #91049

merged 1 commit into from
Nov 29, 2021

Conversation

dimo414
Copy link
Contributor

@dimo414 dimo414 commented Nov 19, 2021

This is similar to the note on Python's os.symlink(). Some additional notes in dimo414/bkt#3.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 19, 2021
@rust-log-analyzer

This comment has been minimized.

@ChrisDenton
Copy link
Member

This looks reasonable to me and should probably be added to symlink_dir too.

I do have one quibble on the advice about developer mode, etc. I'm not sure if this should be in the Rust docs because such things are subject to change outside of Rust. For example, using symlinks in developer mode was introduced in Windows 10 1703 and it's possible this could be changed again in the future (e.g. a separate toggle or by making it the default).

This is similar to the note on [Python's `os.symlink()`](https://docs.python.org/3/library/os.html#os.symlink). Some additional notes in dimo414/bkt#3.
@dimo414
Copy link
Contributor Author

dimo414 commented Nov 20, 2021

Thanks, I've mirrored the text to symlink_dir.

I definitely appreciate not wanting to tightly-couple these docs to Windows' behavior, but this failure mode is really confusing and surprising so I think these are helpful breadcrumbs. I attempted to make it clear these are just suggestions ("Users can try") and not necessarily what will work or what should be done. Open to wordsmithing though :)

@apiraino apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Nov 22, 2021
@kennytm
Copy link
Member

kennytm commented Nov 29, 2021

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 29, 2021

📌 Commit 9c3b0d8 has been approved by kennytm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 29, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2021
…askrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#91049 (Add a caveat to std::os::windows::fs::symlink_file)
 - rust-lang#91281 (Add demonstration test for rust-lang#91161)
 - rust-lang#91327 (Delete an unreachable codepath from format_args implementation)
 - rust-lang#91336 (Remove unused root_parent.)
 - rust-lang#91349 (Accumulate all values of `-C remark` option)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 80277dc into rust-lang:master Nov 29, 2021
@rustbot rustbot added this to the 1.59.0 milestone Nov 29, 2021
@dimo414 dimo414 deleted the patch-1 branch November 29, 2021 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants