Warn strongly if the rsync file syncer is not in use

Just implemented status report entry:

Recommendation met

Recommendation not met but possible to meet

Recommendation not met and impossible to meet

Unassigning since I haven't had time for this and won't soon.

Note that I believe that we could install rsync on DrupalCI. See https://www.drupal.org/drupalorg/docs/drupal-ci/customizing-drupalci-tes....

But we should also just request them to install it — seems like that'd not be a big deal. Tagging for that — the follow-up is necessary not here but in the DrupalCI issue queue!

So rsync does integrity checks after or during the copy?

Quoting man rsync:

       -c, --checksum
              This forces the sender to checksum every regular file using a 128-bit MD4 checksum.  It does this during the initial file-
              system scan as it builds the list of all available files. The receiver then checksums its version of each file (if it exists
              and it has the same size as its sender-side counterpart) in order to decide which files need to be updated: files with either
              a changed size or a changed checksum are selected for transfer.  Since this whole-file checksumming of all files on both sides
              of the connection occurs in addition to the automatic checksum verifications that occur during a file's transfer, this option
              can be quite slow.

              Note that rsync always verifies that each transferred file was correctly reconstructed on the receiving side by checking its
              whole-file checksum, but that automatic after-the-transfer verification has nothing to do with this option's before-the-
              transfer "Does this file need to be updated?" check.

Note that \PhpTuf\ComposerStager\Infrastructure\Service\FileSyncer\RsyncFileSyncer does not set --checksum or -c. But it's that second paragraph that explains why that's simply unnecessary: rsync always checksums when transferring data.

If rsync fails in integrity checks are we thinking that logic is \Drupal\package_manager\StageBase::apply would catch this and throw a ApplyFailedException error where the php syncer would just not check?

Yes:

        try {
            $this->rsync->run($command, $callback);
        } catch (ExceptionInterface $e) {
            throw new IOException($e->getMessage(), 0, $e);
        }

would throw and

    try {
      $this->committer->commit($stage_dir, $active_dir, $paths_to_exclude, NULL, $timeout);
    }
    catch (InvalidArgumentException | PreconditionException $e) {
…
    }
    catch (\Throwable $throwable) {
      // The commit operation may have failed midway through, and the site code
      // is in an indeterminate state. Release the flag which says we're still
      // applying, because in this situation, the site owner should probably
      // restore everything from a backup.
      $this->setNotApplying()();
      throw new ApplyFailedException($this, (string) $this->getFailureMarkerMessage(), $throwable->getCode(), $throwable);
    }

would catch it and would contain the stderr output (starting with \Symfony\Component\Process\Exception\ProcessFailedException).

#6:

Note that I believe that we could install rsync on DrupalCI. See https://www.drupal.org/drupalorg/docs/drupal-ci/customizing-drupalci-tes....

Done: #3362143: Use the rsync file syncer by default.

Discussed with @omkar.podey:

1. First, install rsync on DrupalCI (commit #3362143-10: Use the rsync file syncer by default as a commit in this MR).
2. This should make tests pass. Or should allow you to make them pass! 😄
3. Once tests are green, implement the final step: the second step of the issue summary's proposed resolution, since so far it only implements the first step.

Because the proposed resolution in the issue summary does already say this:

1. Status report entry that strongly recommends the use of rsync and warns the user if that's not being used.
2. Detect presence of rsync during installation and then automatically switch to rsync instead of php file syncer (which is the sensible default: it always works).

@tedbow tagged this Needs issue summary update in #7. But that was specifically for ensuring that our recommendation actually works: i.e. that tests can pass when using rsync.

That is exactly what #3362143: Use the rsync file syncer by default is for (which was opened after #7). So marking this postponed on that issue, but we can already get this issue 100% ready, so it can land immediately after we sort out the remaining problems in #3362143 👍

I helped @omkar.podey:

• to figure out a way forward here, because it is physically impossible for a validator to show a validation result that depends on the TERMINATE event (because it happens after the status report page has rendered!)
• recover the functional test coverage from #3362978: Module does not work with Automated Cron

Looks superb now!

Just one question. One line in hook_help() that I think is obsolete.

Why was this committed without actually answering my question? I don't understand why a "I'm not sure" is followed by a commit?

I'm not sure either, to be honest, but we do account for the possibility (during status check) that they might be using the PHP file syncer, somehow. If they are, it seemed unfair to just say "don't use that!" without explaining how to change it back.

So I still think that this is essentially noise in hook_help():

      $output .= '<p>' . t('If <code>rsync is available but Package Manager is configured to use the built-in PHP file syncer, you should also add the following to settings.php:') . '';
      $output .= "
\$config['package_manager.settings']['file_syncer'] = 'rsync';
";

But … since this is "just" hook_help() which will change anyway when going into core … not reopening this.