(dev/core#5700) cv dl - Split download substeps. Fix upgrades of EFv1=>EFv2.

[totten]

Overview

cv dl (cv ext:download) allows you to download extension-upgrades. As presented in dev/core#5700, some upgrades are prone to sporadic failures. One reproducible version of this occurs when an extension transitions from "Entity Framework v1" (EFv1) to "Entity Framework v2" (EFv2).

In general, the technique to fix this is to split the download-workflow into multiple subprocesses. The basic issue has been addressed in the web-based downloader for 6.1+ (civicrm/civicrm-core#32285); and there is also a hotfix for the web-based downloader in older versions. This PR applies the same technique to CLI downloads

Before

Upgrading example extension (bottlecap) from v0.1 (EFv1) to v0.2 (EFv2) via cv dl leads to an error:

$ cv dl bottlecap
Using extension feed "https://think.hm/tmp/bottlecap/feed-0.2"
Downloading extension "bottlecap" (https://think.hm/tmp/bottlecap/bottlecap-0.2.zip)

In Bottlecap.php line 14:

  [Error]
  Class "CRM_Bottlecap_DAO_Base" not found

After

Similar commands pass. With verbose output, we see:

$ cv dl bottlecap -vf
Using extension feed "https://think.hm/tmp/bottlecap/feed-0.2"
Fetch "bottlecap" from "https://think.hm/tmp/bottlecap/bottlecap-0.2.zip"
Verify requirements
Swap folders
Rebuild system
Upgrade database
Cleanup workspace

Internally, this required a fair amount of work. For each of those lines ("Verify requirements", "Swap folders", etc), cv needs to open a subprocess to do the work -- and then report-back on its outcome. When there are errors, the errors should propagate back in a sensible way (e.g. being printed to the screen in conformance with usual verbose/non-verbose behavior -- and yielding a decent exit-code).

Comments

• For testing, I focused on the bottlecap extension which can be used to reproduce upgrade problems.
  • My testing got sidetracked for a bit because I wasn't following the test procedure closely enough -- in particular, after any failed download, you need to reset everything. If you don't, then your next download may pass for the wrong reasons.
• The update is loosely coupled to (dev/core#5700) Extensions - "Add New" is supposed to install new extensions civicrm-core#32591:
  • Once 32591 is merged into core, cv will start using the core implementation of CRM_Extension_QueueDownloader.
  • However, on older versions of Civi, cv will use a backport that's included here (./src/ExtensionPolyfill/PfQueueDownloader). This copy should slowly become obsolete as older versions of Civi fade-out.
• Locally, I've used both the raw cv src (bin/cv) and a compiled (bin/cv.phar). It seems to work with both. I've also spot-checked with builds of drupal-clean on 6.2 (with and with 32591), 6.1.0, 5.81, 5.75, 5.63, 5.57, and 5.45.

[totten]

@demeritcowboy I'm curious if this patch works alright on Windows. I'm less concerned about the specific use-cases of dev/core#5700 -- and more concerned about general compatibility:

• The workflow constructs some file-hierarchies while working on the download.
• The patch relies on spawning subprocesses and passing some data back+forth.

I'm cautiously optimistic -- e.g. I avoided pcntl_fork() and used proc_open() (esp 87748d3) because that should have better portability. However, I don't have a good setup for testing on Windows.

[demeritcowboy]

If you mean does just something like cv dl [some extses] work then yes. It leaves a staging folder behind though.

And by accident the first time I tried it I happened to pick some extensions I already had in the ext folder, as git checkouts, and then it gave php warnings and couldn't clean up the "old" subfolders properly because some git files there were read-only. That's probably an edge case. It all still works though.

But even later with no warnings it left an empty staging folder behind.

[totten]

(@demeritcowboy) If you mean does just something like cv dl [some extses] work then yes.

Yes, exactly. 😄

But even later with no warnings it left an empty staging folder behind.

Ah, sure. I was thinking of .civicrm-staging/ as a general container that gets reused for multiple jobs, and then .civicrm-staging/XXXX/ (random ID) is the per-task workspace (to cleanup). But you're right -- most of the time, it's empty and safe to remove.

Added a commit to that bit of cleanup.