Skip to content

Conversation

morozov
Copy link
Contributor

@morozov morozov commented Dec 17, 2019

The full new feature is implemented in the first commit and then partially reverted to the deprecation mode in the second one.

Migration path:

  1. Once upgraded, if a user uses PHPBrew with an old PHP version, PHPBrew will display a warning suggesting to set the system interpreter. In the deprecation mode, the use and switch commands will not have the intended PHP version constraints while system and system-off will. This will motivate the users to configure their environment as expected.
  2. In the next minor release, we'll revert the deprecation layer and drop the support for the older PHP version. At this point, the users will be expected to have their environment configured.

TODO:

  • Document the refactoring in this PR.
    1. A new environment variable PHPBREW_SYSTEM_PHP has been introduced. If set, it must contain the full path to the system PHP interpreter, e.g. /usr/bin/php.
    2. __phpbrew_php_exec() is now the key entry point from the shell PHPBrew wrapper to the PHP implementation. It uses the system PHP interpreter if it's set or falls back to the current one. Also it tries to use bin/phpbrew if we're currently in the PHPBrew source directory.
    3. The env subcommand implementations have been removed from the shell whappers since the subcommand can be properly handled by a PHP implementation.
  • Document the feature on Wiki.
  • Split into deprecation and subsequent enforcement of the runtime version.
  • Request feedback from maintainers and active contributors.

Behavior tested manually on Fish and Bash:

Feature: System PHP interpreter

  Scenario: If the system interpreter is not set, the command displays nothing
    Given the system interpreter is not set
    When I check the system interpreter
    Then the output should have been empty

  Scenario: If the system interpreter is set, the command displays the interpreter path
    Given the system interpreter is set to:
      | build  | path         |
      | system | /usr/bin/php |
    When I check system interpreter
    Then the output should have been "/usr/bin/php"

  Scenario: The system interpreter can be set using the interpreter path
    Given the system interpreter is not set
    When I set the system interpreter using "path" to:
      | version | path         |
      | system  | /usr/bin/php |
    And I check the system interpreter
    Then the output should have been "/usr/bin/php"

  Scenario: The system interpreter can be set using the interpreter version
    Given the system interpreter is not set
    When I set the system interpreter using "version" to:
      | version | path                             |
      | 7.4.0   | ~/.phpbrew/php/php-7.4.0/bin/php |
    And I check the system interpreter
    Then the output should have been "~/.phpbrew/php/php-7.4.0/bin/php"

  Scenario: The system interpreter can be set using the interpreter build
    Given the system interpreter is not set
    When I set the system interpreter using "version" to:
      | build     | path                             |
      | php-7.4.0 | ~/.phpbrew/php/php-7.4.0/bin/php |
    And I check the system interpreter
    Then the output should have been "~/.phpbrew/php/php-7.4.0/bin/php"

  Scenario: The system interpreter cannot be set to an unsupported PHP version
    Given the system interpreter is not set
    When I set the system interpreter using "path" to:
      | path                             | PHP_VERSION_ID |
      | ~/.phpbrew/php/php-7.1.0/bin/php | 70100          |
    Then an error should have been displayed
    And the command should have exited with a non-zero exit code
    And the system interpreter should not have been set

  Scenario: The system interpreter cannot be set to a non-existing path
    Given the system interpreter is not set
    When I set the system interpreter using "path" to:
      | path          |
      | /path/to/file |
    Then an error should have been displayed
    And the command should have exited with a non-zero exit code
    And the system interpreter should not have been set

  Scenario: The system interpreter cannot be set to a directory path
    Given the system interpreter is not set
    When I set the system interpreter using "path" to:
      | path |
      | /    |
    Then an error should have been displayed
    And the command should have exited with a non-zero exit code
    And the system interpreter should not have been set

  Scenario: The system interpreter cannot be set to a non-executable file path
    Given the system interpreter is not set
    When I set the system interpreter using "path" to:
      | path       |
      | /etc/fstab |
    Then an error should have been displayed
    And the command should have exited with a non-zero exit code
    And the system interpreter should not have been set

  Scenario: The system interpreter cannot be set to a non-PHP executable
    Given the system interpreter is not set
    When I set the system interpreter using "path" to:
      | path    |
      | /bin/ls |
    Then an error should have been displayed
    And the command should have exited with a non-zero exit code
    And the system interpreter should not have been set

  Scenario: The system interpreter version cannot be purged
    Given the system system interpreter is set to:
      | version | path                             |
      | 7.4.0   | ~/.phpbrew/php/php-7.4.0/bin/php |
    When I purge the PHP version "7.4.0"
    Then an error should have been displayed
    And the command should have exited with a non-zero exit code
    And the system interpreter should have been remained unchanged

  Scenario: An unsupported PHP version cannot be used without a system interpreter
    Given the system interpreter is not set
    When I use the PHP version "7.1.0"
    Then an error should have been displayed
    And the command should have exited with a non-zero exit code
    And the current PHP version should have remained unchanged

  Scenario: The system interpreter cannot be switched off when an unsupported PHP version is used
    Given the system system interpreter is set to:
      | version | path                             |
      | 7.4.0   | ~/.phpbrew/php/php-7.4.0/bin/php |
    When I switch off the system interpreter
    Then an error should have been displayed
    And the command should have exited with a non-zero exit code
    And the system interpreter should have been remained unchanged```

Closes #872.

@morozov
Copy link
Contributor Author

morozov commented Dec 18, 2019

@theofidry, @markwu please take a look if you're interested.

@jhdxr
Copy link
Member

jhdxr commented Dec 19, 2019

I'm all for this idea, and the implementation looks good to me.

the only suggestion is we might need to increase the major version since this is a huge change since we are requiring a up-to-date PHP now.

@morozov
Copy link
Contributor Author

morozov commented Dec 19, 2019

@jhdxr in its current state, it only emits a deprecation warning, so it's safe to merge it as is. I'm still on the fence about tagging 2.0.0 just because of this change. I want to make some more internal changes like:

  1. Replace CLIFramework and Logger with Symfony Console.
  2. Replace CurlKit with Guzzle (+ some caching middleware).
  3. Replace system, proc_open and CommandBuilder with Symfony Process consistently throughout the codebase.
  4. Replace the hard-coded package version with Ocramius/PackageVersions.

This is what necessitates dropping the support for old PHP versions and this is what I'd gladly call 2.0.0. We can implement all (or at least most of) these changes at once and only then actually bump the PHP requirement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PHPBREW_SYSTEM_PHP variable to run the system PHP for phpbrew
3 participants