Skip to content
Snippets Groups Projects

Resolve #3503699 "Make package manager"

Closed Adam G-H requested to merge issue/drupal-3503699:3503699-make-package-manager into 11.x

Closes #3503699

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Adam G-H added 2 commits

    added 2 commits

    Compare with previous version

  • Adam G-H added 1 commit

    added 1 commit

    • f3acce91 - Test change to RsyncValidator

    Compare with previous version

  • Adam G-H added 1 commit

    added 1 commit

    • 1a4b27af - Document AllowDirectWrite attribute

    Compare with previous version

  • Adam G-H added 1 commit

    added 1 commit

    • 3906d926 - Add logging when direct write is enabled

    Compare with previous version

  • 111 111 public function validate(PreOperationStageEvent $event): void {
    112 112 $stage = $event->stage;
    113 113
    114 // If the stage is going to change the active directory directly, we don't
    115 // need to validate the lock file's consistency.
    116 if ($stage->isDirectWrite()) {
  • Adam G-H added 1 commit

    added 1 commit

    • 37bff0e9 - Mark PreconditionBypass as internal

    Compare with previous version

  • Adam G-H added 1 commit

    added 1 commit

    Compare with previous version

  • 464 481 $stage_dir = $this->pathFactory->create($this->getStageDirectory());
    465 482
    466 483 $excluded_paths = $this->getPathsToExclude();
    467 484 $event = new PreApplyEvent($this, $excluded_paths);
    • I think that fact that this event still gets fired even though the changes have been made to live site is a problem. At the very least would need documentation

    • I think documentation (on this and also the pre/post-create events) makes sense, but can you elaborate on what kinds of problems you foresee here?

    • Well, it is not actually pre apply. The changes have already been applied to the site. So any subscriber that assumes it is pre-apply and thinks they can stop the changes via \Drupal\package_manager\Event\PreOperationStageEvent::addError is going have a nasty suprise :scream:

      I haven't look through the docs yet, but from my memory of them is that that change would not be very obvious. Subscribers would need to PreRequireEvent to stop things applying to their site

      Would it be more reasonable in the direct situation to not actually make the change in \Drupal\package_manager\StageBase::require but just save them all up until \Drupal\package_manager\StageBase::apply? I am not saying you should make that change but this is the kind of thing I thinking about as I review it

    • changed this line in version 23 of the diff

    • Please register or sign in to reply
  • Ted Bowman
  • 475 492 $this->failureMarker->write($this, $this->getFailureMarkerMessage());
    476 493
    477 494 try {
    495 if ($this->isDirectWrite()) {
    496 $this->logger?->info($this->t('Direct-write is enabled. Changes have been made to the running code base.'));
  • 345 347 $stage_dir = $this->pathFactory->create($this->getStageDirectory());
    346 348
    347 349 $excluded_paths = $this->getPathsToExclude();
    348 350 $event = new PreCreateEvent($this, $excluded_paths);
  • Ted Bowman
  • Adam G-H added 208 commits

    added 208 commits

    • 59fdd452...d81484de - 206 commits from branch project:11.x
    • 52c61e8f - Merge branch '11.x' into 3503699-make-package-manager
    • a2fb21a9 - Don't do anything in apply() in direct-write mode

    Compare with previous version

  • Adam G-H added 1 commit
  • Adam G-H added 1 commit

    added 1 commit

    • 4cff3106 - Do not dispatch PostApplyEvent

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading