Solr_Reindex build task doesn't work in dev mode in 3.9.0

[GuySartorelli]

Solr_Reindex build task doesn't work in dev mode in 3.9.0

[michalkleiner]

Thanks for looking into it further, @GuySartorelli. I'm under the impression you did what you could and I would say that's more than enough here. You're doing great work lately, keep it up!

[GuySartorelli]

In SolrReindexImmediateHandler.php line 116, if I replace $process->run(); with $process->mustRun(); exposes the following error:

The command "'php /home/webspace/ss4-scratch/site/vendor/silverstripe/framework/cli-script.php dev/tasks/Solr_Reindex' 'index=App\\Search\\SolrIndex' 'class=SilverStripe\\CMS\\Model\\SiteTree' 'group=0' 'groups=7' 'variantstate='''{"SilverStripe\\FullTextSearch\\Search\\Variants\\SearchVariantVersioned":"Live"}'''' 'verbose=1'" failed. Exit Code: 127(Command not found) Working directory: /home/webspace/ss4-scratch/site/public Output: ================ Error Output: ================ sh: 1: exec: php /home/webspace/ss4-scratch/site/vendor/silverstripe/framework/cli-script.php dev/tasks/Solr_Reindex: not found

I think this is caused by retaining this line:

silverstripe-fulltextsearch/src/Solr/Reindex/Handlers/SolrReindexImmediateHandler.php

Line 96 in 355e928

$scriptTask = "{$php} {$scriptPath} dev/tasks/{$taskName}";

as a single string instead of including each component as a distinct value in the $cmd array

silverstripe-fulltextsearch/src/Solr/Reindex/Handlers/SolrReindexImmediateHandler.php

Lines 98 to 106 in 355e928

	$cmd = [
	$scriptTask,
	"index={$indexClassEscaped}",
	"class={$class}",
	"group={$group}",
	"groups={$groups}",
	"variantstate={$statevar}",
	"verbose=1"
	];

Making that change results in this error:

Class App\\Search\\SolrIndex does not exist in /home/webspace/ss4-scratch/site/vendor/silverstripe/framework/src/Core/Injector/InjectionCreator.php:17

And after not escaping the backslashes in the index class name we get:

SilverStripe\\CMS\\Model\\SiteTree is not a subclass of DataObject

Finally, when we don't escape the backslashes in that class name either, we get the reindex working correctly.

I'll raise a PR for this momentarily, but putting this sequence here in case anyone wonders why the PR removes the backslash escaping.

[michalkleiner]

Thanks for troubleshooting this!

Do you know/have you found what changed that this stopped working?

[GuySartorelli]

@michalkleiner In 3.9.0 a change was made to allow symfony/process ^4, and part of the change included changing the command from being passed as a string to being passed as an array (see #301). Passing the string worked fine (with process 3.x) but it seems (and this is just based on the error output I was seeing) like there is some step if the command is passed as an array to confirm that the first item in the array is a valid executable.

Edit: Okay so there's nothing done to it prior to being executed... I can't really make heads of exactly why it ends up failing the way it does (the full array is ultimately imploded and the resultant string, with some alterations, is passed to proc_open)...

So I can't say why it's failing the way it is, just that it is and it didn't used to when it was passed as a string - and that splitting out that first array element into three distinct elements resolves it.

The escaping does make sense though, as both process 3 and 4 use implode(' ', array_map([$this, 'escapeArgument'], $commandline)); when imploding the command array, so we had been unintentionally double escaping the class names.