Skip to content
This repository was archived by the owner on Nov 27, 2020. It is now read-only.

[RFC] Add command in bin for clearing cache even if environment is broken #1105

Closed
wants to merge 1 commit into from

Conversation

curry684
Copy link

Follows the discussion at symfony/symfony#23592

This command allows one to run bin/clear-cache or bin/clear-cache <envName> in any situation. It's consciously implemented als Plain Ole PHP since it is most commonly used to fix a temporary broken environment, in which the vendor directory or its packages may also well be (temporarily) broken. So no use in depending on symfony/console or 'symfony/process`.

If we do this the clearCache command in SensioDistributionBundle will also need to be patched afterwards, and some documentation changed.

Based on 3.3 as it deprecated clear:cache without --no-warmup, no use in adding it to older versions.

@theofidry
Copy link

I think this can be done in the Makefile if not already done tbh

Copy link

@ostrolucky ostrolucky left a comment

Choose a reason for hiding this comment

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

This script is too stupid:

  • assumes composer.json is in same dir
  • doesn't handle paths overridden via AppKernel
  • doesn't return correct exit codes
  • uses system call instead of unlink
  • hardcoded path and newline separators
  • untestable
  • if system call fails, script outputs bogus success message

I suggest not doing anything until fortune of cache:clear is decided

@curry684
Copy link
Author

curry684 commented Jul 31, 2017

@ostrolucky most of those comments are intentional of course.

  • There is no assumption about the location of the composer.json. It is used if you have a "normal" install. Otherwise we can find nor use it.
  • The use case for the script is being able to fix the cache part of broken installs. No external dependencies, so no AppKernel loading as it depends on stuff in the cache and/or autoloader. If you have a "normal" install, it works. If you don't, it won't. But the same goes for bin/console and we keep that in there in the default install. If your environment is too broken to use this script you're stuck with using shell-fu anyway.
  • The use of system commands is intentional, as it provides much more reasonable output than recursive unlink will ever give (think read-only filesystems etc.). Admittedly that output is currently not being piped correctly, and you're right about the exit codes.
  • The path and newline separators do not matter. PHP is well documented since somewhere in the 4.x era to be agnostic to them. Using DIRECTORY_SEPARATOR in paths and PHP_EOL in console output has no added value therefore, they're only useful for example in parsing local paths and writing platform-specific file formats.
  • The bin/console script is just as (un)testable.

@Pierstoval
Copy link
Contributor

Adding such script just for one uncommon use-case is a total mess. There are workarounds for symfony/symfony#23592, and implementing this kind of logic rough and tough like that just lacks understanding of how Symfony works... 😕

@curry684
Copy link
Author

curry684 commented Aug 1, 2017

The workaround appears to be rm -rf according to symfony/symfony#23592 (comment). I know it's rough and tough, but it's a PR in Symfony Standard Edition, not Symfony core. It doesn't matter that you can break it by going power-user in Symfony, as that will break more scripts, including potentially bin/console itself.

Anyway, if the discussion is going to the level of implying I don't understand how Symfony works I'll just close and we'll keep advising people to rm -rf.

@curry684 curry684 closed this Aug 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants