CLI-1171: acli throws error in Windows terminal

[Jura]

Describe the bug
Incorrect condition at

cli/src/Helpers/LocalMachineHelper.php

Line 226 in 23d3e32

if (getenv('MSYSTEM') !== NULL) {

getenv() returns null only if the respective variable is present and contains NULL as a value: https://www.php.net/manual/en/function.getenv.php . If variable doesn't exist getenv() returns FALSE instead. In that case condition currently evaluates as TRUE and substr() on the next line throws error when called as substr(FALSE, 0, 4)

Proposed solution
Update business logic to add additional condition to line#226:

if (getenv('MSYSTEM') !== NULL && getenv('MSYSTEM') !== false) {

or just

if (getenv('MSYSTEM'))

Screenshots
Output including error stack trace:

acli list -vvv

Box Requirements Checker
========================

> Using PHP 8.2.11
> PHP is using the following php.ini file:
  D:\dev\programmes\php8\php.ini

> Checking Box requirements:
  ✔ The application requires a version matching "^8.0".
  ✔ The application requires the extension "json".
  ✔ The package "guzzlehttp/guzzle" requires the extension "json".
  ✔ The package "league/csv" requires the extension "json".
  ✔ The package "m4tthumphrey/php-gitlab-api" requires the extension "json".
  ✔ The package "ramsey/uuid" requires the extension "json".
  ✔ The package "zumba/amplitude-php" requires the extension "json".
  ✔ The package "composer/ca-bundle" requires the extension "openssl".
  ✔ The package "composer/ca-bundle" requires the extension "pcre".
  ✔ The package "vlucas/phpdotenv" requires the extension "pcre".
  ✔ The package "m4tthumphrey/php-gitlab-api" requires the extension "xml".
  ✔ The package "marc-mabe/php-enum" requires the extension "reflection".
  ✔ The package "zumba/amplitude-php" requires the extension "curl".
  ✔ The package "laminas/laminas-servicemanager" conflicts with the extension "psr".
  ✔ The package "symfony/dependency-injection" conflicts with the extension "psr".
  ✔ The package "symfony/service-contracts" conflicts with the extension "psr".


 [OK] Your system is ready to run the application.


PHP Fatal error:  Uncaught TypeError: substr(): Argument #1 ($string) must be of type string, bool given in phar://D:/dev/programmes/cli/acli.phar/src/Helpers/LocalMachineHelper.php:227
Stack trace:
#0 phar://D:/dev/programmes/cli/acli.phar/src/Helpers/LocalMachineHelper.php(227): substr(false, 0, 4)
#1 phar://D:/dev/programmes/cli/acli.phar/bin/acli(60): Acquia\Cli\Helpers\LocalMachineHelper::getHomeDir()
#2 D:\dev\programmes\cli\acli.phar(14): require('phar://D:/dev/p...')
#3 {main}
  thrown in phar://D:/dev/programmes/cli/acli.phar/src/Helpers/LocalMachineHelper.php on line 227

Fatal error: Uncaught TypeError: substr(): Argument #1 ($string) must be of type string, bool given in phar://D:/dev/programmes/cli/acli.phar/src/Helpers/LocalMachineHelper.php:227
Stack trace:
#0 phar://D:/dev/programmes/cli/acli.phar/src/Helpers/LocalMachineHelper.php(227): substr(false, 0, 4)
#1 phar://D:/dev/programmes/cli/acli.phar/bin/acli(60): Acquia\Cli\Helpers\LocalMachineHelper::getHomeDir()
#2 D:\dev\programmes\cli\acli.phar(14): require('phar://D:/dev/p...')
#3 {main}
  thrown in phar://D:/dev/programmes/cli/acli.phar/src/Helpers/LocalMachineHelper.php on line 227

Desktop (please complete the following information):

• OS: Windows 11

[anavarre]

Could you please confirm you're on the latest version of Acquia CLI (should be 2.16.0)?

acli --version

[Jura]

I'm getting the same error on every acli run, including acli --version

But I can attest that I downloaded the most recent version (https://github.com/acquia/cli/releases/tag/2.16.0) just before I filed the bug report

[anavarre]

Thank you, that's helpful. @danepowell for awareness. Looks like #1605 doesn't directly help.

[danepowell]

Thanks, this is unrelated to #1605 despite being the same environment and error class.

[danepowell]

I cannot reproduce this in Windows 11 stock versions of CMD and PowerShell because MSYSTEM is not set. How is that variable set in your environment? Are you using msys2 perhaps?

[Jura]

@danepowell , I don't have it set either, that env variable simply does not exist. Hence getenv('MSYSTEM') call returns false and condition if (getenv('MSYSTEM') !== NULL) { effectively evaluates as true as it is not null but just a bool value, passing that false value to $system = strtoupper(substr(getenv('MSYSTEM'), 0, 4)); on the next line, which throws error.

[danepowell]

Right, I can reproduce this now on vanilla CMD. I got my wires crossed, sorry for the noise!

[danepowell]

PR looks good, just needs tests (see my comment there). I also opened an upstream report, since we borrowed this code from Terminus long ago: pantheon-systems/terminus#2509