Skip to content

Implemented the usage of the system PHP interpreter#1098

Merged
morozov merged 2 commits into
phpbrew:masterfrom
morozov:issues/872
Dec 19, 2019
Merged

Implemented the usage of the system PHP interpreter#1098
morozov merged 2 commits into
phpbrew:masterfrom
morozov:issues/872

Conversation

@morozov
Copy link
Copy Markdown
Contributor

@morozov morozov commented Dec 17, 2019 •
edited
Loading

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 morozov added this to the Release 1.25.0 milestone Dec 17, 2019
@morozov morozov self-assigned this Dec 17, 2019
@morozov morozov requested review from c9s and jhdxr December 18, 2019 01:37
@morozov
Copy link
Copy Markdown
Contributor Author

morozov commented Dec 18, 2019

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

theofidry
theofidry approved these changes Dec 18, 2019
View reviewed changes
Comment thread completion/bash/_phpbrew
Comment thread completion/bash/_phpbrew
Comment thread completion/bash/_phpbrew
Comment thread completion/bash/_phpbrew
@jhdxr
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

morozov commented Dec 19, 2019 •
edited
Loading

@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.

@morozov morozov merged commit 808b116 into phpbrew:master Dec 19, 2019
@morozov morozov deleted the issues/872 branch December 19, 2019 16:02
@morozov morozov mentioned this pull request Jan 6, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhdxr jhdxr jhdxr approved these changes

@c9s c9s Awaiting requested review from c9s

+1 more reviewer

@theofidry theofidry theofidry approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@morozov morozov

Labels

Feature Important Improvement Refactoring RFC System Interpreter

Projects

None yet

Milestone

Release 1.25.0

Development

Successfully merging this pull request may close these issues.

Add PHPBREW_SYSTEM_PHP variable to run the system PHP for phpbrew

3 participants

@morozov @jhdxr @theofidry