[Filesystem] Made sure Filesystem::dumpFile() overwrites an existing file #7820
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'd be for the third param ($overwrite = false) for rename if @fabpot accepts that. So we still get the check for success and it might be useful for others too. Thanks! |
|
@fabpot are you fine with adding an optional parameter to |
|
@jakzal adding an opt arg looks good to me. |
|
Ok. PR updated. |
fabpot
added a commit
that referenced
this pull request
Apr 24, 2013
This PR was merged into the master branch. Discussion ---------- [Filesystem] Made sure Filesystem::dumpFile() overwrites an existing file #7753 introduced a change in behaviour. Before, ConfigCache simply used the ``rename()`` function to save a file. It was replaced by ``Filesystem::dumpFile()`` which internally uses ``Filesystem::rename()``. The later checks if file already exists and throws an exception if it exists. This PR makes sure that ``Filesystem::dumpFile()`` removes the file before creating a new one. Alternatively, we could introduce a third parameter to the ``Filesystem::rename()`` which would allow to overwrite the target file. Before #7752: * [CompilerDebugDumpPass](https://github.com/symfony/symfony/blob/d100ffaf761b46acdb2440c04602378b87b0d610/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CompilerDebugDumpPass.php#L23) uses ConfigCache * [ConfigCache](https://github.com/symfony/symfony/blob/d100ffaf761b46acdb2440c04602378b87b0d610/src/Symfony/Component/Config/ConfigCache.php#L103) uses the [rename()](http://uk1.php.net/rename) function (which overwrites a file if it exists) After #7752: * [CompilerDebugDumpPass](https://github.com/symfony/symfony/blob/ad47bc47380188041b7889f40e380ad3766a0110/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CompilerDebugDumpPass.php#L23) uses the ``Filesystem::dumpFile()`` * [Filesystem::dumpFile()](https://github.com/webfactory/symfony/blob/ad47bc47380188041b7889f40e380ad3766a0110/src/Symfony/Component/Filesystem/Filesystem.php#L458) uses the [Filesystem::rename()](https://github.com/webfactory/symfony/blob/ad47bc47380188041b7889f40e380ad3766a0110/src/Symfony/Component/Filesystem/Filesystem.php#L239) (which throws an exception if file exists) | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #7816 | License | MIT | Doc PR | ~ Commits ------- 4f4ec76 [Filesystem] Made sure Filesystem::dumpFile() overwrites an existing file.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#7753 introduced a change in behaviour. Before, ConfigCache simply used the
rename()function to save a file. It was replaced byFilesystem::dumpFile()which internally usesFilesystem::rename(). The later checks if file already exists and throws an exception if it exists.This PR makes sure that
Filesystem::dumpFile()removes the file before creating a new one.Alternatively, we could introduce a third parameter to the
Filesystem::rename()which would allow to overwrite the target file.Before #7752:
After #7752:
Filesystem::dumpFile()