Page MenuHomePhabricator

git-changed-in-head breaks with paths with spaces in
Closed, DuplicatePublic

Description

https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/471885/ for T183759

	A	tests/phpunit/includes/shell/bin with spaces/echo_333333_stars.php				3	
	A	tests/phpunit/includes/shell/bin with spaces/echo_args.php				8	
	A	tests/phpunit/includes/shell/bin with spaces/echo_env.php				8	
	A	tests/phpunit/includes/shell/bin with spaces/echo_stdin.php				4	
	A	tests/phpunit/includes/shell/bin with spaces/failure_status.php				3	
	A	tests/phpunit/includes/shell/bin with spaces/stdout_stderr.php				7	
	A	tests/phpunit/includes/shell/bin with spaces/success_status.php

https://integration.wikimedia.org/ci/job/mwgate-php70lint/8271/console

Gives

00:53:23 php -l tests/phpunit/includes/shell/bin 
00:53:23 Could not open input file: tests/phpunit/includes/shell/bin
00:53:23 php -l with 
00:53:23 Could not open input file: with
00:53:23 php -l spaces/echo_333333_stars.php 
00:53:23 Could not open input file: spaces/echo_333333_stars.php

Details

Related Gerrit Patches:
integration/jenkins : masterQuote output of script

Event Timeline

Reedy created this task.Nov 6 2018, 10:49 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 6 2018, 10:49 AM

Change 471936 had a related patch set uploaded (by Reedy; owner: Reedy):
[integration/jenkins@master] Quote output of script

https://gerrit.wikimedia.org/r/471936

Reedy updated the task description. (Show Details)Nov 6 2018, 11:20 AM

In short: please do not introduce directories or files having spaces in MediaWiki. It causes issues to multiple people.

This task is a duplicate of the old T89380 the long story:

The output is intended to be one file per line. The proposed patch ( Gerrit #471936 ) would cause the file names to have double quotes. Given git commit altering a file named, foobar.php one would then get:

$ git-changed-in-head
"foobar.php"
$

When consumed by another process or a pipe, that will be literally "foobar.php", which would cause the file to NOT be found. The name is foobar.php (without double quotes) not "foobar.php" (extra double quotes).

A few other notes. This script is only used by the php linting jobs:

mediawiki-core-hhvmlint
mediawiki-core-php55lint
mediawiki-core-php70lint
mwgate-php55lint
mwgate-php56lint
mwgate-php70lint
operations-mw-config-php55lint
php55lint
php56lint
php70lint

They are triggered via the Zuul check pipeline (that is for untrusted users) which is legacy and has to be phased out ( T192217 ). It was merely an optimization for mediawiki/core which has so many php files.

Nowadays one can just run that locally thanks to the introduction of composer test. Specially for mediawiki/core , one must pass a list of files which will run php -l and codesniffer.

For the bug linked, there is an added benefit of having the test scripts on a path with spaces (running shell commands with executables/arguments located in a path that has spaces is a common issue in Windows). Although the linter may complain in this setup, some MediaWiki developers may want to run shell commands with spaces in them, and the tests must validate that such shell commands work as expected. Is there a way to mark files as not linted?

Change 471936 abandoned by Reedy:
Quote output of script

https://gerrit.wikimedia.org/r/471936