The "PHPCS" linter gets out of memory error on large files
Closed, WontfixPublic
Actions

Assigned To

Authored By

	aik099
	Aug 5 2015, 11:01 AM

Description

When analyzing complex files the phpcs can eat lots of memory and this ends up in following arc lint output (when .arclint is using phpcs linter):

Some linters failed:
    - CommandException: Command failed with error #255!
      COMMAND
      'phpcs' '--report=xml' '--standard=vendor/aik099/coding-standard/CodingStandard' '/path/to/analyzed/file.php'
      
      STDOUT
      (empty)
      
      STDERR
      PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 72 bytes) in /home/user/.composer/vendor/squizlabs/php_codesniffer/CodeSniffer/Standards/Generic/Sniffs/WhiteSpace/ScopeIndentSniff.php on line 335
      PHP Stack trace:
      PHP   1. {main}() /home/user/.composer/vendor/squizlabs/php_codesniffer/scripts/phpcs:0
      PHP   2. PHP_CodeSniffer_CLI->runphpcs() /home/user/.composer/vendor/squizlabs/php_codesniffer/scripts/phpcs:25
      PHP   3. PHP_CodeSniffer_CLI->process($values = *uninitialized*) /home/user/.composer/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php:95
      PHP   4. PHP_CodeSniffer->processFiles($files = array (0 => '/home/user/web/p/rpierp/modules/custom/units/sections/e_order_eh.php'), $local = FALSE) /home/user/.composer/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php:855
      PHP   5. PHP_CodeSniffer->processFile($file = '/home/user/web/p/rpierp/modules/custom/units/sections/e_order_eh.php', $contents = N... (9,915 more bytes) ...
(Run with `--trace` for a full exception trace.)

To solve this I propose to lift memory limit, when running PHP_CodeSniffer through using -d option, that it supports.

The ArcanistPhpcsLinter::getMandatoryFlags method before:

protected function getMandatoryFlags() {
  $options = array('--report=xml');

  if ($this->standard) {
    $options[] = '--standard='.$this->standard;
  }

  return $options;
}

The ArcanistPhpcsLinter::getMandatoryFlags method after:

protected function getMandatoryFlags() {
  $options = array('--report=xml', '-d', 'memory_limit=-1');

  if ($this->standard) {
    $options[] = '--standard='.$this->standard;
  }

  return $options;
}

Event Timeline

aik099 created this task.Aug 5 2015, 11:01 AM

aik099 raised the priority of this task from to Needs Triage.

aik099 updated the task description. (Show Details)

aik099 added a project: Lint.

aik099 added a subscriber: aik099.

joshuaspence added a subscriber: joshuaspence.Aug 5 2015, 11:11 AM

Why do you have a memory limit configured on the CLI? Do you legitimately want scripts to exit after consuming 128MB of RAM, or is this just a default you haven't changed?

It's default configuration on CentOS to use same php.ini (which defines memory limit) for both FCGI PHP module (for Nginx), Mod_Php for Apache and CLI. Actually it's 1st time I hear, that PHP in CLI should have no memory limit by default. It doesn't have time limit, because it's not web request, but it surely always had memory limits.

I think this isn't our responsibility.

You've configured a memory limit (or deployed a configuration with a default memory limit), and PHPCS chooses to respect that memory limit. This seems to be working as intended by CentOS and PHPCS.

Raise the memory limit, or file an issue against CentOS or PHPCS if you think this isn't a good default behavior.

Since php.ini settings are shared across all PHP scripts on computer there is no way to increase per-script memory limit other than changing script invocation command line to something like php -d memory_limit=-1 actual_script.

Because of

PHP_CodeSniffer invocation command is hardcoded in linter code
PHP_CodeSniffer doesn't have any system-wide configuration setting for increasing memory limit

there is way to make PHP_CodeSniffer work with large files.

That's why I chose to increasing memory limit in linter code.

I can't remember off the top of my head, but I thought we had a way to pass flags to external linters. Something like this:

.arclint

{
  "linters": {
    "phpcs": {
      "type": "phpcs",
      "flags": ["-d memory_limit=-1"]
    }
  }
}

Yes, this did the trick:

{
	"linters": {
		"checkstyle": {
			"type": "phpcs",
			"flags": [
				"-d",
				"memory_limit=-1"
			]
		}
	}
}

Much appreciated @joshuaspence.

Do you know if this flag passing feature is recently added one (e.g. in last year) or it was there even in times, when linter settings were read from .arcconfig instead of .arclint?

Pretty sure this was only possible after the introduction of .arclint.

The "PHPCS" linter gets out of memory error on large filesClosed, WontfixPublicActions

Description

Event Timeline

The "PHPCS" linter gets out of memory error on large files
Closed, WontfixPublic
Actions