Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send errors to stderr when sake is run via sushi #7

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

Mossman1215
Copy link
Contributor

Supporting this dash PR
https://github.com/silverstripeltd/dash/pull/248
given framework hasn't changed since this request
silverstripe/silverstripe-framework#7909
I have found a workaround by loading an INI file called deploy.ini via the environment var PHP_INI_SCAN_DIR

it avoids the flow of deployments failing and the error logs go to syslog rather than stderr

@Mossman1215 Mossman1215 changed the title dump errors to stderr when sake is run via sushi Send errors to stderr when sake is run via sushi Nov 19, 2020
@brettt89
Copy link
Contributor

A nicer way to do this might be to use cli-script.php instead of sake. That way you can control PHP variables directly via the CLI.

E.g. php -d display_errors=stderr framework/cli-script.php dev flush=1

You could potentially repeat / re-use the same logic that sake uses for identifying the location of this cli-script.php file.
https://github.com/silverstripe/silverstripe-framework/blob/a0d00f89510124a7beecee4131c440c0cc8ca8e9/sake#L19

But I also understand this is a bigger change as a whole. (E.g. how does this affect custom stack such as Census who are forging this file etc).

What are your thoughts?

@Mossman1215
Copy link
Contributor Author

Mossman1215 commented Nov 19, 2020

A nicer way to do this might be to use cli-script.php instead of sake. That way you can control PHP variables directly via the CLI.

E.g. php -d display_errors=stderr framework/cli-script.php dev flush=1

You could potentially repeat / re-use the same logic that sake uses for identifying the location of this cli-script.php file.
https://github.com/silverstripe/silverstripe-framework/blob/a0d00f89510124a7beecee4131c440c0cc8ca8e9/sake#L19

But I also understand this is a bigger change as a whole. (E.g. how does this affect custom stack such as Census who are forging this file etc).

What are your thoughts?

As platform we don't control cli-script.php that comes via the codebase.
So this workaround is to fix it in the platform while the OSS peeps work on their complete overhaul of console comands
image
silverstripe/silverstripe-framework#5542
Jake's already tried to change cli-script in 2018
silverstripe/silverstripe-framework#7909
which was closed.
So although this is a messy hack it's a workaround for our lack of coordination on the OSS side.

@brettt89
Copy link
Contributor

A nicer way to do this might be to use cli-script.php instead of sake. That way you can control PHP variables directly via the CLI.
E.g. php -d display_errors=stderr framework/cli-script.php dev flush=1
You could potentially repeat / re-use the same logic that sake uses for identifying the location of this cli-script.php file.
https://github.com/silverstripe/silverstripe-framework/blob/a0d00f89510124a7beecee4131c440c0cc8ca8e9/sake#L19
But I also understand this is a bigger change as a whole. (E.g. how does this affect custom stack such as Census who are forging this file etc).
What are your thoughts?

As platform we don't control cli-script.php that comes via the codebase.
So this workaround is to fix it in the platform while the OSS peeps work on their complete overhaul of console comands
image
silverstripe/silverstripe-framework#5542
Jake's already tried to change cli-script in 2018
silverstripe/silverstripe-framework#7909
which was closed.
So although this is a messy hack it's a workaround for our lack of coordination on the OSS side.

Oh, I was meaning change Sushi to use cli-script.php directly instead of sake.

@Mossman1215
Copy link
Contributor Author

As platform we don't control cli-script.php that comes via the codebase.
So this workaround is to fix it in the platform while the OSS peeps work on their complete overhaul of console comands
image
silverstripe/silverstripe-framework#5542
Jake's already tried to change cli-script in 2018
silverstripe/silverstripe-framework#7909
which was closed.
So although this is a messy hack it's a workaround for our lack of coordination on the OSS side.

Oh, I was meaning change Sushi to use cli-script.php directly instead of sake.

Right! I don't know what would be missed out if we did invoke cli-script directly and I never considered it.

@brettt89
Copy link
Contributor

Have approved. Feel free to merge when ready, or request another review if changes are needed :).

@Mossman1215 Mossman1215 merged commit 82a5d4a into master Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants