Problem/Motivation
The following is no longer accurate
* This is a powerful function that in many ways performs like an advanced
* version of copy().
* - Checks if $source and $destination are valid and readable/writable.
* - If file already exists in $destination either the call will error out,
* replace the file or rename the file based on the $fileExists parameter.
* - If the $source and $destination are equal, the behavior depends on the
* $fileExists parameter.
* - Provides a fallback using realpaths if the move fails using stream
* wrappers. This can occur because PHP's copy() function does not properly
* support streams if open_basedir is enabled. See
* https://bugs.php.net/bug.php?id=60456
Proposed resolution
See changes in MR
Remaining tasks
Review
commit
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #49 | 2843100-nr-bot.txt | 1.28 KB | needs-review-queue-bot |
Issue fork drupal-2843100
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 2843100-fileunmanagedcopy-documentation-is
changes, plain diff MR !8587
Comments
Comment #2
aaronbaumanComment #3
aaronbaumanComment #4
aaronbaumanComment #6
Munavijayalakshmi commentedThere was error in patch. I've fixed it & added.
Comment #7
Munavijayalakshmi commentedComment #8
bander2 commentedLooks good.
Comment #9
gábor hojtsyThanks all for working on this but I think this still needs some improvement.
This looses "error out" which may mean it just logs an error, but that would still be useful to document (I guess keep documented even though it was/is misleadingly documented).
The new first item is not describing the behavior from the consumer/user side but from the function, which is in stark contrast to the item above and below.
Comment #10
gaurav.kapoor commentedModified as per suggestion in #9.
Comment #11
dinesh18 commentedLooks good to me. +1 to RTBC
Comment #18
tanubansal commentedTested #10 on 9.1. It appears fine to me as well
RTBC + 1
Comment #20
abhijith s commentedPatch can't be applied on 9.2.x dev .Showing error
Comment #21
aaronbaumanComment #22
aaronbaumanThis is N/A for D9, because it's deprecated as of 8.7
Version should not be updated past 8.9
#10 still applies.
Comment #25
larowlanComment #26
immaculatexavier commentedPatch for 9.3
Comment #27
larowlanfile_unmanaged_copy was removed from core in Drupal 9.0 (deprecated in 8.7.0).
The new docs are on \Drupal\Core\File\FileSystemInterface::copy and that's where the updating should occur.
Comment #28
immaculatexavier commentedRerolled the Patch against 9.3.x. Please review
Updated the docs in \Drupal\Core\File\FileSystemInterface::copy
Comment #29
immaculatexavier commentedComment #30
avpadernoIf I understand the coding standards for @see, the method name must include the fully qualified name of its class.
@seeneeds to be on its own line, which in this case means it should be placed after that list.Comment #31
yogeshmpawarAddressed #30 with an interdiff.
Comment #32
avpadernoThe method name must contain the class name, namespace included.
Comment #33
avpadernoComment #35
Arti Anil Pattewar commentedpatch #33 applied cleanly on D9.4.x.
Comment #37
aziza_a commentedpatch #33 applied cleanly on D9.5.x.
Comment #38
smustgrave commentedChange in #33 seems fine too me.
Comment #39
catchI don't think this is right.
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21File%21Fi... - this throws an exception if $source and $destination are the same.
Exception is also documented here: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21File%21Fi...
Also the issue summary is still referencing the procedural functions that were removed a long time ago.
Comment #43
quietone commentedUpdated issue summary and created an MR.
Comment #45
vladimirausAdded example and updated $fileExists parameters.
Comment #46
jannakha commentedthanks for the fix
Comment #47
quietone commentedOh, nice to see the quick response, thanks.
Left a comment on the MR that needs a response.
Comment #48
vladimirausThanks @quietone. Updated.
Comment #49
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #50
vladimirausComment #51
smustgrave commentedWould agree with removing
$file_system->prepareDirectory($directory, FileSystemInterface::CREATE_DIRECTORY);mainly because the sentence below the example mentions copying but nothing about needing to prepare the folder, which to my knowledge isn't always neededComment #52
vladimirausRemoved
$file_system->prepareDirectory($directory, FileSystemInterface::CREATE_DIRECTORY);from example.Comment #53
jannakha commentedDocs are updated as required.
RTBC.
Comment #54
quietone commentedI removed one blank line from the changes so this meets coding standards. And updated credit.
The changes are an improvement so if there is anything more let's put that in a followup. Leaving at RTBC.
Comment #57
nod_Thanks for sorting out the credits.
Committed d135a79 and pushed to 11.x. Thanks!