The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- Status changed to Needs review
over 1 year ago 9:44am 29 March 2023 - 🇮🇳India arunkumark Coimbatore
Rerolled the patch for Drupal 10.1.x version to resolve the issue.
Needs testing.
- 🇮🇳India arunkumark Coimbatore
Updated the patch to result minor validation fixes.
- 🇧🇪Belgium BramDriesen Belgium 🇧🇪
@arunkumark Thanks for your re-rolls. But can you please provide an interdiff → next time? It helps to see what has changed and also to spot potential bad re-rolls :-).
- Status changed to Needs work
over 1 year ago 6:29pm 29 March 2023 - 🇧🇪Belgium BramDriesen Belgium 🇧🇪
Also needs work since the custom commands failed. Please check the test run as those are phpcs issues.
- Status changed to Needs review
over 1 year ago 5:37am 30 March 2023 - 🇮🇳India arunkumark Coimbatore
Oh :( I uploaded the wrong patch file. Now attached is the valid patch file or PHPCS issue fixes.
@BramDriesen thanks for the review and feedbacks.
- Status changed to Needs work
over 1 year ago 2:22pm 30 March 2023 - 🇺🇸United States smustgrave
@arunkumark BramDriesen was asking for an interdiff which wasn't provided.
Also per new policy it needs to be documented why the original patch needed a reroll.
As a bug this will need a test case
Also this will need an issue summary update to document proposed solution.
- 🇧🇪Belgium BramDriesen Belgium 🇧🇪
+++ b/core/lib/Drupal/Core/FileTransfer/SSH.php @@ -55,7 +55,11 @@ protected function copyFileJailed($source, $destination) { + $stream = @ssh2_exec($this->connection, 'cp -Rp ' . escapeshellarg($source) . ' ' . escapeshellarg($destination) . '; echo $?'); + stream_set_blocking($stream, TRUE); + $stream_out = ssh2_fetch_stream($stream, SSH2_STREAM_STDIO); + $status = stream_get_contents($stream_out); + if ($status != 0) {
I have the feeling we are casting too many variables as well. This can be converted into 2 lines of code. Also not sure if the inheritDoc is up to date for this change. This could be cleaned and simplified.
Also the != 0 check can simply be removed I think.
- Status changed to Needs review
over 1 year ago 4:54am 31 March 2023 - 🇮🇳India arunkumark Coimbatore
As per comment #18 a minor correction was made on the
if
statement.Attached interdiff and updated the issue summary.
- Status changed to Needs work
over 1 year ago 7:05pm 31 March 2023 - 🇺🇸United States smustgrave
Was tagged for tests which still need to happen.
Not ready for review yet.